Re: [PATCH 13/26] rust: qom: automatically use Drop trait to implement instance_finalize

[Zhao Liu <zhao1.liu@intel.com>]

On Wed, Dec 11, 2024 at 01:42:32PM +0100, Paolo Bonzini wrote:
> Date: Wed, 11 Dec 2024 13:42:32 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 13/26] rust: qom: automatically use Drop trait to
>  implement instance_finalize
> 
> On Tue, Dec 10, 2024 at 4:58 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> > Great idea. It nicely balances the differences between Rust and C QOM
> > conventions.
> 
> Except it does not work. :(  Suppose you have
> 
>        pub struct MySuperclass {
>            parent: DeviceState,
>            field: Box<MyData>,
>            ...
>        }
> 
>        impl Drop for MySuperclass {
>            ...
>        }
> 
>        pub struct MySubclass {
>            parent: MySuperclass,
>            ...
>        }
> 
> When instance_finalize is called for MySubclass, it will walk the
> struct's list of fields and call the drop method for MySuperclass.
> Then, object_deinit recurses to the superclass and calls the same drop
> method again.  This will cause double-freeing of the Box<MyData>, or
> more in general double-dropping.

Good catch! Yes, there is indeed such an issue... The above example
could become a test case :-), which I supposed could trigger a double-
dropping error when compiling.

> What's happening here is that QOM wants to control the drop order of
> MySuperclass and MySubclass's fields.  To do so, the parent field must
> be marked ManuallyDrop<>, which is quite ugly.  Perhaps we can add a
> wrapper type ParentField<> that is specific to QOM.  This hides the
> implementation detail of *what* is special about the ParentField, and
> it will also be easy to check for in the #[derive(Object)] macro.
> Maybe in the future it will even make sense to have special functions
> implemented on ParentField, I don't know...

I also looked into the implementation of ManuallyDrop, and I agree with
a new ParentField (or simply ObjectParent). This wrapper is simple
enough but also useful for QOM. I will pay more attention to the
recursed relationships in QOM in review as well...

Thanks,
Zhao