From: Paolo Bonzini <pbonzini@redhat.com>
To: Anthony Liguori <aliguori@us.ibm.com>
Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org,
Liu Ping Fan <qemulist@gmail.com>
Subject: Re: [Qemu-devel] [PATCH 1.3 1/5] qom: fix refcount of non-heap-allocated objects
Date: Mon, 26 Nov 2012 17:08:20 +0100 [thread overview]
Message-ID: <50B393F4.5000808@redhat.com> (raw)
In-Reply-To: <87ehjgpdoy.fsf@codemonkey.ws>
Il 26/11/2012 16:49, Anthony Liguori ha scritto:
> But object_property_add_child() will take a reference.
> When the parent object goes away, this will cause that reference to get
> dropped and ultimately the child object to be destroyed.
This is still wrong if you have:
object_init(&obj->subobj, ...)
foo(&obj->subobj);
object_property_add_child()
where foo() does a ref+unref pair (see the pattern of scsi_req_enqueue
for example, even though it's not QOM).
The object's memory area is being kept live by the parent, so it
logically has a nonzero reference count.
We don't have any example of embedding yet in the tree, except for
buses, so I think we are somewhat free to set the rules.
As I said elsewhere in the thread, the rule I'd prefer the most is "it
doesn't matter whether an object is statically- or
dynamically-allocated". That is, even for an embedded object which you
create in instance_init, you should object_unref it explicitly, either
in instance_finalize or after initializing it (as you prefer).
Note that instance_finalize will anyway have an object_unparent of the
embedded object, in order to remove any cyclical references, so it's not
a big change.
> IOW, this change causes embedded objects to get leaked AFAICT.
Hmm, patch 4 in the series offsets this by doing
void qbus_free(BusState *bus)
{
- if (bus->qom_allocated) {
- object_delete(OBJECT(bus));
- } else {
- object_finalize(OBJECT(bus));
- if (bus->glib_allocated) {
- g_free(bus);
- }
- }
+ object_delete(OBJECT(bus));
}
So at the end of the series there is no leak.
Paolo
next prev parent reply other threads:[~2012-11-26 16:08 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-23 8:47 [Qemu-devel] [PATCH 1.3 0/5] QOM/qdev lifetime fixes Paolo Bonzini
2012-11-23 8:47 ` [Qemu-devel] [PATCH 1.3 1/5] qom: fix refcount of non-heap-allocated objects Paolo Bonzini
2012-11-23 16:49 ` Andreas Färber
2012-11-26 15:49 ` Anthony Liguori
2012-11-26 16:08 ` Paolo Bonzini [this message]
2012-11-23 8:47 ` [Qemu-devel] [PATCH 1.3 2/5] qdev: move bus removal to object_unparent Paolo Bonzini
2012-11-23 16:52 ` Andreas Färber
2012-11-27 1:02 ` Andreas Färber
2012-11-23 8:47 ` [Qemu-devel] [PATCH 1.3 3/5] qom: make object_delete usable for statically-allocated objects Paolo Bonzini
2012-11-23 17:02 ` Andreas Färber
2012-11-23 17:10 ` Paolo Bonzini
2012-11-23 8:47 ` [Qemu-devel] [PATCH 1.3 4/5] qdev: simplify (de)allocation of buses Paolo Bonzini
2012-11-23 17:07 ` Andreas Färber
2012-11-23 17:16 ` Paolo Bonzini
2012-11-23 8:47 ` [Qemu-devel] [PATCH 1.3 5/5] qom: make object_finalize static Paolo Bonzini
2012-11-23 17:12 ` Andreas Färber
2012-11-23 17:32 ` Paolo Bonzini
2012-11-26 21:47 ` [Qemu-devel] [PATCH 1.3 0/5] QOM/qdev lifetime fixes Anthony Liguori
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=50B393F4.5000808@redhat.com \
--to=pbonzini@redhat.com \
--cc=aliguori@us.ibm.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=qemulist@gmail.com \
/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).