From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39053) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Td1EV-0002X1-SK for qemu-devel@nongnu.org; Mon, 26 Nov 2012 11:08:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Td1ER-0001Yt-Q1 for qemu-devel@nongnu.org; Mon, 26 Nov 2012 11:08:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:23103) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Td1ER-0001Yp-H1 for qemu-devel@nongnu.org; Mon, 26 Nov 2012 11:08:27 -0500 Message-ID: <50B393F4.5000808@redhat.com> Date: Mon, 26 Nov 2012 17:08:20 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1353660436-8897-1-git-send-email-pbonzini@redhat.com> <1353660436-8897-2-git-send-email-pbonzini@redhat.com> <87ehjgpdoy.fsf@codemonkey.ws> In-Reply-To: <87ehjgpdoy.fsf@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1.3 1/5] qom: fix refcount of non-heap-allocated objects List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org, Liu Ping Fan 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