Re: [PATCH v15 1/9] rust: types: Add Ownable/Owned types

[Andreas Hindborg <a.hindborg@kernel.org>]

Alice Ryhl <aliceryhl@google.com> writes:

> On Fri, Feb 20, 2026 at 10:51:10AM +0100, Andreas Hindborg wrote:
>> From: Asahi Lina <lina+kernel@asahilina.net>
>> 
>> By analogy to `AlwaysRefCounted` and `ARef`, an `Ownable` type is a
>> (typically C FFI) type that *may* be owned by Rust, but need not be. Unlike
>> `AlwaysRefCounted`, this mechanism expects the reference to be unique
>> within Rust, and does not allow cloning.
>> 
>> Conceptually, this is similar to a `KBox<T>`, except that it delegates
>> resource management to the `T` instead of using a generic allocator.
>> 
>> [ om:
>>   - Split code into separate file and `pub use` it from types.rs.
>>   - Make from_raw() and into_raw() public.
>>   - Remove OwnableMut, and make DerefMut dependent on Unpin instead.
>>   - Usage example/doctest for Ownable/Owned.
>>   - Fixes to documentation and commit message.
>> ]
>> 
>> Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
>> Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
>> Co-developed-by: Oliver Mangold <oliver.mangold@pm.me>
>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>> [ Andreas: Updated documentation, examples, and formatting ]
>> Reviewed-by: Gary Guo <gary@garyguo.net>
>> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
>> +///         let result = NonNull::new(KBox::into_raw(result))
>> +///             .expect("Raw pointer to newly allocation KBox is null, this should never happen.");
>
> KBox should probably have an into_raw_nonnull().

I can add that.

>
>> +///    let foo = Foo::new().expect("Failed to allocate a Foo. This shouldn't happen");
>> +///    assert!(*FOO_ALLOC_COUNT.lock() == 1);
>
> Use ? here.

Ok.

>
>> +/// }
>> +/// // `foo` is out of scope now, so we expect no live allocations.
>> +/// assert!(*FOO_ALLOC_COUNT.lock() == 0);
>> +/// ```
>> +pub unsafe trait Ownable {
>> +    /// Releases the object.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// Callers must ensure that:
>> +    /// - `this` points to a valid `Self`.
>> +    /// - `*this` is no longer used after this call.
>> +    unsafe fn release(this: NonNull<Self>);
>
> Honestly, not using it after this call may be too strong. I can imagine
> wanting a value where I have both an ARef<_> and Owned<_> reference to
> something similar to the existing Arc<_>/ListArc<_> pattern, and in that
> case the value may in fact be accessed after this call if you still have
> an ARef<_>.

I do not understand your use case.

You are not supposed to have both an `ARef` and an `Owned` at the same
time. The `Owned` is to `ARef` what `UniqueArc` is to `Arc`. It is
supposed to be unique and no `ARef` can be live while the `Owned` is
live.

A `ListArc` is "at most one per list link" and it takes a refcount on
the object by owning an `Arc`. As far as I recall, it does not provide
mutable access to anything but the list link. To me, that is a very
different situation.

>
> If you modify Owned<_> invariants and Owned::from_raw() safety
> requirements along the lines of what I say below, then this could just
> say that the caller must have permission to call this function. The
> concrete implementer can specify what that means more directly, but here
> all it means is that a prior call to Owned::from_raw() promised to give
> you permission to call it.

I don't think we need the "permission" wording. How about this:


/// A mutable reference to an owned `T`.
///
/// The [`Ownable`] is automatically freed or released when an instance of [`Owned`] is
/// dropped.
///
/// # Invariants
///
/// - Until `T::release` is called, this `Owned<T>` exclusively owns the underlying `T`.
/// - The `T` value is pinned.
pub struct Owned<T: Ownable> {...}


impl<T: Ownable> Owned<T> {
    /// Creates a new instance of [`Owned`].
    ///
    /// This function takes over ownership of the underlying object.
    ///
    /// # Safety
    ///
    /// Callers must ensure that:
    /// - `ptr` points to a valid instance of `T`.
    /// - Until `T::release` is called, the returned `Owned<T>` exclusively owns the underlying `T`.
    pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {...}
}

pub trait Ownable {
    /// Tear down this `Ownable`.
    ///
    /// Implementers of `Ownable` can use this function to clean up the use of `Self`. This can
    /// include freeing the underlying object.
    ///
    /// # Safety
    ///
    /// Callers must ensure that the caller has exclusive ownership of `T`, and this ownership can
    /// be transferred to the `release` method.
    unsafe fn release(&mut self);
}


Note `Ownable` not being an unsafe trait.

>
>> +/// A mutable reference to an owned `T`.
>> +///
>> +/// The [`Ownable`] is automatically freed or released when an instance of [`Owned`] is
>> +/// dropped.
>> +///
>> +/// # Invariants
>> +///
>> +/// - The [`Owned<T>`] has exclusive access to the instance of `T`.
>> +/// - The instance of `T` will stay alive at least as long as the [`Owned<T>`] is alive.
>> +pub struct Owned<T: Ownable> {
>> +    ptr: NonNull<T>,
>> +}
>
> I think some more direct and less fuzzy invariants would be:
>
> - This `Owned<T>` holds permissions to call `T::release()` on the value once.
> - Until `T::release()` is called, this `Owned<T>` may perform mutable access on the `T`.

I do not like the wording for mutable access. Formulating safety
requirements for `from_raw` and safety comments for that function
becomes convoluted like this. I'd rather formulate the
access capability in terms of ownership;

 - Until `T::release()` is called, this `Owned<T>` exclusively owns the
   underlying `T`.

How is that?

> - The `T` value is pinned.

I am unsure about the pinning terminology. If we say that `T` is pinned,
does this mean that it will never move, even if `T: Unpin`? Or is it
implied that `T` may move if it is `Unpin`?

>
>> +    /// Get a pinned mutable reference to the data owned by this `Owned<T>`.
>> +    pub fn as_pin_mut(&mut self) -> Pin<&mut T> {
>> +        // SAFETY: The type invariants guarantee that the object is valid, and that we can safely
>> +        // return a mutable reference to it.
>> +        let unpinned = unsafe { self.ptr.as_mut() };
>> +
>> +        // SAFETY: We never hand out unpinned mutable references to the data in
>> +        // `Self`, unless the contained type is `Unpin`.
>> +        unsafe { Pin::new_unchecked(unpinned) }
>
> I'd prefer if "pinned" was a type invariant, rather than make an
> argument about what kind of APIs exist.

Ok.

>
>> +impl<T: Ownable + Unpin> DerefMut for Owned<T> {
>> +    fn deref_mut(&mut self) -> &mut Self::Target {
>> +        // SAFETY: The type invariants guarantee that the object is valid, and that we can safely
>> +        // return a mutable reference to it.
>> +        unsafe { self.ptr.as_mut() }
>
> Surely this safety comment should say something about pinning.

Yes.


Best regards,
Andreas Hindborg