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