From da45340ab84603cf6932b012b977bc2e7b8d6764 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Wed, 5 Feb 2014 21:41:26 +1100 Subject: [PATCH] Ensure an Rc isn't freed while running its own destructor. A weak pointer inside itself will have its destructor run when the last strong pointer to that data disappears, so we need to make sure that the Weak and Rc destructors don't duplicate work (i.e. freeing). By making the Rcs effectively take a weak pointer, we ensure that no Weak destructor will free the pointer while still ensuring that Weak pointers can't be upgraded to strong ones as the destructors run. This approach of starting weak at 1 is what libstdc++ does. Fixes #12046. --- src/libstd/rc.rs | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/libstd/rc.rs b/src/libstd/rc.rs index 7d0ddb2e4fb09..a1565bc85dede 100644 --- a/src/libstd/rc.rs +++ b/src/libstd/rc.rs @@ -50,7 +50,12 @@ impl Rc { pub fn new(value: T) -> Rc { unsafe { Rc { - ptr: transmute(~RcBox { value: value, strong: 1, weak: 0 }), + // there is an implicit weak pointer owned by all the + // strong pointers, which ensures that the weak + // destructor never frees the allocation while the + // strong destructor is running, even if the weak + // pointer is stored inside the strong one. + ptr: transmute(~RcBox { value: value, strong: 1, weak: 1 }), marker: marker::NoSend, } } @@ -81,6 +86,11 @@ impl Drop for Rc { (*self.ptr).strong -= 1; if (*self.ptr).strong == 0 { read_ptr(self.borrow()); // destroy the contained object + + // remove the implicit "strong weak" pointer now + // that we've destroyed the contents. + (*self.ptr).weak -= 1; + if (*self.ptr).weak == 0 { exchange_free(self.ptr as *u8) } @@ -156,7 +166,9 @@ impl Drop for Weak { unsafe { if self.ptr != 0 as *mut RcBox { (*self.ptr).weak -= 1; - if (*self.ptr).weak == 0 && (*self.ptr).strong == 0 { + // the weak count starts at 1, and will only go to + // zero if all the strong pointers have disappeared. + if (*self.ptr).weak == 0 { exchange_free(self.ptr as *u8) } } @@ -242,4 +254,17 @@ mod tests { let a = Rc::new(RefCell::new(Gc::new(1))); assert!(a.borrow().try_borrow_mut().is_some()); } + + #[test] + fn weak_self_cyclic() { + struct Cycle { + x: RefCell>> + } + + let a = Rc::new(Cycle { x: RefCell::new(None) }); + let b = a.clone().downgrade(); + *a.borrow().x.borrow_mut().get() = Some(b); + + // hopefully we don't double-free (or leak)... + } }