sd-bus: always keep slot reference while dispatching callback

Also, make sure we automatically destroy reply callbacks that are
floating.

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 b868159b5c..3391b1a1b6 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 f160e2343d..51d4a62cee 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 de947bf5c6..ec2843ff73 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 */