Re: [PATCH v2 1/3] qom: Do not finalize twice

[Paolo Bonzini <pbonzini@redhat.com>]

On 9/6/25 04:39, Akihiko Odaki wrote:
> The next change adds code to retain references from an object to the
> parent when it is being unparented to ensure that the parent outlive
> them. This change handles the following scenario with the code:
> 
> 1. The parent starts being finalized without unparenting.
> 2. Unparenting happens during finalization.
> 3. The child retains the reference to the parent.
> 4. The child gets finalized, and releases the reference.
> 
> In this scenario, the reference counter of the parent reaches to zero,
> gets incremented, and gets decremented to reach to zero again. This
> change ensures that finalization will be triggered again in the
> scenario.
> 
> Note that the reference counter needs to reach to zero again before
> finalization ends; otherwise the object will be "resurrected", which
> is not clearly defined and prohibited with an existing assertion.
> 
> One thing that looks concerning with this change is that it adds a bool
> to Object. This is not a problem in the most situations where the host
> uses 64-bit addressing because the member is added to a gap needed for
> alignment, and possible double-free scenarios handled with this change
> are more serious than the extra memory usage for 32-bit hosts.

If this is a problem, we could reserve a special value of ->ref for that 
(such as bit 31) but I think this is okay as long as there is this 
32-bit hole.

Paolo

> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   include/qom/object.h | 1 +
>   qom/object.c         | 5 +++++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 26df6137b911..7f7b1ffea8fe 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -158,6 +158,7 @@ struct Object
>       ObjectFree *free;
>       GHashTable *properties;
>       uint32_t ref;
> +    bool finalizing;
>       Object *parent;
>   };
>   
> diff --git a/qom/object.c b/qom/object.c
> index 1856bb36c74c..b766b2e9baa7 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -725,6 +725,11 @@ static void object_finalize(void *data)
>       Object *obj = data;
>       TypeImpl *ti = obj->class->type;
>   
> +    if (obj->finalizing) {
> +        return;
> +    }
> +
> +    obj->finalizing = true;
>       object_property_del_all(obj);
>       object_deinit(obj, ti);
>   
>