Merge pull request #27605 from YHNdnzj/dbus-activation-followup

core: only refuse Type=dbus service enqueue if dbus has pending stop job

4 files changed, 31 insertions, 18 deletions

diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index 5b89c76586..59d541ebfe 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -1875,13 +1875,30 @@ int bus_unit_queue_job(
             (type == JOB_STOP && u->refuse_manual_stop) ||
             (IN_SET(type, JOB_RESTART, JOB_TRY_RESTART) && (u->refuse_manual_start || u->refuse_manual_stop)) ||
             (type == JOB_RELOAD_OR_START && job_type_collapse(type, u) == JOB_START && u->refuse_manual_start))
-                return sd_bus_error_setf(error, BUS_ERROR_ONLY_BY_DEPENDENCY, "Operation refused, unit %s may be requested by dependency only (it is configured to refuse manual start/stop).", u->id);
-
-        /* dbus-broker issues StartUnit for activation requests, so let's apply the same check
-         * used in signal_activation_request(). */
-        if (type == JOB_START && u->type == UNIT_SERVICE &&
-            SERVICE(u)->type == SERVICE_DBUS && !manager_dbus_is_running(u->manager))
-                return sd_bus_error_set(error, BUS_ERROR_SHUTTING_DOWN, "Refusing activation, D-Bus is not running.");
+                return sd_bus_error_setf(error,
+                                         BUS_ERROR_ONLY_BY_DEPENDENCY,
+                                         "Operation refused, unit %s may be requested by dependency only (it is configured to refuse manual start/stop).",
+                                         u->id);
+
+        /* dbus-broker issues StartUnit for activation requests, and Type=dbus services automatically
+         * gain dependency on dbus.socket. Therefore, if dbus has a pending stop job, the new start
+         * job that pulls in dbus again would cause job type conflict. Let's avoid that by rejecting
+         * job enqueuing early.
+         *
+         * Note that unlike signal_activation_request(), we can't use unit_inactive_or_pending()
+         * here. StartUnit is a more generic interface, and thus users are allowed to use e.g. systemctl
+         * to start Type=dbus services even when dbus is inactive. */
+        if (type == JOB_START && u->type == UNIT_SERVICE && SERVICE(u)->type == SERVICE_DBUS)
+                FOREACH_STRING(dbus_unit, SPECIAL_DBUS_SOCKET, SPECIAL_DBUS_SERVICE) {
+                        Unit *dbus;
+
+                        dbus = manager_get_unit(u->manager, dbus_unit);
+                        if (dbus && unit_stop_pending(dbus))
+                                return sd_bus_error_setf(error,
+                                                         BUS_ERROR_SHUTTING_DOWN,
+                                                         "Operation for unit %s refused, D-Bus is shutting down.",
+                                                         u->id);
+                }
 
         r = sd_bus_message_new_method_return(message, &reply);
         if (r < 0)
diff --git a/src/core/dbus.c b/src/core/dbus.c
index 2f5feeaec3..c41e1a6c74 100644
--- a/src/core/dbus.c
+++ b/src/core/dbus.c
@@ -160,8 +160,9 @@ static int signal_activation_request(sd_bus_message *message, void *userdata, sd
                 return 0;
         }
 
-        if (!manager_dbus_is_running(m)) {
-                r = sd_bus_error_set(&error, BUS_ERROR_SHUTTING_DOWN, "Refusing activation, D-Bus is not running.");
+        if (manager_unit_inactive_or_pending(m, SPECIAL_DBUS_SOCKET) ||
+            manager_unit_inactive_or_pending(m, SPECIAL_DBUS_SERVICE)) {
+                r = sd_bus_error_set(&error, BUS_ERROR_SHUTTING_DOWN, "Refusing activation, D-Bus is shutting down.");
                 goto failed;
         }
 
diff --git a/src/core/manager.c b/src/core/manager.c
index ec8851ca22..36881de479 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1801,7 +1801,7 @@ static void manager_distribute_fds(Manager *m, FDSet *fds) {
         }
 }
 
-bool manager_dbus_is_running_full(Manager *m, bool deserialized) {
+static bool manager_dbus_is_running(Manager *m, bool deserialized) {
         Unit *u;
 
         assert(m);
@@ -1843,7 +1843,7 @@ static void manager_setup_bus(Manager *m) {
                 (void) bus_init_system(m);
 
         /* Let's connect to the bus now, but only if the unit is supposed to be up */
-        if (manager_dbus_is_running_full(m, MANAGER_IS_RELOADING(m))) {
+        if (manager_dbus_is_running(m, MANAGER_IS_RELOADING(m))) {
                 (void) bus_init_api(m);
 
                 if (MANAGER_IS_SYSTEM(m))
@@ -2935,7 +2935,7 @@ static int manager_dispatch_signal_fd(sd_event_source *source, int fd, uint32_t
                 break;
 
         case SIGUSR1:
-                if (manager_dbus_is_running(m)) {
+                if (manager_dbus_is_running(m, false)) {
                         log_info("Trying to reconnect to bus...");
 
                         (void) bus_init_api(m);
@@ -4140,7 +4140,7 @@ void manager_recheck_dbus(Manager *m) {
         if (MANAGER_IS_RELOADING(m))
                 return; /* don't check while we are reloading… */
 
-        if (manager_dbus_is_running(m)) {
+        if (manager_dbus_is_running(m, false)) {
                 (void) bus_init_api(m);
 
                 if (MANAGER_IS_SYSTEM(m))
diff --git a/src/core/manager.h b/src/core/manager.h
index 951d46222e..486eda11b6 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -504,11 +504,6 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
 
 int manager_startup(Manager *m, FILE *serialization, FDSet *fds, const char *root);
 
-bool manager_dbus_is_running_full(Manager *m, bool deserialized);
-static inline bool manager_dbus_is_running(Manager *m) {
-        return manager_dbus_is_running_full(m, false);
-}
-
 Job *manager_get_job(Manager *m, uint32_t id);
 Unit *manager_get_unit(Manager *m, const char *name);