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 <pbonz...@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....@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
