* [PATCH 1/2] spawn: try to keep signals blocked in spawned child
@ 2016-06-18 21:54 4% ` Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2016-06-18 21:54 UTC (permalink / raw)
To: meta
While we only want to stop our daemons and gracefully destroy
subprocesses, it is common for 'Ctrl-C' from a terminal to kill
the entire pgroup.
Killing an entire pgroup nukes subprocesses like git-upload-pack
breaks graceful shutdown on long clones. Make a best effort to
ensure git-upload-pack processes are not broken when somebody
signals an entire process group.
Followup-to: commit 37bf2db81bbbe114d7fc5a00e30d3d5a6fa74de5
("doc: systemd examples should only kill one process")
---
lib/PublicInbox/Spawn.pm | 8 +++++---
lib/PublicInbox/SpawnPP.pm | 7 ++++++-
t/spawn.t | 2 +-
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index 66dce33..8373030 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -104,9 +104,11 @@ int public_inbox_fork_exec(int in, int out, int err,
REDIR(out, 1);
REDIR(err, 2);
for (sig = 1; sig < NSIG; sig++)
- signal(sig, SIG_DFL); /* ignore errorrs on signals */
- ret = sigprocmask(SIG_SETMASK, &old, NULL);
- if (ret != 0) xerr("sigprocmask failed in vfork child");
+ signal(sig, SIG_DFL); /* ignore errors on signals */
+ /*
+ * don't bother unblocking, we don't want signals
+ * to the group taking out a subprocess
+ */
execve(filename, argv, envp);
xerr("execve failed");
}
diff --git a/lib/PublicInbox/SpawnPP.pm b/lib/PublicInbox/SpawnPP.pm
index fe95d12..36223e8 100644
--- a/lib/PublicInbox/SpawnPP.pm
+++ b/lib/PublicInbox/SpawnPP.pm
@@ -3,11 +3,15 @@
package PublicInbox::SpawnPP;
use strict;
use warnings;
-use POSIX qw(dup2);
+use POSIX qw(dup2 :signal_h);
# Pure Perl implementation for folks that do not use Inline::C
sub public_inbox_fork_exec ($$$$$$) {
my ($in, $out, $err, $f, $cmd, $env) = @_;
+ my $old = POSIX::SigSet->new();
+ my $set = POSIX::SigSet->new();
+ $set->fillset or die "fillset failed: $!";
+ sigprocmask(SIG_SETMASK, $set, $old) or die "can't block signals: $!";
my $pid = fork;
if ($pid == 0) {
if ($in != 0) {
@@ -29,6 +33,7 @@ sub public_inbox_fork_exec ($$$$$$) {
die "exec $cmd->[0] failed: $!\n";
}
}
+ sigprocmask(SIG_SETMASK, $old) or die "can't unblock signals: $!";
$pid;
}
diff --git a/t/spawn.t b/t/spawn.t
index 9e58f67..0f75646 100644
--- a/t/spawn.t
+++ b/t/spawn.t
@@ -87,7 +87,7 @@ use PublicInbox::Spawn qw(which spawn popen_rd);
is(kill(0, $pid), 1, 'child process is running');
ok(!defined(sysread($fh, my $buf, 1)) && $!{EAGAIN},
'sysread returned quickly with EAGAIN');
- is(kill(15, $pid), 1, 'child process killed early');
+ is(kill(9, $pid), 1, 'child process killed early');
is(waitpid($pid, 0), $pid, 'child process reapable');
isnt($?, 0, '$? set properly: '.$?);
}
^ permalink raw reply related [relevance 4%]
* [PATCH 2/1] doc: systemd examples should only kill one process
@ 2016-06-13 23:01 7% ` Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2016-06-13 23:01 UTC (permalink / raw)
To: meta
For our daemons, killing only the master process is enough.
Killing the entire control group (as done by default in
systemd) may cause subprocesses such as git to shut down
unexpectedly.
Having systemd kill workers directly will also cause an
immediate shutdown since the master would've already signaled
the workers; and workers will die after two shutdown requests.
---
examples/public-inbox-httpd@.service | 1 +
examples/public-inbox-nntpd@.service | 1 +
examples/unsubscribe-psgi@.service | 1 +
3 files changed, 3 insertions(+)
diff --git a/examples/public-inbox-httpd@.service b/examples/public-inbox-httpd@.service
index 3bb7072..6222de5 100644
--- a/examples/public-inbox-httpd@.service
+++ b/examples/public-inbox-httpd@.service
@@ -24,6 +24,7 @@ User = nobody
Group = nogroup
ExecReload = /bin/kill -HUP $MAINPID
TimeoutStopSec = 3600
+KillMode = process
[Install]
WantedBy = multi-user.target
diff --git a/examples/public-inbox-nntpd@.service b/examples/public-inbox-nntpd@.service
index 078e920..3e203e0 100644
--- a/examples/public-inbox-nntpd@.service
+++ b/examples/public-inbox-nntpd@.service
@@ -26,6 +26,7 @@ User = nobody
Group = nogroup
ExecReload = /bin/kill -HUP $MAINPID
TimeoutStopSec = 3600
+KillMode = process
[Install]
WantedBy = multi-user.target
diff --git a/examples/unsubscribe-psgi@.service b/examples/unsubscribe-psgi@.service
index 2dc4270..acc29e8 100644
--- a/examples/unsubscribe-psgi@.service
+++ b/examples/unsubscribe-psgi@.service
@@ -15,6 +15,7 @@ ExecStart = /usr/local/bin/public-inbox-httpd -W0 /etc/unsubscribe.psgi
Sockets = unsubscribe-psgi.socket
# we need to modify the mlmmj spool
User = mlmmj
+KillMode = process
[Install]
WantedBy = multi-user.target
^ permalink raw reply related [relevance 7%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2016-06-13 21:38 [PATCH] examples: systemd socket and service definitions for daemons Eric Wong
2016-06-13 23:01 7% ` [PATCH 2/1] doc: systemd examples should only kill one process Eric Wong
2016-06-18 21:54 [PATCH 0/2] graceful shutdown tweaks Eric Wong
2016-06-18 21:54 4% ` [PATCH 1/2] spawn: try to keep signals blocked in spawned child Eric Wong
Code repositories for project(s) associated with this public inbox
https://80x24.org/public-inbox.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).