From 4b75a1808a0ee75383f81659f6262fefc7047a09 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 24 Jul 2015 12:25:44 +0200 Subject: [PATCH] show some more code in the workspace --- solutions/src/callbacks.rs | 16 ++++++++++------ src/part06.rs | 4 ++-- src/part07.rs | 8 ++++---- src/part12.rs | 18 +++++++++--------- workspace/src/part06.rs | 1 + workspace/src/part07.rs | 5 ++++- workspace/src/part12.rs | 5 ++--- 7 files changed, 32 insertions(+), 25 deletions(-) diff --git a/solutions/src/callbacks.rs b/solutions/src/callbacks.rs index 93fcb17..a42732d 100644 --- a/solutions/src/callbacks.rs +++ b/solutions/src/callbacks.rs @@ -16,7 +16,7 @@ impl Callbacks { self.callbacks.push(cell); /*@*/ } - pub fn call(&self, val: i32) { + pub fn call(&mut self, val: i32) { for callback in self.callbacks.iter() { // We have to *explicitly* borrow the contents of a `RefCell`. //@ At run-time, the cell will keep track of the number of outstanding shared and mutable borrows, @@ -53,14 +53,18 @@ mod tests { let c = Rc::new(RefCell::new(Callbacks::new())); c.borrow_mut().register(|val| println!("Callback called: {}", val) ); - // If we change the two "borrow" below to "borrow_mut", you can get a panic even with a "call" that requires a - // mutable borrow. However, that panic is then triggered by our own, external `RefCell` (so it's kind of our fault), - // rather than being triggered by the `RefCell` in the `Callbacks`. { let c2 = c.clone(); - c.borrow_mut().register(move |val| c2.borrow().call(val+val) ); + c.borrow_mut().register(move |val| { + let mut guard = c2.borrow_mut(); + println!("Callback called with {}, ready to go for nested call.", val); + guard.call(val+val) + } ); } - c.borrow().call(42); + // We do a clone, and call `call` on that one. This makes sure that it's not our `RefCell` that complains about two mutable borrows, + // but rather the `RefCell` inside the `CallbacksMut`. + let mut c2: Callbacks = c.borrow().clone(); + c2.call(42); } } \ No newline at end of file diff --git a/src/part06.rs b/src/part06.rs index 6d50c2d..8161d2d 100644 --- a/src/part06.rs +++ b/src/part06.rs @@ -30,7 +30,7 @@ fn vec_min(v: &Vec) -> Option { let mut min: Option = None; // If `v` is a shared borrowed vector, then the default for iterating over it is to call `iter`, the iterator that borrows the elements. for e in v { - let e = e.clone(); /*@*/ + let e = e.clone(); min = Some(match min { /*@*/ None => e, /*@*/ Some(n) => e.min_try1(n) /*@*/ @@ -50,7 +50,7 @@ fn vec_min(v: &Vec) -> Option { //@ underlying data is transferred from where `e` borrows from to `min`. But that's not allowed, since //@ we just borrowed `e`, so we cannot empty it! We can, however, call `clone` on it. Then we own //@ the copy that was created, and hence we can store it in `min`.
-//@ Of course, making such a full copy is expensive, so we'd like to avoid it. We'll some to that in the next part. +//@ Of course, making such a full copy is expensive, so we'd like to avoid it. We'll come to that in the next part. // ## `Copy` types //@ But before we go there, I should answer the second question I brought up above: Why did our old `vec_min` work? diff --git a/src/part07.rs b/src/part07.rs index 151a3d6..4c143d5 100644 --- a/src/part07.rs +++ b/src/part07.rs @@ -17,10 +17,10 @@ pub trait Minimum { pub fn vec_min(v: &Vec) -> Option<&T> { let mut min: Option<&T> = None; for e in v { - min = Some(match min { /*@*/ - None => e, /*@*/ - Some(n) => n.min(e) /*@*/ - }); /*@*/ + min = Some(match min { + None => e, + Some(n) => n.min(e) + }); } min } diff --git a/src/part12.rs b/src/part12.rs index 58afd8d..f186b2a 100644 --- a/src/part12.rs +++ b/src/part12.rs @@ -127,12 +127,13 @@ impl CallbacksMut { //@ appropriately updates the number of active borrows. //@ //@ Since `call` is the only place that borrows the environments of the closures, we should expect that - //@ the check will always succeed. However, this function would still typecheck with an immutable borrow of `self` (since we are - //@ relying on the interior mutability of `RefCell`). Under this condition, it could happen that a callback - //@ will in turn trigger another round of callbacks, so that `call` would indirectly call itself. - //@ This is called reentrancy. It would imply that we borrow the closure a second time, and - //@ panic at run-time. I hope this also makes it clear that there's absolutely no hope of Rust - //@ performing these checks statically, at compile-time: It would have to detect reentrancy! + //@ the check will always succeed. However, this is not actually true. Several different `CallbacksMut` could share + //@ a callback (as they were created with `clone`), and calling one callback here could trigger calling + //@ all callbacks of the other `CallbacksMut`, which would end up calling the initial callback again. This issue is called *reentrancy*, + //@ and it can lead to subtle bugs. Here, it would mean that the closure runs twice, each time thinking it has the only + //@ mutable borrow of its environment - so it may end up dereferencing a dangling pointer. Ouch! Lucky enough, + //@ Rust detects this at run-time and panics once we try to borrow the same environment again. I hope this also makes it + //@ clear that there's absolutely no hope of Rust performing these checks statically, at compile-time: It would have to detect reentrancy! let mut closure = callback.borrow_mut(); // Unfortunately, Rust's auto-dereference of pointers is not clever enough here. We thus have to explicitly // dereference the smart pointer and obtain a mutable borrow of the content. @@ -157,8 +158,7 @@ fn demo_mut(c: &mut CallbacksMut) { c.call(1); c.clone().call(2); } -// **Exercise 12.1**: Change the type of `call` to ask only for a shared borrow. Then write some piece of code using only the available, public -// interface of `CallbacksMut` such that a reentrant call to `call` is happening, and the program aborts because the `RefCell` refuses to hand -// out a second mutable borrow to its content. +// **Exercise 12.1**: Write some piece of code using only the available, public interface of `CallbacksMut` such that a reentrant call to a closure +// is happening, and the program aborts because the `RefCell` refuses to hand out a second mutable borrow of the closure's environment. //@ [index](main.html) | [previous](part11.html) | [next](part13.html) diff --git a/workspace/src/part06.rs b/workspace/src/part06.rs index 15b55d2..04a0847 100644 --- a/workspace/src/part06.rs +++ b/workspace/src/part06.rs @@ -27,6 +27,7 @@ fn vec_min(v: &Vec) -> Option { let mut min: Option = None; // If `v` is a shared borrowed vector, then the default for iterating over it is to call `iter`, the iterator that borrows the elements. for e in v { + let e = e.clone(); unimplemented!() } min diff --git a/workspace/src/part07.rs b/workspace/src/part07.rs index e2796b4..75bd0cc 100644 --- a/workspace/src/part07.rs +++ b/workspace/src/part07.rs @@ -11,7 +11,10 @@ pub trait Minimum { pub fn vec_min(v: &Vec) -> Option<&T> { let mut min: Option<&T> = None; for e in v { - unimplemented!() + min = Some(match min { + None => e, + Some(n) => n.min(e) + }); } min } diff --git a/workspace/src/part12.rs b/workspace/src/part12.rs index 23db4f6..86fb4bb 100644 --- a/workspace/src/part12.rs +++ b/workspace/src/part12.rs @@ -103,7 +103,6 @@ fn demo_mut(c: &mut CallbacksMut) { c.call(1); c.clone().call(2); } -// **Exercise 12.1**: Change the type of `call` to ask only for a shared borrow. Then write some piece of code using only the available, public -// interface of `CallbacksMut` such that a reentrant call to `call` is happening, and the program aborts because the `RefCell` refuses to hand -// out a second mutable borrow to its content. +// **Exercise 12.1**: Write some piece of code using only the available, public interface of `CallbacksMut` such that a reentrant call to a closure +// is happening, and the program aborts because the `RefCell` refuses to hand out a second mutable borrow of the closure's environment. -- 2.30.2