Hey all,

I'm currently reviewing our Debian patches for systemd, and came
across this one which sounds important for other distributions, too.
This was reported and fixed two years ago in
https://bugs.debian.org/635777 which has all the details and logs, but
the summary is:

Distributions have quite a lot of "run scripts in this .d/ directory
if stuff happens"; e. g. the ISC DHCP client has
/etc/dhcp/dhclient-{enter,exit}-hooks.d/, Debian's ifupdown has
/etc/network/if-{up,down}.d/, and of course init.d scripts themselves
also occasionally call "service foo reload" and similar. It can happen
that when requesting a shutdown, a script of the above kind reloads or
restarts another service. In this case, postfix wants to reload itself
after a network interface goes up or down; during runtime that works
fine, but if it happens during shutdown, that "systemctl reload
postfix" will cause the entire shutdown to stall for 90s (the default
timeout).

Michael Stapelberg wrote the attached patch which fixes this by
disallowing unit reloads/restarts when final.target is queued. This is
admittedly not very elegant as it hardcodes the "final.target" name
(and also, for upstream the comment should be adjusted of course), but
it does fix the issue. I can still reproduce the problem with 218 in a
VM.

Is this something which we can fix upstream in a more elegant manner?

Thanks,

Martin

-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
From: Tollef Fog Heen <tfh...@err.no>
Date: Tue, 16 Oct 2012 18:39:27 +0200
Subject: Avoid reloading services when shutting down

Doing so won't work and makes no sense.  Thanks to Michael Stapelberg
for the patch.  Closes: #624599.
---
 src/core/manager.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/src/core/manager.c b/src/core/manager.c
index 6382400..acef626 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1147,6 +1147,8 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
 int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool override, sd_bus_error *e, Job **_ret) {
         int r;
         Transaction *tr;
+        Job *j;
+        Iterator i;
 
         assert(m);
         assert(type < _JOB_TYPE_MAX);
@@ -1156,6 +1158,29 @@ int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, bool ove
         if (mode == JOB_ISOLATE && type != JOB_START)
                 return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, "Isolate is only valid for start.");
 
+        if (type == JOB_RELOAD || type == JOB_RELOAD_OR_START || type == JOB_RESTART || type == JOB_TRY_RESTART) {
+                HASHMAP_FOREACH(j, m->jobs, i) {
+                        assert(j->installed);
+
+                        /* If final.target is queued (happens on poweroff, reboot and
+                         * halt), we will not accept new reload jobs. They would not be
+                         * executed ever anyways (since the shutdown comes first), but
+                         * they block the shutdown process: when systemd tries to stop
+                         * a unit such as ifup@eth0.service, that unit might invoke a
+                         * systemctl reload command, which blockingly waits (but only
+                         * gets executed after all other queued units for the shutdown
+                         * have been executed).
+                         *
+                         * See http://bugs.debian.org/624599 and
+                         *     http://bugs.debian.org/635777 */
+                        if (strcmp(j->unit->id, "final.target") == 0) {
+                                log_debug("final.target is queued, ignoring %s request for unit %s", job_type_to_string(type), unit->id);
+                                sd_bus_error_setf(e, BUS_ERROR_SHUTTING_DOWN, "final.target is queued, ignoring %s request for unit %s", job_type_to_string(type), unit->id);
+                                return -EINVAL;
+                        }
+                }
+        }
+
         if (mode == JOB_ISOLATE && !unit->allow_isolate)
                 return sd_bus_error_setf(e, BUS_ERROR_NO_ISOLATION, "Operation refused, unit may not be isolated.");
 

Attachment: signature.asc
Description: Digital signature

_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to