Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors

[Eduardo Habkost]

On Wed, May 20, 2015 at 06:11:33PM +0200, Andreas Färber wrote:
> Am 20.05.2015 um 18:06 schrieb Eduardo Habkost:
> > On Wed, May 20, 2015 at 04:18:03PM +0100, Daniel P. Berrange wrote:
> >> On Wed, May 20, 2015 at 11:44:19AM -0300, Eduardo Habkost wrote:
> >>> On Tue, May 19, 2015 at 06:11:05PM +0200, Paolo Bonzini wrote:
> >>>> On 19/05/2015 17:55, Daniel P. Berrange wrote:
> >>>>> Paolo told me on previous posting that object_property_add_child()
> >>>>> holds a reference on 'obj' for as long as it is registered in the
> >>>>> object hierarchy composition. So it sufficient to rely on that long
> >>>>> term reference, and let the caller dispose of the object by calling
> >>>>> object_unparent(obj) when finally done.
> >>>>
> >>>> For an example of the same pattern:
> >>>>
> >>>> DeviceState *qdev_try_create(BusState *bus, const char *type)
> >>>> {
> >>>>     DeviceState *dev;
> >>>>
> >>>>     if (object_class_by_name(type) == NULL) {
> >>>>         return NULL;
> >>>>     }
> >>>>     dev = DEVICE(object_new(type));
> >>>>     if (!dev) {
> >>>>         return NULL;
> >>>>     }
> >>>>
> >>>>     if (!bus) {
> >>>>         bus = sysbus_get_default();
> >>>>     }
> >>>>
> >>>>     qdev_set_parent_bus(dev, bus);
> >>>>     object_unref(OBJECT(dev));
> >>>>     return dev;
> >>>> }
> >>>>
> >>>> Effectively this is idea as GObject's "floating reference".
> >>>> qdev_set_parent_bus (in qdev_try_create) and object_property_add_child
> >>>> (in Daniel's patches) "sink" the floating reference by doing
> >>>> object_unref.  If we had floating references, the object would be
> >>>> returned to the caller unref'ed anyway.
> >>>
> >>> I was agreeing with Andreas at first (because it would make the
> >>> reference ownership rules simpler and easier to understand), until I
> >>> noticed that every call of qdev_try_create() and object_resolve_path()
> >>> in the code would need an additional object_unref() call if we didn't
> >>> use this pattern.
> >>>
> >>> But it bothers me that this exceptional behavior is not documented on
> >>> neither qdev_try_create() or object_resolve_path().
> >>>
> >>>>
> >>>> Of course, the reference can go away via QMP.  But that will only happen
> >>>> after the caller would have called object_unref itself.
> >>>
> >>> But the caller won't ever call object_unref() because it doesn't own any
> >>> reference, right? In this case, can we clarify the rules about how long
> >>> can callers safely expect the object to stay around? Can the object be
> >>> disposed in another thread? Can it be disposed only when some specific
> >>> events happen?
> >>
> >> In the inline docs for object_new_with_props I wrote
> >>
> >>   * The returned object will have one stable reference maintained
> >>   * for as long as it is present in the object hierarchy.
> >>
> >> We could expand it to explicitly say that 'object_unparent' is required
> >> to remove the object from the hierarchy and free it.
> > 
> > What's missing to me is some clarification on how long it is safe to
> > assume that the object won't be removed from the hierarchy by other
> > code.
> 
> There is no guarantee. Therefore any caller of object_resolve_path()
> must ref and unref as needed if doing more than a single operation, such
> as setting a link target.

So, does that mean zero guarantee, or guarantee for "a single
operation"? It can't be both.

(And how exactly "a single operation" should be defined?)

-- 
Eduardo