From b5d4862707d0b3829c500904474fa2453ffaa525 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 25 Jan 2024 20:31:39 +0000 Subject: [PATCH] sd-bus: fix exiting event loop when sd_bus_set_exit_on_disconnect is used If sd_bus_set_exit_on_disconnect is used and the bus is part of an event loop, and the D-Bus connection goes away (e.g.: soft-reboot), sd-bus will always exit() the program instead of returning from the loop, as the reference to the event is removed before it is checked. --- src/libsystemd/sd-bus/sd-bus.c | 18 +++++++++++++----- src/libsystemd/sd-bus/test-bus-watch-bind.c | 9 +++++++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 46a367cbd7..4fdfc9a581 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -3087,7 +3087,7 @@ null_message: return r; } -static int bus_exit_now(sd_bus *bus) { +static int bus_exit_now(sd_bus *bus, sd_event *event) { assert(bus); /* Exit due to close, if this is requested. If this is bus object is attached to an event source, invokes @@ -3104,8 +3104,11 @@ static int bus_exit_now(sd_bus *bus) { log_debug("Bus connection disconnected, exiting."); - if (bus->event) - return sd_event_exit(bus->event, EXIT_FAILURE); + if (!event) + event = bus->event; + + if (event) + return sd_event_exit(event, EXIT_FAILURE); else exit(EXIT_FAILURE); @@ -3167,6 +3170,7 @@ static int process_closing_reply_callback(sd_bus *bus, struct reply_callback *c) static int process_closing(sd_bus *bus, sd_bus_message **ret) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; + _cleanup_(sd_event_unrefp) sd_event *event = NULL; struct reply_callback *c; int r; @@ -3201,6 +3205,10 @@ static int process_closing(sd_bus *bus, sd_bus_message **ret) { if (r < 0) return r; + /* sd_bus_close() will deref the event and set bus->event to NULL. But in bus_exit_now() we use + * bus->event to decide whether to return from the event loop or exit(), but given it's always NULL + * at that point, it always exit(). Ref it here and pass it through further down to avoid that. */ + event = sd_event_ref(bus->event); sd_bus_close(bus); bus->current_message = m; @@ -3216,7 +3224,7 @@ static int process_closing(sd_bus *bus, sd_bus_message **ret) { /* Nothing else to do, exit now, if the condition holds */ bus->exit_triggered = true; - (void) bus_exit_now(bus); + (void) bus_exit_now(bus, event); if (ret) *ret = TAKE_PTR(m); @@ -4312,7 +4320,7 @@ _public_ int sd_bus_set_exit_on_disconnect(sd_bus *bus, int b) { bus->exit_on_disconnect = b; /* If the exit condition was triggered already, exit immediately. */ - return bus_exit_now(bus); + return bus_exit_now(bus, /* event= */ NULL); } _public_ int sd_bus_get_exit_on_disconnect(sd_bus *bus) { diff --git a/src/libsystemd/sd-bus/test-bus-watch-bind.c b/src/libsystemd/sd-bus/test-bus-watch-bind.c index d6938a7f09..7f73c6e7b8 100644 --- a/src/libsystemd/sd-bus/test-bus-watch-bind.c +++ b/src/libsystemd/sd-bus/test-bus-watch-bind.c @@ -7,6 +7,7 @@ #include "sd-id128.h" #include "alloc-util.h" +#include "bus-internal.h" #include "fd-util.h" #include "fs-util.h" #include "mkdir.h" @@ -27,8 +28,11 @@ static int method_foobar(sd_bus_message *m, void *userdata, sd_bus_error *ret_er static int method_exit(sd_bus_message *m, void *userdata, sd_bus_error *ret_error) { log_info("Got Exit() call"); - assert_se(sd_event_exit(sd_bus_get_event(sd_bus_message_get_bus(m)), 1) >= 0); - return sd_bus_reply_method_return(m, NULL); + + assert_se(sd_bus_reply_method_return(m, NULL) >= 0); + /* Simulate D-Bus going away to test the bus_exit_now() path with exit_on_disconnect set */ + bus_enter_closing(sd_bus_message_get_bus(m)); + return 0; } static const sd_bus_vtable vtable[] = { @@ -100,6 +104,7 @@ static void* thread_server(void *p) { log_debug("Accepted server connection"); assert_se(sd_bus_new(&bus) >= 0); + assert_se(sd_bus_set_exit_on_disconnect(bus, true) >= 0); assert_se(sd_bus_set_description(bus, "server") >= 0); assert_se(sd_bus_set_fd(bus, bus_fd, bus_fd) >= 0); assert_se(sd_bus_set_server(bus, true, id) >= 0); -- 2.25.1