Update documentation for Arc::from_raw, Arc::increment_strong_count, and Arc::decrement_strong_count to clarify allocator requirement

[DiuDiu777]

Related Issue:

This update addresses parts of the issue raised in #134242, where Arc's documentation lacks Global Allocator safety descriptions for three APIs. And this was confirmed by @workingjubilee :

Wait, nevermind. I apparently forgot the increment_strong_count is implicitly A = Global. Ugh. Another reason these things are hard to track, unfortunately.

PR Description

This PR updates the document for the following APIs:

• Arc::from_raw
• Arc::increment_strong_count
• Arc::decrement_strong_count

These APIs currently lack an important piece of documentation: the raw pointer must point to a block of memory allocated by the global allocator. This crucial detail is specified in the source code but is not reflected in the documentation, which could lead to confusion or incorrect usage by users.

Problem:

The following example demonstrates the potential confusion caused by the lack of documentation:

#![feature(allocator_api)]
use std::alloc::{Allocator,AllocError, Layout};
use std::ptr::NonNull;
use std::sync::Arc;

struct LocalAllocator {
    memory: NonNull<u8>,
    size: usize,
}

impl LocalAllocator {
    fn new(size: usize) -> Self {
        Self {
            memory: unsafe { NonNull::new_unchecked(&mut 0u8 as *mut u8) },
            size,
        }
    }
}

unsafe impl Allocator for LocalAllocator {
    fn allocate(&self, _layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
        Ok(NonNull::slice_from_raw_parts(self.memory, self.size))
    }

    unsafe fn deallocate(&self, _ptr: NonNull<u8>, _layout: Layout) {
    }
}

fn main() {
    let allocator = LocalAllocator::new(64);
    let arc = Arc::new_in(5, &allocator); // Here, allocator could be any non-global allocator
    let ptr = Arc::into_raw(arc);

    unsafe {
        Arc::increment_strong_count(ptr);
        let arc = Arc::from_raw(ptr);
        assert_eq!(2, Arc::strong_count(&arc)); // Failed here!
    }
}

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ibraheemdev (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[ibraheemdev]

I'm not sure I understand your example. Creating an Arc from any pointer other than a pointer returned to into_raw is unsound anyways, so your example is unsound in the first call to from_raw. The issue here would be using Arc::new_in and then using Arc::from_raw instead of from_raw_in, or similar.

The change itself looks good to me, but could we link to the appropriate *_in method as well?

[DiuDiu777]

@ibraheemdev
Well, the essence of this example is to highlight that the pointers used by from_raw and increment_strong_count can originate from non-Global allocators (a safety concern omitted in the documentation).

You're absolutely right—from_raw is already unsound in this case. But if the pointer originates from new_in with a non-Global allocator (as shown in my updated example), it results in undefined behavior (UB).

The change itself looks good to me, but could we link to the appropriate *_in method as well?

Regarding this issue, I think it would be helpful to explicitly mention Global in the changed parts. While the *_in methods specify that allocations need to use A, users who don't check the from_raw's source code might not realize that it implicitly uses A=Global.

Thanks for taking the time to review this. I hope this clarifies the issue and provides a reasonable update to the std doc.

[ibraheemdev]

r=me after the one typo.

[ibraheemdev]

Thanks! @bors r+

[bors]

📌 Commit 48e671e has been approved by ibraheemdev

It is now in the queue for this repository.