Re: [PATCH v9 2/2] rust: xarray: Add an abstraction for XArray

[Boqun Feng <boqun.feng@gmail.com>]

On Mon, Nov 18, 2024 at 11:33:36AM -0500, Tamir Duberstein wrote:
[...]
> +
> +/// A lock guard.
> +///
> +/// The lock is unlocked when the guard goes out of scope.
> +#[must_use = "the lock unlocks immediately when the guard is unused"]
> +pub struct Guard<'a, T: ForeignOwnable> {
> +    xa: &'a XArray<T>,
> +}

`Guard` would be `Send` if `XArray<T>` is `Sync`, however, it's
incorrect since `Guard` represents an xa_lock() held, and that's a
spin_lock, so cannot be dropped on another thread/context. `Guard`
should probably be marked as `!Send`. Or am I missing something subtle
here?

Regards,
Boqun

> +
> +impl<T: ForeignOwnable> Drop for Guard<'_, T> {
> +    fn drop(&mut self) {
> +        // SAFETY: `self.xa.xa` is always valid by the type invariant.
> +        //
> +        // SAFETY: The caller holds the lock, so it is safe to unlock it.
> +        unsafe { bindings::xa_unlock(self.xa.xa.get()) };
> +    }
> +}
> +
[...]
> +// SAFETY: It is safe to send `XArray<T>` to another thread when the underlying `T` is `Send`
> +// because XArray is thread-safe and all mutation operations are synchronized.
> +unsafe impl<T: ForeignOwnable + Send> Send for XArray<T> {}
> +
> +// SAFETY: It is safe to send `&XArray<T>` to another thread when the underlying `T` is `Sync`
> +// because it effectively means sharing `&T` (which is safe because `T` is `Sync`). Additionally,
> +// `T` is `Send` because XArray is thread-safe and all mutation operations are internally locked.
> +unsafe impl<T: ForeignOwnable + Send + Sync> Sync for XArray<T> {}
> 
> -- 
> 2.47.0
> 
>