Re: [Qemu-devel] [PATCH] qom: Make object_unref() free the object's memory when refcount goes to 0.

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/22/2012 12:00 PM, alexander_barabash@mentor.com wrote:
> From: Alexander Barabash<alexander_barabash@mentor.com>
>
> In the existing implementation, object_delete()
> calls object_unref(), then frees the object's storage.
> Running object_delete() on an object with reference count
> different from 1 causes program failure.
>
> In the existing implementation, object_unref()
> finalizes the object when its reference count becomes 0.
>
> In the new implementation, object_unref()
> finalizes and frees the object's storage when the reference count becomes 0.
>
> In the new implementation, object_delete()
> just calls object_unref().
> Running object_delete() on an object with reference count
> different from 1 still causes program failure.

This isn't correct.  QOM objects don't necessarily have heap allocated objects.

I've been thinking about this general problem and I think the right way to solve 
it is to have a delete notifier list.  That way, object_new() can register a 
delete notifier that calls g_free() whenever refcount=0.  That way an explicit 
object_delete() isn't needed anymore.

Although I think we should keep the call around as it's convenient for replacing 
occurrences of qdev_free() where you really want the assert.

Regards,

Anthony Liguori

>
> Signed-off-by: Alexander Barabash<alexander_barabash@mentor.com>
> ---
>   qom/object.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/qom/object.c b/qom/object.c
> index e6591e1..8d36a9c 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -373,9 +373,8 @@ Object *object_new(const char *typename)
>
>   void object_delete(Object *obj)
>   {
> +    g_assert(obj->ref == 1);
>       object_unref(obj);
> -    g_assert(obj->ref == 0);
> -    g_free(obj);
>   }
>
>   static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type)
> @@ -585,6 +584,7 @@ void object_unref(Object *obj)
>       /* parent always holds a reference to its children */
>       if (obj->ref == 0) {
>           object_finalize(obj);
> +        g_free(obj);
>       }
>   }
>