Re: [Qemu-devel] [RFC 1/3] qom: Document reference count ownership rules
On Fri, Jul 13, 2018 at 11:07:15AM +0200, Thomas Huth wrote: > On 12.07.2018 21:45, Eduardo Habkost wrote: > > The documentation for QOM is not clear about who owns references > > to objects (i.e. who is responsible for calling object_unref() > > later). > > > > This is important considering there are a few inconsistencies in > > the API (e.g. callers of object_new() need to call object_unref() > > later, but callers of object_new_with_props() must not do it). > > > > Update the documentation so that every mention of object > > references also mention who exactly owns the reference. > > > > Signed-off-by: Eduardo Habkost > > --- > > include/qom/object.h | 73 +--- > > 1 file changed, 42 insertions(+), 31 deletions(-) > > > > diff --git a/include/qom/object.h b/include/qom/object.h > > index f3d2308d56..08a1bbba7d 100644 > > --- a/include/qom/object.h > > +++ b/include/qom/object.h > > @@ -376,7 +376,7 @@ typedef void (ObjectUnparent)(Object *obj); > > * ObjectFree: > > * @obj: the object being freed > > * > > - * Called when an object's last reference is removed. > > + * Called when an object's last reference is dropped using object_unref(). > > */ > > typedef void (ObjectFree)(void *obj); > > > > @@ -601,8 +601,8 @@ struct InterfaceClass > > * @typename: The name of the type of the object to instantiate. > > * > > * This function will initialize a new object using heap allocated memory. > > - * The returned object has a reference count of 1, and will be freed when > > - * the last reference is dropped. > > + * The returned object has a reference count of 1, and the reference will > > be > > + * owned by the caller. > > * > > * Returns: The newly allocated and instantiated object. > > */ > > @@ -617,8 +617,8 @@ Object *object_new(const char *typename); > > * @...: list of property names and values > > * > > * This function will initialize a new object using heap allocated memory. > > - * The returned object has a reference count of 1, and will be freed when > > - * the last reference is dropped. > > + * The returned object has a reference count of 1, and the reference will > > + * be owned by the caller. > > That's the description of object_new_with_props here already, isn't it? > So the reference will be owned by the parent object, not by the caller. Oops, you're right. Thanks! > > > * The @id parameter will be used when registering the object as a > > * child of @parent in the composition tree. > > @@ -652,8 +652,8 @@ Object *object_new(const char *typename); > > * > > * > > * > > - * The returned object will have one stable reference maintained > > - * for as long as it is present in the object hierarchy. > > + * The returned object will have one reference, owned by the > > + * parent object (not by the caller). > > ... and then this information here is somewhat redundant. I suggest to > remove one of the two spots. Will change this to: /** * object_new_with_props: * @typename: The name of the type of the object to instantiate. * @parent: the parent object * @id: The unique ID of the object * @errp: pointer to error object * @...: list of property names and values * * This function will initialize a new object using heap allocated memory. - * The returned object has a reference count of 1, and will be freed when - * the last reference is dropped. + * The returned object will have one reference, owned by the + * parent object (not by the caller). * * The @id parameter will be used when registering the object as a * child of @parent in the composition tree. * * The variadic parameters are a list of pairs of (propname, propvalue) * strings. The propname of %NULL indicates the end of the property * list. If the object implements the user creatable interface, the * object will be marked complete once all the properties have been * processed. * * * Creating an object with properties * * Error *err = NULL; * Object *obj; * * obj = object_new_with_props(TYPE_MEMORY_BACKEND_FILE, * object_get_objects_root(), * "hostmem0", * , * "share", "yes", * "mem-path", "/dev/shm/somefile", * "prealloc", "yes", * "size", "1048576", * NULL); * * if (!obj) { * g_printerr("Cannot create memory backend: %s\n", *error_get_pretty(err)); * } * * * - * The returned object will have one stable reference maintained - * for as long as it is present in the object hierarchy. - * * Returns: The newly allocated, instantiated & initialized object. */ Object *object_new_with_props(const char *typename, Object *parent,
Re: [Qemu-devel] [RFC 1/3] qom: Document reference count ownership rules
On 12.07.2018 21:45, Eduardo Habkost wrote: > The documentation for QOM is not clear about who owns references > to objects (i.e. who is responsible for calling object_unref() > later). > > This is important considering there are a few inconsistencies in > the API (e.g. callers of object_new() need to call object_unref() > later, but callers of object_new_with_props() must not do it). > > Update the documentation so that every mention of object > references also mention who exactly owns the reference. > > Signed-off-by: Eduardo Habkost > --- > include/qom/object.h | 73 +--- > 1 file changed, 42 insertions(+), 31 deletions(-) > > diff --git a/include/qom/object.h b/include/qom/object.h > index f3d2308d56..08a1bbba7d 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -376,7 +376,7 @@ typedef void (ObjectUnparent)(Object *obj); > * ObjectFree: > * @obj: the object being freed > * > - * Called when an object's last reference is removed. > + * Called when an object's last reference is dropped using object_unref(). > */ > typedef void (ObjectFree)(void *obj); > > @@ -601,8 +601,8 @@ struct InterfaceClass > * @typename: The name of the type of the object to instantiate. > * > * This function will initialize a new object using heap allocated memory. > - * The returned object has a reference count of 1, and will be freed when > - * the last reference is dropped. > + * The returned object has a reference count of 1, and the reference will be > + * owned by the caller. > * > * Returns: The newly allocated and instantiated object. > */ > @@ -617,8 +617,8 @@ Object *object_new(const char *typename); > * @...: list of property names and values > * > * This function will initialize a new object using heap allocated memory. > - * The returned object has a reference count of 1, and will be freed when > - * the last reference is dropped. > + * The returned object has a reference count of 1, and the reference will > + * be owned by the caller. That's the description of object_new_with_props here already, isn't it? So the reference will be owned by the parent object, not by the caller. > * The @id parameter will be used when registering the object as a > * child of @parent in the composition tree. > @@ -652,8 +652,8 @@ Object *object_new(const char *typename); > * > * > * > - * The returned object will have one stable reference maintained > - * for as long as it is present in the object hierarchy. > + * The returned object will have one reference, owned by the > + * parent object (not by the caller). ... and then this information here is somewhat redundant. I suggest to remove one of the two spots. Thomas
[Qemu-devel] [RFC 1/3] qom: Document reference count ownership rules
The documentation for QOM is not clear about who owns references to objects (i.e. who is responsible for calling object_unref() later). This is important considering there are a few inconsistencies in the API (e.g. callers of object_new() need to call object_unref() later, but callers of object_new_with_props() must not do it). Update the documentation so that every mention of object references also mention who exactly owns the reference. Signed-off-by: Eduardo Habkost --- include/qom/object.h | 73 +--- 1 file changed, 42 insertions(+), 31 deletions(-) diff --git a/include/qom/object.h b/include/qom/object.h index f3d2308d56..08a1bbba7d 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -376,7 +376,7 @@ typedef void (ObjectUnparent)(Object *obj); * ObjectFree: * @obj: the object being freed * - * Called when an object's last reference is removed. + * Called when an object's last reference is dropped using object_unref(). */ typedef void (ObjectFree)(void *obj); @@ -601,8 +601,8 @@ struct InterfaceClass * @typename: The name of the type of the object to instantiate. * * This function will initialize a new object using heap allocated memory. - * The returned object has a reference count of 1, and will be freed when - * the last reference is dropped. + * The returned object has a reference count of 1, and the reference will be + * owned by the caller. * * Returns: The newly allocated and instantiated object. */ @@ -617,8 +617,8 @@ Object *object_new(const char *typename); * @...: list of property names and values * * This function will initialize a new object using heap allocated memory. - * The returned object has a reference count of 1, and will be freed when - * the last reference is dropped. + * The returned object has a reference count of 1, and the reference will + * be owned by the caller. * * The @id parameter will be used when registering the object as a * child of @parent in the composition tree. @@ -652,8 +652,8 @@ Object *object_new(const char *typename); * * * - * The returned object will have one stable reference maintained - * for as long as it is present in the object hierarchy. + * The returned object will have one reference, owned by the + * parent object (not by the caller). * * Returns: The newly allocated, instantiated & initialized object. */ @@ -713,9 +713,6 @@ Object *object_new_with_propv(const char *typename, * * * - * The returned object will have one stable reference maintained - * for as long as it is present in the object hierarchy. - * * Returns: -1 on error, 0 on success */ int object_set_props(Object *obj, @@ -744,7 +741,7 @@ int object_set_propv(Object *obj, * * This function will initialize an object. The memory for the object should * have already been allocated. The returned object has a reference count of 1, - * and will be finalized when the last reference is dropped. + * and the reference will be owned by the caller. */ void object_initialize(void *obj, size_t size, const char *typename); @@ -928,8 +925,12 @@ GSList *object_class_get_list_sorted(const char *implements_type, * object_ref: * @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. + * Increase the reference count of a object. A object won't be freed as + * long as its reference count is greater than zero. + * + * The new reference will be owned by the caller, meaning the caller is + * responsible for ensuring object_unref() will be called once the + * reference is not needed anymore. */ void object_ref(Object *obj); @@ -937,8 +938,10 @@ void object_ref(Object *obj); * object_unref: * @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. + * Drop a reference to a object, decreasing its reference count. This + * function should be called only if the caller owns a reference taken using + * object_ref() or returned by functions like object_initialize() and + * object_new(). */ void object_unref(Object *obj); @@ -1104,10 +1107,13 @@ char *object_property_get_str(Object *obj, const char *name, * * Writes an object's canonical path to a property. * - * If the link property was created with - * OBJ_PROP_LINK_STRONG bit, the old target object is - * unreferenced, and a reference is added to the new target object. + * If the link property was created with OBJ_PROP_LINK_STRONG + * bit, the property will take a reference to the object, and drop + * a reference to the old object. * + * If OBJ_PROP_LINK_STRONG was not set for the property, the + * caller is responsible for ensuring the object will stay alive until the + * property is modified or deleted. */ void object_property_set_link(Object *obj, Object *value, const char *name,