qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org, "Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: Re: [PATCH 2/2] qom: Assert that objects being destroyed have no parent
Date: Wed, 16 Dec 2020 16:52:27 +0000	[thread overview]
Message-ID: <87lfdxsnee.fsf@linaro.org> (raw)
In-Reply-To: <20201215224133.3545901-3-ehabkost@redhat.com>


Eduardo Habkost <ehabkost@redhat.com> writes:

> QOM reference counting bugs are often hard to detect, but there's
> one kind of bug that's easier: if we are freeing an object but is
> still attached to a parent, it means the reference count is wrong
> (because the parent always hold a reference to their children).
>
> Add an assertion to make sure we detect those cases.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

I'm happy with an assert and crash early approach if the alternative it
scratching your head when things crash in weird ways. I'll of course
defer to the maintainer but from me:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  qom/object.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/qom/object.c b/qom/object.c
> index f2ae6e6b2a..5cfed6d7c6 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -685,6 +685,7 @@ static void object_finalize(void *data)
>      object_deinit(obj, ti);
>  
>      g_assert(obj->ref == 0);
> +    g_assert(obj->parent == NULL);
>      if (obj->free) {
>          obj->free(obj);
>      }


-- 
Alex Bennée


      parent reply	other threads:[~2020-12-16 17:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-15 22:41 [PATCH 0/2] Fix test-char reference counting bug Eduardo Habkost
2020-12-15 22:41 ` [PATCH 1/2] test-char: Destroy chardev correctly at char_file_test_internal() Eduardo Habkost
2020-12-16  7:45   ` Marc-André Lureau
2020-12-16 16:50   ` Alex Bennée
2020-12-15 22:41 ` [PATCH 2/2] qom: Assert that objects being destroyed have no parent Eduardo Habkost
2020-12-16  7:53   ` Marc-André Lureau
2020-12-16  9:55     ` Daniel P. Berrangé
2020-12-16 13:31     ` Paolo Bonzini
2020-12-16 16:15       ` Alex Bennée
2020-12-16 16:52   ` Alex Bennée [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lfdxsnee.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=berrange@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).