Messages in this thread | | | Date | Mon, 25 Sep 2023 14:55:30 -0700 | From | Boqun Feng <> | Subject | Re: [PATCH v2 2/2] rust: arc: remove `ArcBorrow` in favour of `WithRef` |
| |
On Mon, Sep 25, 2023 at 09:03:52PM +0000, Benno Lossin wrote: > On 25.09.23 20:51, Boqun Feng wrote: > > On Mon, Sep 25, 2023 at 05:00:45PM +0000, Benno Lossin wrote: > >> On 25.09.23 18:16, Boqun Feng wrote: > >>> On Mon, Sep 25, 2023 at 03:07:44PM +0000, Benno Lossin wrote: > >>>> ```rust > >>>> struct MutatingDrop { > >>>> value: i32, > >>>> } > >>>> > >>>> impl Drop for MutatingDrop { > >>>> fn drop(&mut self) { > >>>> self.value = 0; > >>>> } > >>>> } > >>>> > >>>> let arc = Arc::new(MutatingDrop { value: 42 }); > >>>> let wr = arc.as_with_ref(); // this creates a shared `&` reference to the MutatingDrop > >>>> let arc2: Arc<MutatingDrop> = wr.into(); // increments the reference count to 2 > >>> > >>> More precisely, here we did a > >>> > >>> &WithRef<_> -> NonNull<WithRef<_>> > >>> > >>> conversion, and later on, we may use the `NonNull<WithRef<_>>` in > >>> `drop` to get a `Box<WithRef<_>>`. > >> > >> Indeed. > >> > > > > Can we workaround this issue by (ab)using the `UnsafeCell` inside > > `WithRef<T>`? > > > > impl<T: ?Sized> From<&WithRef<T>> for Arc<T> { > > fn from(b: &WithRef<T>) -> Self { > > // SAFETY: The existence of the references proves that > > // `b.refcount.get()` is a valid pointer to `WithRef<T>`. > > let ptr = unsafe { NonNull::new_unchecked(b.refcount.get().cast::<WithRef<T>>()) }; > > > > // SAFETY: see the SAFETY above `let ptr = ..` line. > > ManuallyDrop::new(unsafe { Arc::from_inner(ptr) }) > > .deref() > > .clone() > > } > > } > > > > This way, the raw pointer in the new Arc no longer derives from the > > reference of `WithRef<T>`. > > No, the code above only obtains a pointer that has provenance valid > for a `bindings::refcount_t` (or type with the same layout, such as > `Opaque<bindings::refcount_t>`). But not the whole `WithRef<T>`, so accessing > it by reading/writing will still be UB. >
Hmm... but we do the similar thing in `Arc::from_raw()`, right?
pub unsafe fn from_raw(ptr: *const T) -> Self { .. }
, what we have is a pointer to T, and we construct a pointer to `ArcInner<T>/WithRef<T>`, in that function. Because the `sub` on pointer gets away from provenance? If so, we can also do a sub(0) in the above code.
Regards, Boqun
> -- > Cheers, > Benno > >
| |