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.

diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c
index 1bf41bff3d31f9f552a4277868eae48405f9ddf6..f2338ae2b6a8e73bc99d40fd342ab7d481650618 100644 (file)
--- 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 e8145622de84416336472b261dfc2e2927e092d1..177f88c2c7a4c73ab4128128e932982638de72e6 100644 (file)
--- 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 5fab814abce32ebf15be8f7c828a181ba5360be1..5644263fb5573b9e1f24f4a0bb41f617a514217a 100644 (file)
--- 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 73fe27275501a9e4b04275ae7aa2d99094b74127..1bf4ee7017119f70f119885b82ed744563ceccc8 100644 (file)
--- 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;
 }