From 7d897789c28fca8248851f8a6d6976c08c133b1c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 13 Feb 2026 15:47:43 -0800 Subject: [PATCH] Refactor sized Rc/Arc allocation/initialization This commit refactors the `Rc` and `Arc` types in the standard library to have a new narrow waist for all allocations of sized types. Specifically a new `{Rc,Arc}::from_box_uninit_inner` function is added which takes an entirely-uninitialized `Box` pointer, initializes the reference counts, and then returns `Rc`/`Arc` where the payload is `MaybeUninit`. This is purely an internal refactor of functions at this time and doesn't expose any new API surface area for either `Arc` or `Rc`. It's also worth noting that there remain two primary ways of allocating `Rc`/`Arc`, like before. Notably allocating `Rc<[T]>` is not possible with this helper because `MaybeUninit` does not support unsized types at this time. It's hoped though that in the future if this is supported that all allocations of `Rc`/`Arc` could be funneled through this method. While this refactor was done I went through a number of other constructors/etc and simplified the implementations to use combinations of other APIs of `Rc`/`Arc`, removing a number of `unsafe` blocks along the way. The overall goal of this refactor is to move towards [this discussion on Zulip][Zulip]. Eventually the `from_box_uninit_inner` function would perhaps be exposed as a `From` impl on `Rc` and `Arc`, or maybe a public inherent function. This would require making the `{Rc,Arc}Inner` types public, however, so this part is deferred for a future ACP. In the meantime though I thought that this pattern of initialization was pretty nifty and would be a good internal refactoring to do in the meantime regardless. [Zulip]: https://rust-lang.zulipchat.com/#narrow/channel/197181-t-libs.2Fwg-allocators/topic/Fallible.20allocation.20of.20.60Arc.3CT.3E.60.20w.2Fout.20all.20of.20.60allocator_api.60/near/569793763 --- library/alloc/src/rc.rs | 198 ++++++++++++++++------------------ library/alloc/src/sync.rs | 222 +++++++++++++++++--------------------- 2 files changed, 191 insertions(+), 229 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index cec41524325e0..02250d4de69d6 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -411,16 +411,16 @@ impl Rc { #[cfg(not(no_global_oom_handling))] #[stable(feature = "rust1", since = "1.0.0")] pub fn new(value: T) -> Rc { - // 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. + let mut rc = Rc::new_uninit(); + + // SAFETY: this is a freshly allocated `Rc` so it's guaranteed there are + // no other strong or weak pointers other than `rc` itself. unsafe { - Self::from_inner( - Box::leak(Box::new(RcInner { strong: Cell::new(1), weak: Cell::new(1), value })) - .into(), - ) + Rc::get_mut_unchecked(&mut rc).write(value); } + + // SAFETY: this allocation was just initialized above. + unsafe { rc.assume_init() } } /// Constructs a new `Rc` while giving you a `Weak` to the allocation, @@ -503,13 +503,7 @@ impl Rc { #[stable(feature = "new_uninit", since = "1.82.0")] #[must_use] pub fn new_uninit() -> Rc> { - unsafe { - Rc::from_ptr(Rc::allocate_for_layout( - Layout::new::(), - |layout| Global.allocate(layout), - <*mut u8>::cast, - )) - } + Rc::from_box_uninit_inner(Box::new_uninit()) } /// Constructs a new `Rc` with uninitialized contents, with the memory @@ -534,13 +528,7 @@ impl Rc { #[stable(feature = "new_zeroed_alloc", since = "1.92.0")] #[must_use] pub fn new_zeroed() -> Rc> { - unsafe { - Rc::from_ptr(Rc::allocate_for_layout( - Layout::new::(), - |layout| Global.allocate_zeroed(layout), - <*mut u8>::cast, - )) - } + Rc::from_box_uninit_inner(Box::new_zeroed()) } /// Constructs a new `Rc`, returning an error if the allocation fails @@ -556,20 +544,16 @@ impl Rc { /// ``` #[unstable(feature = "allocator_api", issue = "32838")] pub fn try_new(value: T) -> Result, AllocError> { - // 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. + let mut rc = Rc::try_new_uninit()?; + + // SAFETY: this is a freshly allocated `Rc` so it's guaranteed there are + // no other strong or weak pointers other than `rc` itself. unsafe { - Ok(Self::from_inner( - Box::leak(Box::try_new(RcInner { - strong: Cell::new(1), - weak: Cell::new(1), - value, - })?) - .into(), - )) + Rc::get_mut_unchecked(&mut rc).write(value); } + + // SAFETY: this allocation was just initialized above. + Ok(unsafe { rc.assume_init() }) } /// Constructs a new `Rc` with uninitialized contents, returning an error if the allocation fails @@ -593,13 +577,7 @@ impl Rc { /// ``` #[unstable(feature = "allocator_api", issue = "32838")] pub fn try_new_uninit() -> Result>, AllocError> { - unsafe { - Ok(Rc::from_ptr(Rc::try_allocate_for_layout( - Layout::new::(), - |layout| Global.allocate(layout), - <*mut u8>::cast, - )?)) - } + Ok(Rc::from_box_uninit_inner(Box::try_new_uninit()?)) } /// Constructs a new `Rc` with uninitialized contents, with the memory @@ -625,14 +603,9 @@ impl Rc { /// [zeroed]: mem::MaybeUninit::zeroed #[unstable(feature = "allocator_api", issue = "32838")] pub fn try_new_zeroed() -> Result>, AllocError> { - unsafe { - Ok(Rc::from_ptr(Rc::try_allocate_for_layout( - Layout::new::(), - |layout| Global.allocate_zeroed(layout), - <*mut u8>::cast, - )?)) - } + Ok(Rc::from_box_uninit_inner(Box::try_new_zeroed()?)) } + /// Constructs a new `Pin>`. If `T` does not implement `Unpin`, then /// `value` will be pinned in memory and unable to be moved. #[cfg(not(no_global_oom_handling))] @@ -780,16 +753,7 @@ impl Rc { #[unstable(feature = "allocator_api", issue = "32838")] #[inline] pub fn new_uninit_in(alloc: A) -> Rc, A> { - unsafe { - Rc::from_ptr_in( - Rc::allocate_for_layout( - Layout::new::(), - |layout| alloc.allocate(layout), - <*mut u8>::cast, - ), - alloc, - ) - } + Rc::from_box_uninit_inner(Box::new_uninit_in(alloc)) } /// Constructs a new `Rc` with uninitialized contents, with the memory @@ -817,16 +781,7 @@ impl Rc { #[unstable(feature = "allocator_api", issue = "32838")] #[inline] pub fn new_zeroed_in(alloc: A) -> Rc, A> { - unsafe { - Rc::from_ptr_in( - Rc::allocate_for_layout( - Layout::new::(), - |layout| alloc.allocate_zeroed(layout), - <*mut u8>::cast, - ), - alloc, - ) - } + Rc::from_box_uninit_inner(Box::new_zeroed_in(alloc)) } /// Constructs a new `Rc` in the given allocator while giving you a `Weak` to the allocation, @@ -923,15 +878,16 @@ impl Rc { #[unstable(feature = "allocator_api", issue = "32838")] #[inline] pub fn try_new_in(value: T, alloc: A) -> Result { - // 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. - let (ptr, alloc) = Box::into_unique(Box::try_new_in( - RcInner { strong: Cell::new(1), weak: Cell::new(1), value }, - alloc, - )?); - Ok(unsafe { Self::from_inner_in(ptr.into(), alloc) }) + let mut rc = Rc::try_new_uninit_in(alloc)?; + + // SAFETY: this is a freshly allocated `Rc` so it's guaranteed there are + // no other strong or weak pointers other than `rc` itself. + unsafe { + Rc::get_mut_unchecked(&mut rc).write(value); + } + + // SAFETY: this allocation was just initialized above. + unsafe { Ok(rc.assume_init()) } } /// Constructs a new `Rc` with uninitialized contents, in the provided allocator, returning an @@ -961,16 +917,7 @@ impl Rc { #[unstable(feature = "allocator_api", issue = "32838")] #[inline] pub fn try_new_uninit_in(alloc: A) -> Result, A>, AllocError> { - unsafe { - Ok(Rc::from_ptr_in( - Rc::try_allocate_for_layout( - Layout::new::(), - |layout| alloc.allocate(layout), - <*mut u8>::cast, - )?, - alloc, - )) - } + Ok(Rc::from_box_uninit_inner(Box::try_new_uninit_in(alloc)?)) } /// Constructs a new `Rc` with uninitialized contents, with the memory @@ -999,16 +946,7 @@ impl Rc { #[unstable(feature = "allocator_api", issue = "32838")] #[inline] pub fn try_new_zeroed_in(alloc: A) -> Result, A>, AllocError> { - unsafe { - Ok(Rc::from_ptr_in( - Rc::try_allocate_for_layout( - Layout::new::(), - |layout| alloc.allocate_zeroed(layout), - <*mut u8>::cast, - )?, - alloc, - )) - } + Ok(Rc::from_box_uninit_inner(Box::try_new_zeroed_in(alloc)?)) } /// Constructs a new `Pin>` in the provided allocator. If `T` does not implement `Unpin`, then @@ -1098,6 +1036,40 @@ impl Rc { pub fn into_inner(this: Self) -> Option { Rc::try_unwrap(this).ok() } + + fn from_box_uninit_inner( + boxed: Box>, A>, + ) -> Rc, A> { + let (inner, alloc) = Box::into_non_null_with_allocator(boxed); + + // This translates `NonNull>>` + // to `NonNull>>`. It's not yet safe to + // turn that into an `Rc` because the strong/weak fields need + // initialization. That's done next. + let inner = inner.cast::>>(); + + // SAFETY: `inner` is a valid pointer to an uninitialized allocation + // which means that it is safe to initialize. Here of the three fields + // that `RcInner` contains, weak/strong/value, two are initialized. The + // `value` field is left uninitialized which is modeled through the + // return value of this function where `MaybeUninit` is used to + // represent the payload of the `Rc`. + // + // 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. Hence both the strong and the weak + // count are both initialized to one. + unsafe { + (&raw mut (*inner.as_ptr()).strong).write(Cell::new(1)); + (&raw mut (*inner.as_ptr()).weak).write(Cell::new(1)); + } + + // SAFETY: `inner` is allocated by `alloc`, it's strong/weak counts are + // initialized, and `value` is modeled as uninitialized in the return + // value to represent its lack of initialization. + unsafe { Rc::from_inner_in(inner, alloc) } + } } impl Rc<[T]> { @@ -4300,17 +4272,27 @@ impl UniqueRc { #[cfg(not(no_global_oom_handling))] #[unstable(feature = "unique_rc_arc", issue = "112566")] pub fn new_in(value: T, alloc: A) -> Self { - let (ptr, alloc) = Box::into_unique(Box::new_in( - RcInner { - strong: Cell::new(0), - // keep one weak reference so if all the weak pointers that are created are dropped - // the UniqueRc still stays valid. - weak: Cell::new(1), - value, - }, - alloc, - )); - Self { ptr: ptr.into(), _marker: PhantomData, _marker2: PhantomData, alloc } + let mut rc = Self::from_box_uninit_inner(Box::new_uninit_in(alloc)); + unsafe { + rc.write(value); + rc.assume_init() + } + } + + fn from_box_uninit_inner( + boxed: Box>, A>, + ) -> UniqueRc, A> { + let (inner, alloc) = Box::into_non_null_with_allocator(boxed); + let inner = inner.cast::>>(); + + // SAFETY: same as above for `Rc` + unsafe { + (&raw mut (*inner.as_ptr()).strong).write(Cell::new(0)); + // keep one weak reference so if all the weak pointers that are + // created are dropped the UniqueRc still stays valid. + (&raw mut (*inner.as_ptr()).weak).write(Cell::new(1)); + } + unsafe { UniqueRc::from_inner_in(inner, alloc) } } } diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index dc82357dd146b..bb741e76279c4 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -417,14 +417,16 @@ impl Arc { #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub fn new(data: T) -> Arc { - // Start the weak pointer count as 1 which is the weak pointer that's - // held by all the strong pointers (kinda), see std/rc.rs for more info - let x: Box<_> = Box::new(ArcInner { - strong: atomic::AtomicUsize::new(1), - weak: atomic::AtomicUsize::new(1), - data, - }); - unsafe { Self::from_inner(Box::leak(x).into()) } + let mut arc = Arc::new_uninit(); + + // SAFETY: this is a freshly allocated `Arc` so it's guaranteed there + // are no other strong or weak pointers other than `arc` itself. + unsafe { + Arc::get_mut_unchecked(&mut arc).write(data); + } + + // SAFETY: this allocation was just initialized above. + unsafe { arc.assume_init() } } /// Constructs a new `Arc` while giving you a `Weak` to the allocation, @@ -509,13 +511,7 @@ impl Arc { #[stable(feature = "new_uninit", since = "1.82.0")] #[must_use] pub fn new_uninit() -> Arc> { - unsafe { - Arc::from_ptr(Arc::allocate_for_layout( - Layout::new::(), - |layout| Global.allocate(layout), - <*mut u8>::cast, - )) - } + Arc::from_box_uninit_inner(Box::new_uninit()) } /// Constructs a new `Arc` with uninitialized contents, with the memory @@ -541,13 +537,7 @@ impl Arc { #[stable(feature = "new_zeroed_alloc", since = "1.92.0")] #[must_use] pub fn new_zeroed() -> Arc> { - unsafe { - Arc::from_ptr(Arc::allocate_for_layout( - Layout::new::(), - |layout| Global.allocate_zeroed(layout), - <*mut u8>::cast, - )) - } + Arc::from_box_uninit_inner(Box::new_zeroed()) } /// Constructs a new `Pin>`. If `T` does not implement `Unpin`, then @@ -580,14 +570,16 @@ impl Arc { #[unstable(feature = "allocator_api", issue = "32838")] #[inline] pub fn try_new(data: T) -> Result, AllocError> { - // Start the weak pointer count as 1 which is the weak pointer that's - // held by all the strong pointers (kinda), see std/rc.rs for more info - let x: Box<_> = Box::try_new(ArcInner { - strong: atomic::AtomicUsize::new(1), - weak: atomic::AtomicUsize::new(1), - data, - })?; - unsafe { Ok(Self::from_inner(Box::leak(x).into())) } + let mut arc = Arc::try_new_uninit()?; + + // SAFETY: this is a freshly allocated `Arc` so it's guaranteed there + // are no other strong or weak pointers other than `arc` itself. + unsafe { + Arc::get_mut_unchecked(&mut arc).write(data); + } + + // SAFETY: this allocation was just initialized above. + Ok(unsafe { arc.assume_init() }) } /// Constructs a new `Arc` with uninitialized contents, returning an error @@ -612,13 +604,7 @@ impl Arc { /// ``` #[unstable(feature = "allocator_api", issue = "32838")] pub fn try_new_uninit() -> Result>, AllocError> { - unsafe { - Ok(Arc::from_ptr(Arc::try_allocate_for_layout( - Layout::new::(), - |layout| Global.allocate(layout), - <*mut u8>::cast, - )?)) - } + Ok(Arc::from_box_uninit_inner(Box::try_new_uninit()?)) } /// Constructs a new `Arc` with uninitialized contents, with the memory @@ -644,13 +630,7 @@ impl Arc { /// [zeroed]: mem::MaybeUninit::zeroed #[unstable(feature = "allocator_api", issue = "32838")] pub fn try_new_zeroed() -> Result>, AllocError> { - unsafe { - Ok(Arc::from_ptr(Arc::try_allocate_for_layout( - Layout::new::(), - |layout| Global.allocate_zeroed(layout), - <*mut u8>::cast, - )?)) - } + Ok(Arc::from_box_uninit_inner(Box::try_new_zeroed()?)) } /// Maps the value in an `Arc`, reusing the allocation if possible. @@ -758,18 +738,16 @@ impl Arc { #[cfg(not(no_global_oom_handling))] #[unstable(feature = "allocator_api", issue = "32838")] pub fn new_in(data: T, alloc: A) -> Arc { - // Start the weak pointer count as 1 which is the weak pointer that's - // held by all the strong pointers (kinda), see std/rc.rs for more info - let x = Box::new_in( - ArcInner { - strong: atomic::AtomicUsize::new(1), - weak: atomic::AtomicUsize::new(1), - data, - }, - alloc, - ); - let (ptr, alloc) = Box::into_unique(x); - unsafe { Self::from_inner_in(ptr.into(), alloc) } + let mut arc = Arc::new_uninit_in(alloc); + + // SAFETY: this is a freshly allocated `Arc` so it's guaranteed there + // are no other strong or weak pointers other than `arc` itself. + unsafe { + Arc::get_mut_unchecked(&mut arc).write(data); + } + + // SAFETY: this allocation was just initialized above. + unsafe { arc.assume_init() } } /// Constructs a new `Arc` with uninitialized contents in the provided allocator. @@ -798,16 +776,7 @@ impl Arc { #[unstable(feature = "allocator_api", issue = "32838")] #[inline] pub fn new_uninit_in(alloc: A) -> Arc, A> { - unsafe { - Arc::from_ptr_in( - Arc::allocate_for_layout( - Layout::new::(), - |layout| alloc.allocate(layout), - <*mut u8>::cast, - ), - alloc, - ) - } + Arc::from_box_uninit_inner(Box::new_uninit_in(alloc)) } /// Constructs a new `Arc` with uninitialized contents, with the memory @@ -835,16 +804,7 @@ impl Arc { #[unstable(feature = "allocator_api", issue = "32838")] #[inline] pub fn new_zeroed_in(alloc: A) -> Arc, A> { - unsafe { - Arc::from_ptr_in( - Arc::allocate_for_layout( - Layout::new::(), - |layout| alloc.allocate_zeroed(layout), - <*mut u8>::cast, - ), - alloc, - ) - } + Arc::from_box_uninit_inner(Box::new_zeroed_in(alloc)) } /// Constructs a new `Arc` in the given allocator while giving you a `Weak` to the allocation, @@ -978,18 +938,16 @@ impl Arc { #[unstable(feature = "allocator_api", issue = "32838")] #[inline] pub fn try_new_in(data: T, alloc: A) -> Result, AllocError> { - // Start the weak pointer count as 1 which is the weak pointer that's - // held by all the strong pointers (kinda), see std/rc.rs for more info - let x = Box::try_new_in( - ArcInner { - strong: atomic::AtomicUsize::new(1), - weak: atomic::AtomicUsize::new(1), - data, - }, - alloc, - )?; - let (ptr, alloc) = Box::into_unique(x); - Ok(unsafe { Self::from_inner_in(ptr.into(), alloc) }) + let mut arc = Arc::try_new_uninit_in(alloc)?; + + // SAFETY: this is a freshly allocated `Arc` so it's guaranteed there + // are no other strong or weak pointers other than `arc` itself. + unsafe { + Arc::get_mut_unchecked(&mut arc).write(data); + } + + // SAFETY: this allocation was just initialized above. + Ok(unsafe { arc.assume_init() }) } /// Constructs a new `Arc` with uninitialized contents, in the provided allocator, returning an @@ -1019,16 +977,7 @@ impl Arc { #[unstable(feature = "allocator_api", issue = "32838")] #[inline] pub fn try_new_uninit_in(alloc: A) -> Result, A>, AllocError> { - unsafe { - Ok(Arc::from_ptr_in( - Arc::try_allocate_for_layout( - Layout::new::(), - |layout| alloc.allocate(layout), - <*mut u8>::cast, - )?, - alloc, - )) - } + Ok(Arc::from_box_uninit_inner(Box::try_new_uninit_in(alloc)?)) } /// Constructs a new `Arc` with uninitialized contents, with the memory @@ -1057,17 +1006,9 @@ impl Arc { #[unstable(feature = "allocator_api", issue = "32838")] #[inline] pub fn try_new_zeroed_in(alloc: A) -> Result, A>, AllocError> { - unsafe { - Ok(Arc::from_ptr_in( - Arc::try_allocate_for_layout( - Layout::new::(), - |layout| alloc.allocate_zeroed(layout), - <*mut u8>::cast, - )?, - alloc, - )) - } + Ok(Arc::from_box_uninit_inner(Box::try_new_zeroed_in(alloc)?)) } + /// Returns the inner value, if the `Arc` has exactly one strong reference. /// /// Otherwise, an [`Err`] is returned with the same `Arc` that was @@ -1244,6 +1185,29 @@ impl Arc { Some(inner) } + + fn from_box_uninit_inner( + boxed: Box>, A>, + ) -> Arc, A> { + let (inner, alloc) = Box::into_non_null_with_allocator(boxed); + + // This translates `NonNull>>` + // to `NonNull>>`. It's not yet safe to + // turn that into an `Arc` because the strong/weak fields need + // initialization. That's done next. + let inner = inner.cast::>>(); + + // SAFETY: `inner` is a valid pointer to an uninitialized allocation + // which means that it is safe to initialize. + unsafe { + Arc::initialize_arcinner_refcnts(inner.as_ptr()); + } + + // SAFETY: `inner` is allocated by `alloc`, it's strong/weak counts are + // initialized, and `value` is modeled as uninitialized in the return + // value to represent its lack of initialization. + unsafe { Arc::from_inner_in(inner, alloc) } + } } impl Arc<[T]> { @@ -2193,12 +2157,18 @@ impl Arc { debug_assert_eq!(unsafe { Layout::for_value_raw(inner) }, layout); unsafe { - (&raw mut (*inner).strong).write(atomic::AtomicUsize::new(1)); - (&raw mut (*inner).weak).write(atomic::AtomicUsize::new(1)); + Arc::initialize_arcinner_refcnts(inner); } inner } + + unsafe fn initialize_arcinner_refcnts(inner: *mut ArcInner) { + unsafe { + (&raw mut (*inner).strong).write(atomic::AtomicUsize::new(1)); + (&raw mut (*inner).weak).write(atomic::AtomicUsize::new(1)); + } + } } impl Arc { @@ -4730,17 +4700,27 @@ impl UniqueArc { #[must_use] // #[unstable(feature = "allocator_api", issue = "32838")] pub fn new_in(data: T, alloc: A) -> Self { - let (ptr, alloc) = Box::into_unique(Box::new_in( - ArcInner { - strong: atomic::AtomicUsize::new(0), - // keep one weak reference so if all the weak pointers that are created are dropped - // the UniqueArc still stays valid. - weak: atomic::AtomicUsize::new(1), - data, - }, - alloc, - )); - Self { ptr: ptr.into(), _marker: PhantomData, _marker2: PhantomData, alloc } + let mut arc = Self::from_box_uninit_inner(Box::new_uninit_in(alloc)); + unsafe { + arc.write(data); + arc.assume_init() + } + } + + fn from_box_uninit_inner( + boxed: Box>, A>, + ) -> UniqueArc, A> { + let (inner, alloc) = Box::into_non_null_with_allocator(boxed); + let inner = inner.cast::>>(); + + // SAFETY: same as above for `Arc` + unsafe { + (&raw mut (*inner.as_ptr()).strong).write(atomic::AtomicUsize::new(0)); + // keep one weak reference so if all the weak pointers that are + // created are dropped the UniqueArc still stays valid. + (&raw mut (*inner.as_ptr()).weak).write(atomic::AtomicUsize::new(1)); + } + unsafe { UniqueArc::from_inner_in(inner, alloc) } } }