From f389bf15d0b732027669690ed9606a96c0568bbd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 3 Feb 2014 13:26:24 +0100 Subject: [PATCH] bus: when closing the bus don't end up in a recursive destruction deadlock --- src/libsystemd/sd-bus/sd-bus.c | 57 +++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 420393054..4fdc246b7 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -107,21 +107,21 @@ static void bus_node_destroy(sd_bus *b, struct node *n) { } static void bus_reset_queues(sd_bus *b) { - unsigned i; - assert(b); - for (i = 0; i < b->rqueue_size; i++) - sd_bus_message_unref(b->rqueue[i]); + while (b->rqueue_size > 0) + sd_bus_message_unref(b->rqueue[--b->rqueue_size]); + free(b->rqueue); + b->rqueue = NULL; + b->rqueue_allocated = 0; - for (i = 0; i < b->wqueue_size; i++) - sd_bus_message_unref(b->wqueue[i]); - free(b->wqueue); + while (b->wqueue_size > 0) + sd_bus_message_unref(b->wqueue[--b->wqueue_size]); - b->rqueue = b->wqueue = NULL; - b->rqueue_allocated = b->wqueue_allocated = 0; - b->rqueue_size = b->wqueue_size = 0; + free(b->wqueue); + b->wqueue = NULL; + b->wqueue_allocated = 0; } static void bus_free(sd_bus *b) { @@ -1340,21 +1340,36 @@ _public_ sd_bus *sd_bus_unref(sd_bus *bus) { if (!bus) return NULL; - i = REFCNT_DEC(bus->n_ref); - if (i != bus->rqueue_size + bus->wqueue_size) - return NULL; + if (REFCNT_GET(bus->n_ref) == bus->rqueue_size + bus->wqueue_size + 1) { + bool q = true; + + for (i = 0; i < bus->rqueue_size; i++) + if (bus->rqueue[i]->n_ref > 1) { + q = false; + break; + } - for (i = 0; i < bus->rqueue_size; i++) - if (bus->rqueue[i]->n_ref > 1) - return NULL; + if (q) { + for (i = 0; i < bus->wqueue_size; i++) + if (bus->wqueue[i]->n_ref > 1) { + q = false; + break; + } + } - for (i = 0; i < bus->wqueue_size; i++) - if (bus->wqueue[i]->n_ref > 1) - return NULL; + /* We are the only holders on the messages, and the + * messages are the only holders on us, so let's drop + * the messages and thus implicitly also kill our own + * last references */ - /* we are the only holders on the messages */ - bus_free(bus); + bus_reset_queues(bus); + } + i = REFCNT_DEC(bus->n_ref); + if (i > 0) + return NULL; + + bus_free(bus); return NULL; } -- 2.30.2