Re: [PATCH 1/2] qom: Reject temporary object resurrection

[Kevin Wolf <kwolf@redhat.com>]

Am 15.06.2026 um 06:11 hat Akihiko Odaki geschrieben:
> If object_ref() is called during finalization, it will temporarily
> "resurrect" the object. Although object_finalize() asserts that no
> resurrecting reference remains before freeing the object, the assertion
> cannot catch the case where the resurrecting reference is dropped before
> freeing the object.

Ok, and why is that a problem?

In the cover letter, you referred to an earlier patch you had posted:

    The temporary object resurrection patch originated from:
    https://lore.kernel.org/qemu-devel/20250906-mr-v2-1-2820f5a3d282@rsg.ci.i.u-tokyo.ac.jp/
    ("[PATCH v2 1/3] qom: Do not finalize twice")

From that patch, it's clear that one problem is that .finalize() can be
called multiple times, which you probably don't want. But what is the
reason for switching from just ignoring the second .finalize() like in
the old patch to completely forbidding a temporary refcount increase?

> One way to catch this would be to add a check in object_ref() to reject
> resurrecting references before they are created. However, object_ref()
> is frequently called so it is better to minimize the overhead.

We're talking about a single comparison with zero here. So if that's the
worse alternative, the other one can't have any drawbacks.

> To avoid adding the overhead, change how the reference count is
> represented and let an existing assertion detect resurrection. More
> concretely, obj->ref now stores the current reference count minus 1. The
> stored count is therefore 0 after object_initialize(), and the final
> object_unref() decrements it from 0 to UINT32_MAX and starts
> finalization. A resurrecting object_ref() will then trip the existing
> obj->ref < INT_MAX assertion.

A refcount field where 0 means that a last reference is still remaining
is highly confusing. If we do want to forbid taking a temporary new
reference in .finalize() for some reason, the explicit check in
object_ref() sounds much better to me.

> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>  include/qom/object.h            |  4 ++--
>  block/throttle-groups.c         |  2 +-
>  qom/object.c                    | 15 ++++++---------
>  tests/unit/check-qom-proplist.c |  8 ++++----
>  4 files changed, 13 insertions(+), 16 deletions(-)

Kevin