On Mon, Jun 15, 2026 at 01:11:06PM +0900, Akihiko Odaki wrote: > 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.
What's the real world problem here ? > > 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. > > 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. Please no, this is horribly surprising semantics. > > Signed-off-by: Akihiko Odaki <[email protected]> > --- > 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(-) > > diff --git a/include/qom/object.h b/include/qom/object.h > index 11f55613fcd0..c4dcf65dcf7f 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -1121,7 +1121,7 @@ GSList *object_class_get_list_sorted(const char > *implements_type, > * @obj: the object > * > * Increase the reference count of a object. A object cannot be freed as > long > - * as its reference count is greater than zero. > + * as a reference remains. > * Returns: @obj > */ > Object *object_ref(void *obj); > @@ -1131,7 +1131,7 @@ Object *object_ref(void *obj); > * @obj: the object > * > * Decrease the reference count of a object. A object cannot be freed as > long > - * as its reference count is greater than zero. > + * as a reference remains. > */ > void object_unref(void *obj); > > diff --git a/block/throttle-groups.c b/block/throttle-groups.c > index 4b1b1944c20a..3b2a89f5849d 100644 > --- a/block/throttle-groups.c > +++ b/block/throttle-groups.c > @@ -962,7 +962,7 @@ static void throttle_group_get_limits(Object *obj, > Visitor *v, > > static bool throttle_group_can_be_deleted(UserCreatable *uc) > { > - return OBJECT(uc)->ref == 1; > + return OBJECT(uc)->ref == 0; > } > > static void throttle_group_obj_class_init(ObjectClass *klass, > diff --git a/qom/object.c b/qom/object.c > index 0ac201de4c15..761eff1337e6 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -497,7 +497,6 @@ static void object_initialize_with_type(Object *obj, > size_t size, TypeImpl *type > > memset(obj, 0, type->instance_size); > obj->class = type->class; > - object_ref(obj); > object_class_property_init_all(obj); > obj->properties = g_hash_table_new_full(g_str_hash, g_str_equal, > NULL, object_property_free); > @@ -560,11 +559,10 @@ bool object_initialize_child_with_propsv(Object > *parentobj, > > out: > /* > - * We want @obj's reference to be 1 on success, 0 on failure. > - * On success, it's 2: one taken by object_initialize(), and one > - * by object_property_add_child(). > - * On failure in object_initialize() or earlier, it's 1. > - * On failure afterwards, it's also 1: object_unparent() releases > + * We want @obj's reference to be 0 on success, UINT32_MAX on failure. > + * On success, it's 1: one taken by object_property_add_child(). > + * On failure in object_initialize() or earlier, it's 0. > + * On failure afterwards, it's also 0: object_unparent() releases > * the reference taken by object_property_add_child(). > */ > object_unref(obj); > @@ -668,7 +666,6 @@ static void object_finalize(void *data) > object_property_del_all(obj); > object_deinit(obj, ti); > > - g_assert(obj->ref == 0); > g_assert(obj->parent == NULL); > if (obj->free) { > obj->free(obj); > @@ -1329,10 +1326,10 @@ void object_unref(void *objptr) > if (!obj) { > return; > } > - g_assert(obj->ref > 0); > + g_assert(obj->ref < INT_MAX); > > /* parent always holds a reference to its children */ > - if (qatomic_fetch_dec(&obj->ref) == 1) { > + if (qatomic_fetch_dec(&obj->ref) == 0) { > object_finalize(obj); > } > } > diff --git a/tests/unit/check-qom-proplist.c b/tests/unit/check-qom-proplist.c > index 89de92b7d91f..b5527dd52afd 100644 > --- a/tests/unit/check-qom-proplist.c > +++ b/tests/unit/check-qom-proplist.c > @@ -351,7 +351,7 @@ static void test_dummy_createv_tree(void) > NULL)); > > g_assert(err == NULL); > - g_assert_cmpint(dobj->parent_obj.ref, ==, 1); > + g_assert_cmpint(dobj->parent_obj.ref, ==, 0); > g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss"); > g_assert(dobj->bv == true); > g_assert(dobj->av == DUMMY_PLATYPUS); > @@ -375,7 +375,7 @@ static void test_dummy_createv_parentless(void) > NULL)); > > g_assert(err == NULL); > - g_assert_cmpint(dobj->parent_obj.ref, ==, 1); > + g_assert_cmpint(dobj->parent_obj.ref, ==, 0); > g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss"); > g_assert(dobj->bv == true); > g_assert(dobj->av == DUMMY_PLATYPUS); > @@ -414,7 +414,7 @@ static void test_dummy_createlist_tree(void) > NULL)); > > g_assert(err == NULL); > - g_assert_cmpint(dobj->parent_obj.ref, ==, 1); > + g_assert_cmpint(dobj->parent_obj.ref, ==, 0); > g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss"); > g_assert(dobj->bv == true); > g_assert(dobj->av == DUMMY_PLATYPUS); > @@ -450,7 +450,7 @@ static void test_dummy_createlist_parentless(void) > NULL)); > > g_assert(err == NULL); > - g_assert_cmpint(dobj->parent_obj.ref, ==, 1); > + g_assert_cmpint(dobj->parent_obj.ref, ==, 0); > g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss"); > g_assert(dobj->bv == true); > g_assert(dobj->av == DUMMY_PLATYPUS); > > -- > 2.54.0 > With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
