On Fri, Feb 25, 2022 at 09:10:44AM -0500, Michael S. Tsirkin wrote: > QOM reference counting is not designed with an infinite amount of > references in mind, trying to take a reference in a loop will overflow > the integer. We will then eventually assert when dereferencing, but the > real problem is in object_ref so let's assert there to make such issues > cleaner and easier to debug.
What is the actual bug / scenario that led you to hit this problem ? I'm surprised you saw an assert in object_unref, as that would imply you had exactly UINT32_MAX calls to object_ref and then one to object_unref. > Some micro-benchmarking shows using fetch and add this is essentially > free on x86. > > Signed-off-by: Michael S. Tsirkin <[email protected]> > --- > qom/object.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/qom/object.c b/qom/object.c > index 4f0677cca9..5db3974f04 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -1167,10 +1167,14 @@ GSList *object_class_get_list_sorted(const char > *implements_type, > Object *object_ref(void *objptr) > { > Object *obj = OBJECT(objptr); > + uint32_t ref; > + > if (!obj) { > return NULL; > } > - qatomic_inc(&obj->ref); > + ref = qatomic_fetch_inc(&obj->ref); > + /* Assert waaay before the integer overflows */ > + g_assert(ref < INT_MAX); Not that I expect this to hit, but why choose this lower bound instead of g_assert(ref > 0) which is the actual failure scenario, matching the existing object_unref assert. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
