rust-lang / rust-lang/rust

Arc::from_raw shouldn't require the pointer came from into_raw specifically

Open
#150,299 7 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

C-discussion T-libs T-opsem
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

Currently Arc::from_raw requires specifically that the pointer it accepts must have come from Arc::into_raw for the appropriate type. I think this is unnecessarily restrictive—any of the following should be allowed as well, where U has an appropriate type in the same way as specified today:

  • The result of Arc<U>::as_ptr.
  • The value &*some_arc as *const T where some_arc is Arc<U> and the caller has arranged for the refcounts to work out, for example using Arc::increment_strong_count or by never dropping the result.
  • (If you want to treat it as distinct from the one above, then also any &U value where the reference ultimately came from an Arc<U>. For example, self in a method on U called with an Arc parameter.)

As far as I can tell reading the implementation, this would be a no-op change in reality, since Arc::into_raw is alrady implemented using exactly equivalent code to what I'm requesting. So here I'm only asking for the documented safety requirements to be relaxed to match.


The motivation for requesting this change is to avoid paying for unnecessary refcounting operations in unsafe code that manipulates raw pointers to arc-managed objects. If Arc::into_raw is the only way to round trip a raw pointer and we only have around an &Arc, then round tripping a raw pointer requires incrementing and decrementing the refcount where the user may otherwise be able to prove that this isn't needed for lifetime safety.

For example, here is a sketch of the motivating case where this came up in a real application:

/// A struct that contains the state needed to implement a waker.
struct Foo {}

/// Create a waker that uses the supplied Foo struct without needing to clone
/// anything if it's used immediately.
///
/// # Safety
///
/// The waker must not be dropped, or consumed by `wake`.
///
/// Methods on the waker must only be called while Foo is still known to exist
/// and be managed by Arc.
///
/// In both cases it's fine to do anything with clones of the waker. These
/// restrictions provide only to the specific waker returned by this function.
unsafe fn make_waker(foo: &Arc<Foo>) -> ManuallyDrop<Waker> {
   // Here we would set up a waker vtable whose data pointer is a raw pointer
   // to *foo, with a wake_by_ref function that doesn't need to clone anything
   // and with a clone function that uses Arc::increment_strong (or
   // Arc::from_raw plus Arc::clone without dropping the first's result).
   todo!();
}

/// Poll a future once with a waker that uses the state in a Foo. Don't pay the
/// cost of cloning the Foo just to create the waker, which Future::poll accepts
/// by reference, not by value.
fn poll_with_foo<F>(future: Pin<&mut F>, foo: &Arc<Foo>) -> Poll<F::Output> where F: Future{
    // Safety:
    //
    // *   We only hand over the waker by reference, so the callee can't drop or
    //     consume it, and we never do so ourselves.
    //
    // *   The presence of the `foo` parameter ensures the Foo exists and is
    //     managed by Arc for as long as the waker might be used.
    //
    let waker = unsafe { make_waker(foo) };

    future.poll(&mut Context::from_waker(&waker))
}

(Playground)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with Arc::from_raw, Arc::as_ptr, and Arc::increment_strong_count in library/alloc/src/sync.rs, including the implementation linked in the issue. Review the current safety documentation and related raw-pointer behavior, then determine whether the proposed requirements can be relaxed without changing soundness. Done means the documented contract and any necessary tests consistently cover the allowed pointer origins.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.