On Mon, 17 Apr 2017 at 12:05:41 +0200, Niels Thykier wrote:
> I have reviewed the nodm systemd service file plus the init.d script
> and I cannot see that it takes plymouth into account (which leads to
> #782456). To the best of my knowledge, #782456 have to be fixed in
> the DMs.
I tried preparing an NMU to address this. I did this as an NMU because
I was added to the Uploaders by the nodm maintainers during the last
upload, but I don't feel particularly responsible for this package,
and I have no commit access to the pkg-fso git repository where it is
maintained.
I've gone with what Niels requested rather than trying to over-analyze
what is going on. That matches what they do in Fedora, which is where
both Plymouth and systemd originated, so presumably they're getting
this more or less right.
While testing the resulting package, using openbox as a minimal "desktop
environment", I found that stopping nodm during shutdown does not work
as expected: it just hangs until systemd times out after 90 seconds.
I think this is a regression caused by the native systemd unit: systemd
sends SIGTERM to all processes in the service cgroup by default, then
waits up to 90 seconds and sends SIGKILL to any survivors, whereas
the LSB init script used start-stop-daemon --retry=10, which would
send SIGTERM to only the main process, wait up to 10 seconds and send
SIGKILL.
I've included a patch that makes the sysvinit service imitate the
LSB init script, which seems to resolve the 90 second hang. Niels, would
that be acceptable for an unblock, or do you want me to revert it?
Patches attached, also available from:
git fetch git+ssh://git.debian.org/git/users/smcv/nodm.git bug860463
Thanks,
S
>From a2faa9e91b37bb440f720030856d6bae20035c7f Mon Sep 17 00:00:00 2001
From: Simon McVittie <s...@debian.org>
Date: Wed, 26 Apr 2017 11:01:07 +0100
Subject: [PATCH 1/5] Order nodm.service after plymouth-quit.service
Closes: #860463 for systemd systems
---
debian/changelog | 8 +++++++
...e-Ask-Plymouth-to-stop-before-starting-no.patch | 27 ++++++++++++++++++++++
debian/patches/series | 1 +
3 files changed, 36 insertions(+)
create mode 100644 debian/patches/nodm.service-Ask-Plymouth-to-stop-before-starting-no.patch
create mode 100644 debian/patches/series
diff --git a/debian/changelog b/debian/changelog
index 7eadc35..2d13f5d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+nodm (0.13-2) UNRELEASED; urgency=medium
+
+ * d/p/nodm.service-Ask-Plymouth-to-stop-before-starting-no.patch:
+ Order nodm.service after plymouth-quit.service
+ (Closes: #860463 for systemd systems)
+
+ -- Simon McVittie <s...@debian.org> Wed, 26 Apr 2017 10:59:46 +0100
+
nodm (0.13-1) unstable; urgency=medium
* New upstream release.
diff --git a/debian/patches/nodm.service-Ask-Plymouth-to-stop-before-starting-no.patch b/debian/patches/nodm.service-Ask-Plymouth-to-stop-before-starting-no.patch
new file mode 100644
index 0000000..283211d
--- /dev/null
+++ b/debian/patches/nodm.service-Ask-Plymouth-to-stop-before-starting-no.patch
@@ -0,0 +1,27 @@
+From 6acf5fb593159dfc6f8a78d3e256901754e787c8 Mon Sep 17 00:00:00 2001
+From: Simon McVittie <s...@debian.org>
+Date: Sun, 23 Apr 2017 13:23:50 +0100
+Subject: [PATCH] nodm.service: Ask Plymouth to stop before starting nodm
+
+Bug-Debian: https://bugs.debian.org/860463
+Signed-off-by: Simon McVittie <s...@debian.org>
+---
+ nodm.service.in | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/nodm.service.in b/nodm.service.in
+index 638f919..09119f0 100644
+--- a/nodm.service.in
++++ b/nodm.service.in
+@@ -1,7 +1,7 @@
+ [Unit]
+ Description=Display manager for automatic session logins
+ Documentation=man:nodm(8)
+-After=systemd-user-sessions.service
++After=plymouth-quit.service systemd-user-sessions.service
+
+ [Service]
+ EnvironmentFile=-/etc/default/nodm
+--
+2.11.0
+
diff --git a/debian/patches/series b/debian/patches/series
new file mode 100644
index 0000000..33cce78
--- /dev/null
+++ b/debian/patches/series
@@ -0,0 +1 @@
+nodm.service-Ask-Plymouth-to-stop-before-starting-no.patch
--
2.11.0
>From 5ed832909dadc98286b80c31890fcc3b77911225 Mon Sep 17 00:00:00 2001
From: Simon McVittie <s...@debian.org>
Date: Wed, 26 Apr 2017 11:02:18 +0100
Subject: [PATCH 2/5] d/nodm.init: Ask Plymouth to stop before starting nodm
Closes: #860463 for non-systemd systems
---
debian/changelog | 2 ++
debian/nodm.init | 3 +++
2 files changed, 5 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index 2d13f5d..495dc78 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -3,6 +3,8 @@ nodm (0.13-2) UNRELEASED; urgency=medium
* d/p/nodm.service-Ask-Plymouth-to-stop-before-starting-no.patch:
Order nodm.service after plymouth-quit.service
(Closes: #860463 for systemd systems)
+ * d/nodm.init: Ask Plymouth to stop before starting nodm
+ (Closes: #860463 for non-systemd systems)
-- Simon McVittie <s...@debian.org> Wed, 26 Apr 2017 10:59:46 +0100
diff --git a/debian/nodm.init b/debian/nodm.init
index ba16fe8..d737641 100644
--- a/debian/nodm.init
+++ b/debian/nodm.init
@@ -51,6 +51,9 @@ export NODM_XINIT NODM_XSESSION NODM_X_OPTIONS NODM_USER NODM_MIN_SESSION_TIME N
case "$1" in
start)
+ if [ -x /bin/plymouth ]; then
+ /bin/plymouth quit
+ fi
[ "$VERBOSE" != no ] && log_daemon_msg "Starting $DESC" "$NAME"
if [ "$NODM_ENABLED" = "no" ] || [ "$NODM_ENABLED" = "false" ]
then
--
2.11.0
>From 80441008823f96deb5d8330f7f90b934f6cb86af Mon Sep 17 00:00:00 2001
From: Simon McVittie <s...@debian.org>
Date: Thu, 27 Apr 2017 18:27:33 +0100
Subject: [PATCH 3/5] Make systemd service shutdown sequence resemble LSB init
script
Kill main process with SIGTERM, then kill the whole service with SIGKILL
if it is still up after 10 seconds. This approximates the behaviour of
the LSB init script under systemd, and prevents a 90 second hang during
shutdown under some circumstances, when the SIGTERM signal sent to the
entire process group causes the main process to try to respawn the
child processes.
---
debian/changelog | 5 +++++
...e-Use-KillMode-mixed-with-10-second-timeo.patch | 25 ++++++++++++++++++++++
debian/patches/series | 1 +
3 files changed, 31 insertions(+)
create mode 100644 debian/patches/nodm.service-Use-KillMode-mixed-with-10-second-timeo.patch
diff --git a/debian/changelog b/debian/changelog
index 495dc78..eccc045 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -5,6 +5,11 @@ nodm (0.13-2) UNRELEASED; urgency=medium
(Closes: #860463 for systemd systems)
* d/nodm.init: Ask Plymouth to stop before starting nodm
(Closes: #860463 for non-systemd systems)
+ * d/p/nodm.service-Use-KillMode-mixed-with-10-second-timeo.patch:
+ Kill main process with SIGTERM, then kill the whole service with
+ SIGKILL if it is still up after 10 seconds. This approximates the
+ behaviour of the LSB init script under systemd, and prevents a
+ 90 second hang during shutdown under some circumstances.
-- Simon McVittie <s...@debian.org> Wed, 26 Apr 2017 10:59:46 +0100
diff --git a/debian/patches/nodm.service-Use-KillMode-mixed-with-10-second-timeo.patch b/debian/patches/nodm.service-Use-KillMode-mixed-with-10-second-timeo.patch
new file mode 100644
index 0000000..df17b01
--- /dev/null
+++ b/debian/patches/nodm.service-Use-KillMode-mixed-with-10-second-timeo.patch
@@ -0,0 +1,25 @@
+From: Simon McVittie <s...@debian.org>
+Date: Thu, 27 Apr 2017 18:26:38 +0100
+Subject: nodm.service: Use KillMode=mixed with 10 second timeout
+
+For a graceful exit, this should result in almost the same practical
+effect as the use of "--retry 10" in the LSB init script: send SIGTERM
+to main process, wait 10 seconds, send SIGKILL to any remaining
+processes. The difference is that the SIGKILL will catch the entire
+graphical session, not just the nodm processes.
+
+Signed-off-by: Simon McVittie <s...@debian.org>
+---
+ nodm.service.in | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/nodm.service.in b/nodm.service.in
+index 09119f0..0b7e95b 100644
+--- a/nodm.service.in
++++ b/nodm.service.in
+@@ -8,3 +8,5 @@ EnvironmentFile=-/etc/default/nodm
+ ExecStartPre=/usr/bin/test ${NODM_ENABLED} != no -a ${NODM_ENABLED} != false
+ ExecStart=@sbindir@/nodm $NODM_OPTIONS
+ Restart=always
++KillMode=mixed
++TimeoutStopSec=10
diff --git a/debian/patches/series b/debian/patches/series
index 33cce78..e6e7026 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,2 @@
nodm.service-Ask-Plymouth-to-stop-before-starting-no.patch
+nodm.service-Use-KillMode-mixed-with-10-second-timeo.patch
--
2.11.0
>From b46bdae5593116ee8c1d4c8379999a825133820a Mon Sep 17 00:00:00 2001
From: Simon McVittie <s...@debian.org>
Date: Thu, 27 Apr 2017 10:28:49 +0100
Subject: [PATCH 4/5] Remove myself from Uploaders
I do not intend to take long-term responsibility for this package.
---
debian/changelog | 2 ++
debian/control | 1 -
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/debian/changelog b/debian/changelog
index eccc045..5d88f7e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -10,6 +10,8 @@ nodm (0.13-2) UNRELEASED; urgency=medium
SIGKILL if it is still up after 10 seconds. This approximates the
behaviour of the LSB init script under systemd, and prevents a
90 second hang during shutdown under some circumstances.
+ * Remove myself from Uploaders. I do not intend to take long-term
+ responsibility for this package.
-- Simon McVittie <s...@debian.org> Wed, 26 Apr 2017 10:59:46 +0100
diff --git a/debian/control b/debian/control
index 0309a83..03b7839 100644
--- a/debian/control
+++ b/debian/control
@@ -6,7 +6,6 @@ Uploaders:
Joachim Breitner <nome...@debian.org>,
Enrico Zini <enr...@debian.org>,
Mike Gabriel <sunwea...@debian.org>,
- Simon McVittie <s...@debian.org>,
Build-Depends:
debhelper (>= 9),
dpkg-dev (>= 1.16.1.1),
--
2.11.0
>From be482af48557e75926ca965c07ac053c8267f99b Mon Sep 17 00:00:00 2001
From: Simon McVittie <s...@debian.org>
Date: Thu, 27 Apr 2017 10:29:32 +0100
Subject: [PATCH 5/5] Non-maintainer upload.
---
debian/changelog | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/debian/changelog b/debian/changelog
index 5d88f7e..8dc6834 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,4 +1,4 @@
-nodm (0.13-2) UNRELEASED; urgency=medium
+nodm (0.13-1.1) unstable; urgency=medium
* d/p/nodm.service-Ask-Plymouth-to-stop-before-starting-no.patch:
Order nodm.service after plymouth-quit.service
@@ -13,7 +13,7 @@ nodm (0.13-2) UNRELEASED; urgency=medium
* Remove myself from Uploaders. I do not intend to take long-term
responsibility for this package.
- -- Simon McVittie <s...@debian.org> Wed, 26 Apr 2017 10:59:46 +0100
+ -- Simon McVittie <s...@debian.org> Thu, 27 Apr 2017 18:37:16 +0100
nodm (0.13-1) unstable; urgency=medium
--
2.11.0
