Skip to content

Commit

Permalink
auto merge of #12047 : huonw/rust/cyclic-rc, r=thestinger
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Feb 6, 2014
2 parents d8c4e78 + da45340 commit 9a9a70b
Showing 1 changed file with 27 additions and 2 deletions.
29 changes: 27 additions & 2 deletions src/libstd/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,12 @@ impl<T> Rc<T> {
pub fn new(value: T) -> Rc<T> {
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,
}
}
Expand Down Expand Up @@ -81,6 +86,11 @@ impl<T> Drop for Rc<T> {
(*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)
}
Expand Down Expand Up @@ -156,7 +166,9 @@ impl<T> Drop for Weak<T> {
unsafe {
if self.ptr != 0 as *mut RcBox<T> {
(*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)
}
}
Expand Down Expand Up @@ -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<Option<Weak<Cycle>>>
}

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)...
}
}

0 comments on commit 9a9a70b

Please sign in to comment.