Re: [PATCH v3 2/4] rust: convert `Arc` to use `Refcount`

[Gary Guo <gary@garyguo.net>]

On Fri, 21 Feb 2025 12:27:46 -0500
Tamir Duberstein <tamird@gmail.com> wrote:

> > > > @@ -412,16 +402,14 @@ fn clone(&self) -> Self {
> > > >
> > > >  impl<T: ?Sized> Drop for Arc<T> {
> > > >      fn drop(&mut self) {
> > > > -        // SAFETY: By the type invariant, there is necessarily a reference to the object. We cannot
> > > > -        // touch `refcount` after it's decremented to a non-zero value because another thread/CPU
> > > > -        // may concurrently decrement it to zero and free it. It is ok to have a raw pointer to
> > > > -        // freed/invalid memory as long as it is never dereferenced.
> > > > -        let refcount = unsafe { self.ptr.as_ref() }.refcount.get();
> > > > -
> > > >          // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and
> > > >          // this instance is being dropped, so the broken invariant is not observable.
> > > > -        // SAFETY: Also by the type invariant, we are allowed to decrement the refcount.
> > > > -        let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) };
> > > > +        // SAFETY: By the type invariant, there is necessarily a reference to the object.
> > > > +        // NOTE: we cannot touch `refcount` after it's decremented to a non-zero value because
> > > > +        // another thread/CPU may concurrently decrement it to zero and free it. However it is okay
> > > > +        // to have a transient reference to decrement the refcount, see
> > > > +        // https://github.com/rust-lang/rust/issues/55005.
> > > > +        let is_zero = unsafe { self.ptr.as_ref().refcount.dec_and_test() };  
> > >
> > > How come this careful handling is not required in into_unique_or_drop?
> > > At least, the SAFETY comment there is much more mundane.  
> >
> > Because `into_unique_or_drop` doesn't actually remove the allocation
> > (it only decrements refcount for non-zero or turn it into `UniqueArc`).  
> 
> I don't follow. This comment here talks about a race with another CPU
> decrementing to zero; isn't the same race possible between any
> combination of `into_unique_or_drop` and `drop` callers?

Oh you're right. It's indeed the same situation. However I'd want to
avoid repeating justification multiple times, given that this is really
an explaination note to the reader. Do you have any suggestion on how
to organise the comment so I can avoid repeating this multiple times?

Best,
Gary