From f03d0b38d6a33a64307d83f8ddd3df8ef57ca537 Mon Sep 17 00:00:00 2001 From: mental32 Date: Thu, 27 Aug 2020 19:19:29 +0100 Subject: [PATCH 1/3] `impl Rc::new_cyclic` --- library/alloc/src/rc.rs | 34 ++++++++++++++++++ library/alloc/src/rc/tests.rs | 66 +++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 1046397f4be60..76266d77bb0d2 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -325,6 +325,40 @@ impl Rc { ) } + /// Constructs a new `Rc` using a weak reference to itself. Attempting + /// to upgrade the weak reference before this function retuns will result + /// in a `None` value. However, the weak reference may be cloned freely and + /// stored for use at a later time. + #[inline] + #[unstable(feature = "arc_new_cyclic", issue = "75861")] + pub fn new_cyclic(data_fn: impl FnOnce(&Weak) -> T) -> Rc { + let uninit_ptr: NonNull<_> = Box::leak(box RcBox { + strong: Cell::new(0), + weak: Cell::new(1), + value: mem::MaybeUninit::::uninit(), + }) + .into(); + + let init_ptr: NonNull> = uninit_ptr.cast(); + + let weak = Weak { ptr: init_ptr }; + + let data = data_fn(&weak); + + unsafe { + let inner = init_ptr.as_ptr(); + ptr::write(&raw mut (*inner).value, data); + + let prev_value = (*inner).strong.get(); + debug_assert_eq!(prev_value, 0, "No prior strong references should exist"); + (*inner).strong.set(1); + } + + let strong = Rc::from_inner(init_ptr); + mem::forget(weak); + strong + } + /// Constructs a new `Rc` with uninitialized contents. /// /// # Examples diff --git a/library/alloc/src/rc/tests.rs b/library/alloc/src/rc/tests.rs index e88385faf4fd4..fed48a59f809e 100644 --- a/library/alloc/src/rc/tests.rs +++ b/library/alloc/src/rc/tests.rs @@ -434,3 +434,69 @@ fn test_array_from_slice() { let a: Result, _> = r.clone().try_into(); assert!(a.is_err()); } + +#[test] +fn test_rc_cyclic_with_zero_refs() { + struct ZeroRefs { + inner: Weak, + } + + let zero_refs = Rc::new_cyclic(|inner| { + assert_eq!(inner.strong_count(), 0); + assert!(inner.upgrade().is_none()); + ZeroRefs { inner: Weak::new() } + }); + + assert_eq!(Rc::strong_count(&zero_refs), 1); + assert_eq!(Rc::weak_count(&zero_refs), 0); + assert_eq!(zero_refs.inner.strong_count(), 0); + assert_eq!(zero_refs.inner.weak_count(), 0); +} + +#[test] +fn test_rc_cyclic_with_one_ref() { + struct OneRef { + inner: Weak, + } + + let one_ref = Rc::new_cyclic(|inner| { + assert_eq!(inner.strong_count(), 0); + assert!(inner.upgrade().is_none()); + OneRef { inner: inner.clone() } + }); + + assert_eq!(Rc::strong_count(&one_ref), 1); + assert_eq!(Rc::weak_count(&one_ref), 1); + + let one_ref2 = Weak::upgrade(&one_ref.inner).unwrap(); + assert!(Rc::ptr_eq(&one_ref, &one_ref2)); + + assert_eq!(one_ref.inner.strong_count(), 2); + assert_eq!(one_ref.inner.weak_count(), 1); +} + +#[test] +fn test_rc_cyclic_with_two_ref() { + struct TwoRefs { + inner: Weak, + inner1: Weak, + } + + let two_refs = Rc::new_cyclic(|inner| { + assert_eq!(inner.strong_count(), 0); + assert!(inner.upgrade().is_none()); + TwoRefs { inner: inner.clone(), inner1: inner.clone() } + }); + + assert_eq!(Rc::strong_count(&two_refs), 1); + assert_eq!(Rc::weak_count(&two_refs), 2); + + let two_ref3 = Weak::upgrade(&two_refs.inner).unwrap(); + assert!(Rc::ptr_eq(&two_refs, &two_ref3)); + + let two_ref2 = Weak::upgrade(&two_refs.inner1).unwrap(); + assert!(Rc::ptr_eq(&two_refs, &two_ref2)); + + assert_eq!(Rc::strong_count(&two_refs), 3); + assert_eq!(Rc::weak_count(&two_refs), 2); +} From 42fb27001e07e832cb40604c7daeaa6aada07675 Mon Sep 17 00:00:00 2001 From: mental Date: Sat, 29 Aug 2020 07:39:03 +0100 Subject: [PATCH 2/3] typo Co-authored-by: Andrew Hickman --- library/alloc/src/rc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 76266d77bb0d2..7dbdc8f6017fe 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -326,7 +326,7 @@ impl Rc { } /// Constructs a new `Rc` using a weak reference to itself. Attempting - /// to upgrade the weak reference before this function retuns will result + /// to upgrade the weak reference before this function returns will result /// in a `None` value. However, the weak reference may be cloned freely and /// stored for use at a later time. #[inline] From 0f301e8bb40aaf7cbfefb8c16ce3b0a112c6d5c1 Mon Sep 17 00:00:00 2001 From: mental Date: Tue, 1 Sep 2020 09:46:48 +0100 Subject: [PATCH 3/3] Removed [inline] and copied over comments from Arc::new_cyclic --- library/alloc/src/rc.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index 7dbdc8f6017fe..1a2dfd2888261 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -329,9 +329,10 @@ impl Rc { /// to upgrade the weak reference before this function returns will result /// in a `None` value. However, the weak reference may be cloned freely and /// stored for use at a later time. - #[inline] #[unstable(feature = "arc_new_cyclic", issue = "75861")] pub fn new_cyclic(data_fn: impl FnOnce(&Weak) -> T) -> Rc { + // Construct the inner in the "uninitialized" state with a single + // weak reference. let uninit_ptr: NonNull<_> = Box::leak(box RcBox { strong: Cell::new(0), weak: Cell::new(1), @@ -343,6 +344,12 @@ impl Rc { let weak = Weak { ptr: init_ptr }; + // It's important we don't give up ownership of the weak pointer, or + // else the memory might be freed by the time `data_fn` returns. If + // we really wanted to pass ownership, we could create an additional + // weak pointer for ourselves, but this would result in additional + // updates to the weak reference count which might not be necessary + // otherwise. let data = data_fn(&weak); unsafe { @@ -355,6 +362,9 @@ impl Rc { } let strong = Rc::from_inner(init_ptr); + + // Strong references should collectively own a shared weak reference, + // so don't run the destructor for our old weak reference. mem::forget(weak); strong }