Repository navigation
Documentation of Arc::from_raw is unnecessarily restrictive #106124
Description
Activity
Isn't this code pattern you want documented as valid unnecessarily dangerous, like
Vec::from_raw_partswith a pointer fromVec::as_ptr?Why do you want to pass around an
Arccreated like this instead of a reference or a raw pointer?@rustbot claim
Beware of the footgun: To call
Arc::into_raw, you consume one reference count, asinto_rawconsumes theArc; this is the symmetry withfrom_raw. On the other hand,as_rawtakes an&Arc, so its possible to call this many times wrt the same reference count; if you then callfrom_rawon those pointers, the reference count will underflow when those newly createdArcs get dropped.
Obviously we are inunsafe-land anyway, but the docs should point this out explicitly, asas_raw+from_rawis much easier to get wrong thaninto_raw/from_rawReacted by Élie ROUDNINSKIIsn't this code pattern you want documented as valid unnecessarily dangerous, like
Vec::from_raw_partswith a pointer fromVec::as_ptr?Why do you want to pass around an
Arccreated like this instead of a reference or a raw pointer?I was able to find a different solution using
into_rawandfrom_raw, but i still think mentioning the possibility of usingArc::as_rawshould be mentioned.Beware of the footgun: To call
Arc::into_raw, you consume one reference count, asinto_rawconsumes theArc; this is the symmetry withfrom_raw. On the other hand,as_rawtakes an&Arc, so its possible to call this many times wrt the same reference count; if you then callfrom_rawon those pointers, the reference count will underflow when those newly createdArcs get dropped. Obviously we are inunsafe-land anyway, but the docs should point this out explicitly, asas_raw+from_rawis much easier to get wrong thaninto_raw/from_rawYea, you are absolutely right with that but by passing around ManuallyDrop<Arc> this can be largely avoided (except if one manually drops the
Arc)Why would you need to do
ManuallyDrop::new(from_raw(as_ptr))instead offrow_raw(into_raw(clone))?I can't imagine avoiding a single atomic increment brings measurable benefit.
Why would you need to do
ManuallyDrop::new(from_raw(as_ptr))instead offrow_raw(into_raw(clone))?I can't imagine avoiding a single atomic increment brings measurable benefit.
If you want to you can perform measurements, this is the repo: https://github.com/terrarier2111/SwapArc
(it's essentially a data structure that presents a faster alternative to ArcSwap)
but yea generally speaking on the fast path i only have a single Relaxed load and even on the slow paths i try to use as few cmp_exchg and fetch_xxx operations as possible because these are pretty much the most expensive atomic operations i am using, even without contention (at least on x86_64), also every atomic modification of the counter invalidates its cache line which can also be very punishing. But i am not alone with this concern, the author of ArcSwap also noticed the impact of Arc cloning on performance in his implementation.I can't because that would mean returning a type that's not just
T(inArc<T>) which is a requirement for my API (as it is generic and allows to use more than just Arc)Couldn't you put an associated type on
DataPtrConvert? For instance:impl<T: Send + Sync> DataPtrConvert<T> for Arc<T> { type Weak = sync::sync::Weak<T>; fn as_weak(&self) -> Self::Weak { todo!() } }
Couldn't you put an associated type on
DataPtrConvert? For instance:impl<T: Send + Sync> DataPtrConvert<T> for Arc<T> { type Weak = sync::sync::Weak<T>; fn as_weak(&self) -> Self::Weak { todo!() } }
Uhm maybe that could work, i am not really experienced in working with associated types.
Does Weak introduce any overhead for example branches or the like to check whether the "weak link" is still valid, or doesn't it check that and it's free?I also having the exact use case of this. I'm in the process of migrating the C++ application to Rust and what I did is building a Rust crate as a static library and do something like this:
#[no_mangle] pub unsafe extern "C" fn run( argc: c_int, argv: *mut *mut c_char, cpp: unsafe extern "C" fn(*const App, c_int, *mut *mut c_char) -> c_int ) -> c_int { let app = Arc::new(App {}); let code = cpp(Arc::as_ptr(&app), argc, argv); // Graceful shutdown code here. code }
The
runfunction will be called immediately when the execution landed at the C++ entry point. When the control is transferred back to C++ side it will have a pointer toAppthat returned fromArc::as_ptr(&app), which can be passed back to some Rust functions. The problem is some Rust functions required to spawn a background task that required to clone theAppobject into it, which currently cannot be done in an elegance way.Seems like what I actually need is something like
clone_from_rawinstead.Closing this as this isn't bug or there's anything to be done here
I think what needs to be done is update
from_rawdocumentation to allow a pointer fromas_ptr.
Location
Arc::from_raw
Summary
Arc::from_rawmentions that the provided pointer has to have been returned fromArc::into_rawbut it doesn't mentionArc::as_ptrat all. This disallows many usages that depend on not modifying the reference counter while passingArcaround in form of a pointer and converting it back by callingArc::from_rawon the pointer and wrapping the result inside aManuallyDrop(as after one conversion round ofArc::into_raw->ManuallyDrop<Arc::from_raw>->Arc::as_ptrwould disallow recreating theArcthroughArc::from_raw)