From cf063b8a1c3a04ba36a9ee11d06ff9a5f342cc36 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Thu, 13 Nov 2025 21:59:18 +0100 Subject: [PATCH] sd-bus: Exit event loop with error code instead of EXIT_FAILURE Instead of failing the event loop with a generic EXIT_FAILURE error code when exit-on-disconnect is used, let's propagate the error code instead of swallowing it. Whereas previously sd_event_loop() would always fail with exit code '1' when exit-on-disconnect is used with an sd-bus instance registered with the event loop that encounters a failure, now we'll correctly propagate the error to sd_event_loop() that caused sd-bus to fail and exit the event loop. Additionally, the error is now also properly propagated to outstanding reply callbacks for async dbus calls started with sd_bus_call_async() and friends, whereas before we always used ETIMEDOUT for these calls which is extremely confusing for users. Why is this confusing? We always start sd-bus instances asynchronously, in other words, sd_bus_start() will not actually wait until the bus instance is connected, but it'll happen in the background, either driven by the first sd_bus_call() when there is no event loop or by sd-event when there is an event loop attached to the sd-bus instance. Assuming an event loop is attached, when we fail to connect to the bus, the sd-bus instance will close down and the first async method call we queued will fail with ETIMEDOUT. Nowhere in this process do we inform the user that we failed to connect to the bus because of e.g. a permission error, except for a debug log message. By propagating the error to sd_event_exit() if exit-on-disconnect is enabled and always propagating it to outstanding reply callbacks, debugging failures becomes much easier as users will now get the actual error code causing the bus instance to close down instead of ETIMEDOUT and 1 respectively. --- src/libsystemd/sd-bus/bus-control.c | 10 ++--- src/libsystemd/sd-bus/bus-internal.h | 3 +- src/libsystemd/sd-bus/sd-bus.c | 50 +++++++++++---------- src/libsystemd/sd-bus/test-bus-watch-bind.c | 2 +- 4 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c index 1bf41bff3d3..f2338ae2b6a 100644 --- a/src/libsystemd/sd-bus/bus-control.c +++ b/src/libsystemd/sd-bus/bus-control.c @@ -140,7 +140,7 @@ static int default_request_name_handler( "Unable to request name, failing connection: %s", sd_bus_message_get_error(m)->message); - bus_enter_closing(sd_bus_message_get_bus(m)); + bus_enter_closing(sd_bus_message_get_bus(m), -sd_bus_message_get_errno(m)); return 1; } @@ -164,12 +164,12 @@ static int default_request_name_handler( case BUS_NAME_EXISTS: log_debug("Requested service name already owned, failing connection."); - bus_enter_closing(sd_bus_message_get_bus(m)); + bus_enter_closing(sd_bus_message_get_bus(m), -EEXIST); return 1; } log_debug("Unexpected response from RequestName(), failing connection."); - bus_enter_closing(sd_bus_message_get_bus(m)); + bus_enter_closing(sd_bus_message_get_bus(m), -EPROTO); return 1; } @@ -294,7 +294,7 @@ static int default_release_name_handler( "Unable to release name, failing connection: %s", sd_bus_message_get_error(m)->message); - bus_enter_closing(sd_bus_message_get_bus(m)); + bus_enter_closing(sd_bus_message_get_bus(m), -sd_bus_message_get_errno(m)); return 1; } @@ -318,7 +318,7 @@ static int default_release_name_handler( } log_debug("Unexpected response from ReleaseName(), failing connection."); - bus_enter_closing(sd_bus_message_get_bus(m)); + bus_enter_closing(sd_bus_message_get_bus(m), -EPROTO); return 1; } diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index e8145622de8..177f88c2c7a 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -164,6 +164,7 @@ typedef struct sd_bus { BusState state; int input_fd, output_fd; int inotify_fd; + int exit_code; int message_version; int message_endian; @@ -403,6 +404,6 @@ int bus_maybe_reply_error(sd_bus_message *m, int r, const sd_bus_error *e); return sd_bus_error_set_errno(error, r); \ } while (false) -void bus_enter_closing(sd_bus *bus); +void bus_enter_closing(sd_bus *bus, int exit_code); void bus_set_state(sd_bus *bus, BusState state); diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 5fab814abce..5644263fb55 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -257,6 +257,7 @@ _public_ int sd_bus_new(sd_bus **ret) { .input_fd = -EBADF, .output_fd = -EBADF, .inotify_fd = -EBADF, + .exit_code = EXIT_FAILURE, .message_version = 1, .creds_mask = SD_BUS_CREDS_WELL_KNOWN_NAMES|SD_BUS_CREDS_UNIQUE_NAME, .accept_fd = true, @@ -1813,13 +1814,14 @@ _public_ sd_bus* sd_bus_flush_close_unref(sd_bus *bus) { return sd_bus_close_unref(bus); } -void bus_enter_closing(sd_bus *bus) { +void bus_enter_closing(sd_bus *bus, int exit_code) { assert(bus); if (!IN_SET(bus->state, BUS_WATCH_BIND, BUS_OPENING, BUS_AUTHENTICATING, BUS_HELLO, BUS_RUNNING)) return; bus_set_state(bus, BUS_CLOSING); + bus->exit_code = exit_code; } /* Define manually so we can add the PID check */ @@ -2189,9 +2191,10 @@ _public_ int sd_bus_send(sd_bus *bus, sd_bus_message *_m, uint64_t *ret_cookie) r = bus_write_message(bus, m, &idx); if (ERRNO_IS_NEG_DISCONNECT(r)) { - bus_enter_closing(bus); + bus_enter_closing(bus, r); return -ECONNRESET; - } else if (r < 0) + } + if (r < 0) return r; if (idx < BUS_MESSAGE_SIZE(m)) { @@ -2486,7 +2489,7 @@ _public_ int sd_bus_call( r = bus_read_message(bus); if (r < 0) { if (ERRNO_IS_DISCONNECT(r)) { - bus_enter_closing(bus); + bus_enter_closing(bus, r); r = -ECONNRESET; } @@ -2521,7 +2524,7 @@ _public_ int sd_bus_call( r = dispatch_wqueue(bus); if (r < 0) { if (ERRNO_IS_DISCONNECT(r)) { - bus_enter_closing(bus); + bus_enter_closing(bus, r); r = -ECONNRESET; } @@ -3110,14 +3113,14 @@ static int bus_exit_now(sd_bus *bus, sd_event *event) { event = bus->event; if (event) - return sd_event_exit(event, EXIT_FAILURE); + return sd_event_exit(event, bus->exit_code); exit(EXIT_FAILURE); assert_not_reached(); } static int process_closing_reply_callback(sd_bus *bus, BusReplyCallback *c) { - _cleanup_(sd_bus_error_free) sd_bus_error error_buffer = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL, error_buffer = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; sd_bus_slot *slot; int r; @@ -3125,11 +3128,12 @@ static int process_closing_reply_callback(sd_bus *bus, BusReplyCallback *c) { assert(bus); assert(c); - r = bus_message_new_synthetic_error( - bus, - c->cookie, - &SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_NO_REPLY, "Connection terminated"), - &m); + (void) sd_bus_error_set_errnof( + &error, + bus->exit_code < 0 ? bus->exit_code : -ETIMEDOUT, + "Connection terminated"); + + r = bus_message_new_synthetic_error(bus, c->cookie, &error, &m); if (r < 0) return r; @@ -3293,7 +3297,7 @@ static int bus_process_internal(sd_bus *bus, sd_bus_message **ret) { } if (ERRNO_IS_NEG_DISCONNECT(r)) { - bus_enter_closing(bus); + bus_enter_closing(bus, r); r = 1; } else if (r < 0) return r; @@ -3427,7 +3431,7 @@ _public_ int sd_bus_flush(sd_bus *bus) { for (;;) { r = dispatch_wqueue(bus); if (ERRNO_IS_NEG_DISCONNECT(r)) { - bus_enter_closing(bus); + bus_enter_closing(bus, r); return -ECONNRESET; } else if (r < 0) return r; @@ -3478,17 +3482,17 @@ static int add_match_callback( sd_bus_slot *match_slot = ASSERT_PTR(userdata); bool failed = false; - int r; + int r = 0; assert(m); sd_bus_slot_ref(match_slot); if (sd_bus_message_is_method_error(m, NULL)) { - log_debug_errno(sd_bus_message_get_errno(m), - "Unable to add match %s, failing connection: %s", - match_slot->match_callback.match_string, - sd_bus_message_get_error(m)->message); + r = log_debug_errno(sd_bus_message_get_errno(m), + "Unable to add match %s, failing connection: %s", + match_slot->match_callback.match_string, + sd_bus_message_get_error(m)->message); failed = true; } else @@ -3518,7 +3522,7 @@ static int add_match_callback( bus->current_userdata = userdata; } else { if (failed) /* Generic failure handling: destroy the connection */ - bus_enter_closing(sd_bus_message_get_bus(m)); + bus_enter_closing(sd_bus_message_get_bus(m), r); r = 1; } @@ -3649,7 +3653,7 @@ static int io_callback(sd_event_source *s, int fd, uint32_t revents, void *userd r = sd_bus_process(bus, NULL); if (r < 0) { log_debug_errno(r, "Processing of bus failed, closing down: %m"); - bus_enter_closing(bus); + bus_enter_closing(bus, r); } return 1; @@ -3662,7 +3666,7 @@ static int time_callback(sd_event_source *s, uint64_t usec, void *userdata) { r = sd_bus_process(bus, NULL); if (r < 0) { log_debug_errno(r, "Processing of bus failed, closing down: %m"); - bus_enter_closing(bus); + bus_enter_closing(bus, r); } return 1; @@ -3714,7 +3718,7 @@ static int prepare_callback(sd_event_source *s, void *userdata) { fail: log_debug_errno(r, "Preparing of bus events failed, closing down: %m"); - bus_enter_closing(bus); + bus_enter_closing(bus, r); return 1; } diff --git a/src/libsystemd/sd-bus/test-bus-watch-bind.c b/src/libsystemd/sd-bus/test-bus-watch-bind.c index 73fe2727550..1bf4ee70171 100644 --- a/src/libsystemd/sd-bus/test-bus-watch-bind.c +++ b/src/libsystemd/sd-bus/test-bus-watch-bind.c @@ -33,7 +33,7 @@ static int method_exit(sd_bus_message *m, void *userdata, sd_bus_error *ret_erro ASSERT_OK(sd_bus_reply_method_return(m, NULL)); /* 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)); + bus_enter_closing(sd_bus_message_get_bus(m), EXIT_FAILURE); return 0; } -- 2.47.3