From 1b64f8382956cdd9a2afc50a7ab638529acb912e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 15 May 2014 17:08:24 +0200 Subject: [PATCH] sd-bus: always keep slot reference while dispatching callback Also, make sure we automatically destroy reply callbacks that are floating. --- src/libsystemd/sd-bus/bus-match.c | 7 ++- src/libsystemd/sd-bus/bus-objects.c | 37 ++++++++------ src/libsystemd/sd-bus/sd-bus.c | 76 +++++++++++++++++------------ 3 files changed, 70 insertions(+), 50 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-match.c b/src/libsystemd/sd-bus/bus-match.c index b868159b5..3391b1a1b 100644 --- a/src/libsystemd/sd-bus/bus-match.c +++ b/src/libsystemd/sd-bus/bus-match.c @@ -290,16 +290,15 @@ int bus_match_run( /* Run the callback. And then invoke siblings. */ if (node->leaf.callback) { - sd_bus_slot *slot; - _cleanup_bus_error_free_ sd_bus_error error_buffer = SD_BUS_ERROR_NULL; + sd_bus_slot *slot; slot = container_of(node->leaf.callback, sd_bus_slot, match_callback); if (bus) - bus->current_slot = slot; + bus->current_slot = sd_bus_slot_ref(slot); r = node->leaf.callback->callback(bus, m, slot->userdata, &error_buffer); if (bus) - bus->current_slot = NULL; + bus->current_slot = sd_bus_slot_unref(slot); r = bus_maybe_reply_error(m, r, &error_buffer); if (r != 0) diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index f160e2343..51d4a62ce 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -50,9 +50,9 @@ static int node_vtable_get_userdata( s = container_of(c, sd_bus_slot, node_vtable); u = s->userdata; if (c->find) { - bus->current_slot = s; + bus->current_slot = sd_bus_slot_ref(s); r = c->find(bus, path, c->interface, u, &u, error); - bus->current_slot = NULL; + bus->current_slot = sd_bus_slot_unref(s); if (r < 0) return r; @@ -115,13 +115,16 @@ static int add_enumerated_to_set( LIST_FOREACH(enumerators, c, first) { char **children = NULL, **k; + sd_bus_slot *slot; if (bus->nodes_modified) return 0; - bus->current_slot = container_of(c, sd_bus_slot, node_enumerator); - r = c->callback(bus, prefix, bus->current_slot->userdata, &children, error); - bus->current_slot = NULL; + slot = container_of(c, sd_bus_slot, node_enumerator); + + bus->current_slot = sd_bus_slot_ref(slot); + r = c->callback(bus, prefix, slot->userdata, &children, error); + bus->current_slot = sd_bus_slot_unref(slot); if (r < 0) return r; @@ -248,6 +251,7 @@ static int node_callbacks_run( LIST_FOREACH(callbacks, c, first) { _cleanup_bus_error_free_ sd_bus_error error_buffer = SD_BUS_ERROR_NULL; + sd_bus_slot *slot; if (bus->nodes_modified) return 0; @@ -266,9 +270,11 @@ static int node_callbacks_run( if (r < 0) return r; - bus->current_slot = container_of(c, sd_bus_slot, node_callback); - r = c->callback(bus, m, bus->current_slot->userdata, &error_buffer); - bus->current_slot = NULL; + slot = container_of(c, sd_bus_slot, node_callback); + + bus->current_slot = sd_bus_slot_ref(slot); + r = c->callback(bus, m, slot->userdata, &error_buffer); + bus->current_slot = sd_bus_slot_unref(slot); r = bus_maybe_reply_error(m, r, &error_buffer); if (r != 0) @@ -394,10 +400,13 @@ static int method_callbacks_run( m->enforced_reply_signature = strempty(c->vtable->x.method.result); if (c->vtable->x.method.handler) { + sd_bus_slot *slot; + + slot = container_of(c->parent, sd_bus_slot, node_vtable); - bus->current_slot = container_of(c->parent, sd_bus_slot, node_vtable); + bus->current_slot = sd_bus_slot_ref(slot); r = c->vtable->x.method.handler(bus, m, u, &error); - bus->current_slot = NULL; + bus->current_slot = sd_bus_slot_unref(slot); return bus_maybe_reply_error(m, r, &error); } @@ -434,9 +443,9 @@ static int invoke_property_get( if (v->x.property.get) { - bus->current_slot = slot; + bus->current_slot = sd_bus_slot_ref(slot); r = v->x.property.get(bus, path, interface, property, reply, userdata, error); - bus->current_slot = NULL; + bus->current_slot = sd_bus_slot_unref(slot); if (r < 0) return r; @@ -496,9 +505,9 @@ static int invoke_property_set( if (v->x.property.set) { - bus->current_slot = slot; + bus->current_slot = sd_bus_slot_ref(slot); r = v->x.property.set(bus, path, interface, property, value, userdata, error); - bus->current_slot = NULL; + bus->current_slot = sd_bus_slot_unref(slot); if (r < 0) return r; diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index de947bf5c..ec2843ff7 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -2090,6 +2090,7 @@ static int process_timeout(sd_bus *bus) { _cleanup_bus_error_free_ sd_bus_error error_buffer = SD_BUS_ERROR_NULL; _cleanup_bus_message_unref_ sd_bus_message* m = NULL; struct reply_callback *c; + sd_bus_slot *slot; usec_t n; int r; @@ -2123,18 +2124,22 @@ static int process_timeout(sd_bus *bus) { hashmap_remove(bus->reply_callbacks, &c->cookie); c->cookie = 0; - bus->current_message = m; - bus->current_slot = container_of(c, sd_bus_slot, reply_callback); + slot = container_of(c, sd_bus_slot, reply_callback); bus->iteration_counter ++; - r = c->callback(bus, m, bus->current_slot->userdata, &error_buffer); - r = bus_maybe_reply_error(m, r, &error_buffer); - + bus->current_message = m; + bus->current_slot = sd_bus_slot_ref(slot); + r = c->callback(bus, m, slot->userdata, &error_buffer); + bus->current_slot = sd_bus_slot_unref(slot); bus->current_message = NULL; - bus->current_slot = NULL; - return r; + if (slot->floating) { + bus_slot_disconnect(slot); + sd_bus_slot_unref(slot); + } + + return bus_maybe_reply_error(m, r, &error_buffer); } static int process_hello(sd_bus *bus, sd_bus_message *m) { @@ -2162,8 +2167,8 @@ static int process_hello(sd_bus *bus, sd_bus_message *m) { static int process_reply(sd_bus *bus, sd_bus_message *m) { _cleanup_bus_message_unref_ sd_bus_message *synthetic_reply = NULL; _cleanup_bus_error_free_ sd_bus_error error_buffer = SD_BUS_ERROR_NULL; - sd_bus_slot *slot; struct reply_callback *c; + sd_bus_slot *slot; int r; assert(bus); @@ -2184,12 +2189,8 @@ static int process_reply(sd_bus *bus, sd_bus_message *m) { return 0; c->cookie = 0; - slot = container_of(c, sd_bus_slot, reply_callback); - if (c->timeout != 0) { - prioq_remove(bus->reply_callbacks_prioq, c, &c->prioq_idx); - c->timeout = 0; - } + slot = container_of(c, sd_bus_slot, reply_callback); if (m->n_fds > 0 && !(bus->hello_flags & KDBUS_HELLO_ACCEPT_FD)) { @@ -2202,32 +2203,34 @@ static int process_reply(sd_bus *bus, sd_bus_message *m) { &SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_INCONSISTENT_MESSAGE, "Reply message contained file descriptor"), &synthetic_reply); if (r < 0) - goto finish; + return r; r = bus_seal_synthetic_message(bus, synthetic_reply); if (r < 0) - goto finish; + return r; m = synthetic_reply; } else { r = sd_bus_message_rewind(m, true); if (r < 0) - goto finish; + return r; } - bus->current_slot = slot; - r = c->callback(bus, m, bus->current_slot->userdata, &error_buffer); - bus->current_slot = NULL; + if (c->timeout != 0) { + prioq_remove(bus->reply_callbacks_prioq, c, &c->prioq_idx); + c->timeout = 0; + } - r = bus_maybe_reply_error(m, r, &error_buffer); + bus->current_slot = sd_bus_slot_ref(slot); + r = c->callback(bus, m, slot->userdata, &error_buffer); + bus->current_slot = sd_bus_slot_unref(slot); -finish: if (slot->floating) { bus_slot_disconnect(slot); sd_bus_slot_unref(slot); } - return r; + return bus_maybe_reply_error(m, r, &error_buffer); } static int process_filter(sd_bus *bus, sd_bus_message *m) { @@ -2242,6 +2245,7 @@ static int process_filter(sd_bus *bus, sd_bus_message *m) { bus->filter_callbacks_modified = false; LIST_FOREACH(callbacks, l, bus->filter_callbacks) { + sd_bus_slot *slot; if (bus->filter_callbacks_modified) break; @@ -2256,9 +2260,11 @@ static int process_filter(sd_bus *bus, sd_bus_message *m) { if (r < 0) return r; - bus->current_slot = container_of(l, sd_bus_slot, filter_callback); - r = l->callback(bus, m, bus->current_slot->userdata, &error_buffer); - bus->current_slot = NULL; + slot = container_of(l, sd_bus_slot, filter_callback); + + bus->current_slot = sd_bus_slot_ref(slot); + r = l->callback(bus, m, slot->userdata, &error_buffer); + bus->current_slot = sd_bus_slot_unref(slot); r = bus_maybe_reply_error(m, r, &error_buffer); if (r != 0) @@ -2505,6 +2511,7 @@ static int process_closing(sd_bus *bus, sd_bus_message **ret) { c = hashmap_first(bus->reply_callbacks); if (c) { _cleanup_bus_error_free_ sd_bus_error error_buffer = SD_BUS_ERROR_NULL; + sd_bus_slot *slot; /* First, fail all outstanding method calls */ r = bus_message_new_synthetic_error( @@ -2527,17 +2534,22 @@ static int process_closing(sd_bus *bus, sd_bus_message **ret) { hashmap_remove(bus->reply_callbacks, &c->cookie); c->cookie = 0; - bus->current_message = m; - bus->current_slot = container_of(c, sd_bus_slot, reply_callback); + slot = container_of(c, sd_bus_slot, reply_callback); bus->iteration_counter++; - r = c->callback(bus, m, bus->current_slot->userdata, &error_buffer); - r = bus_maybe_reply_error(m, r, &error_buffer); - - bus->current_slot = NULL; + bus->current_message = m; + bus->current_slot = sd_bus_slot_ref(slot); + r = c->callback(bus, m, slot->userdata, &error_buffer); + bus->current_slot = sd_bus_slot_unref(slot); + bus->current_message = NULL; + + if (slot->floating) { + bus_slot_disconnect(slot); + sd_bus_slot_unref(slot); + } - goto finish; + return bus_maybe_reply_error(m, r, &error_buffer); } /* Then, synthesize a Disconnected message */ -- 2.30.2