[PATCH 0/2] daemon: kill listener early at shutdown

[PATCH 0/2] daemon: kill listener early at shutdown

[Eric Wong]

Not everybody uses systemd (or similar) which allow socket
activation.  So we now get rid of listeners ASAP during graceful
shutdown to ensure replacement processes can bind to the same
listen socket w/o requiring SO_REUSEPORT.

Eric Wong (2):
  daemon: shorten lifetime of listener_names mapping
  daemon: drop listeners early in master on graceful shutdown

 lib/PublicInbox/Daemon.pm | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

[PATCH 1/2] daemon: shorten lifetime of listener_names mapping

[Eric Wong]

Keeping a ref to the IO::Socket handle was preventing
close(2) from being invoked on graceful shutdown of
worker.
---
 lib/PublicInbox/Daemon.pm | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index dd9e7780..842ff1cc 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -25,7 +25,6 @@ my (@cfg_listen, $stdout, $stderr, $group, $user, $pid_file, $daemonize);
 my $worker_processes = 1;
 my @listeners;
 my %pids;
-my %listener_names; # sockname => IO::Handle
 my %tls_opt; # scheme://sockname => args for IO::Socket::SSL->start_SSL
 my $reexec_pid;
 my ($uid, $gid);
@@ -77,6 +76,7 @@ sub sig_setmask { sigprocmask(SIG_SETMASK, @_) or die "sigprocmask: $!" }
 
 sub daemon_prepare ($) {
 	my ($default_listen) = @_;
+	my $listener_names = {}; # sockname => IO::Handle
 	$oldset = POSIX::SigSet->new();
 	$newset = POSIX::SigSet->new();
 	$newset->fillset or die "fillset: $!";
@@ -99,11 +99,11 @@ sub daemon_prepare ($) {
 	if (defined $pid_file && $pid_file =~ /\.oldbin\z/) {
 		die "--pid-file cannot end with '.oldbin'\n";
 	}
-	@listeners = inherit();
+	@listeners = inherit($listener_names);
 
 	# allow socket-activation users to set certs once and not
 	# have to configure each socket:
-	my @inherited_names = keys(%listener_names) if defined($default_cert);
+	my @inherited_names = keys(%$listener_names) if defined($default_cert);
 
 	# ignore daemonize when inheriting
 	$daemonize = undef if scalar @listeners;
@@ -128,7 +128,7 @@ sub daemon_prepare ($) {
 		}
 		# TODO: use scheme to load either NNTP.pm or HTTP.pm
 
-		next if $listener_names{$l}; # already inherited
+		next if $listener_names->{$l}; # already inherited
 		my (%o, $sock_pkg);
 		if (index($l, '/') == 0) {
 			$sock_pkg = 'IO::Socket::UNIX';
@@ -159,7 +159,7 @@ sub daemon_prepare ($) {
 		warn "error binding $l: $! ($@)\n" unless $s;
 		umask $prev;
 		if ($s) {
-			$listener_names{sockname($s)} = $s;
+			$listener_names->{sockname($s)} = $s;
 			$s->blocking(0);
 			push @listeners, $s;
 		}
@@ -353,7 +353,8 @@ sub host_with_port ($) {
 	$@ ? ('127.0.0.1', 0) : ($host, $port);
 }
 
-sub inherit () {
+sub inherit ($) {
+	my ($listener_names) = @_;
 	return () if ($ENV{LISTEN_PID} || 0) != $$;
 	my $fds = $ENV{LISTEN_FDS} or return ();
 	my $end = $fds + 2; # LISTEN_FDS_START - 1
@@ -369,7 +370,7 @@ Set 'NonBlocking = true' in the systemd.service unit to avoid stalled
 processes when multiple service instances start.
 
 			}
-			$listener_names{$k} = $s;
+			$listener_names->{$k} = $s;
 			push @rv, $s;
 		} else {
 			warn "failed to inherit fd=$fd (LISTEN_FDS=$fds)";

[PATCH 2/2] daemon: drop listeners early in master on graceful shutdown

[Eric Wong]

For users not relying on socket activation via systemd (or
similar), we want to drop listeners ASAP so another process
can bind to their address.  While we're at it, disable
TTIN and HUP handlers since we have no chance of starting
usable workers without listeners.
---
 lib/PublicInbox/Daemon.pm | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 842ff1cc..9db472a1 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -462,23 +462,27 @@ sub unlink_pid_file_safe_ish ($$) {
 	}
 }
 
+sub master_quit ($) {
+	exit unless @listeners;
+	@listeners = ();
+	kill_workers($_[0]);
+}
+
 sub master_loop {
 	pipe(my ($p0, $p1)) or die "failed to create parent-pipe: $!";
 	# 1031: F_SETPIPE_SZ, 4096: page size
 	fcntl($p1, 1031, 4096) if $^O eq 'linux';
 	my $set_workers = $worker_processes;
 	reopen_logs();
-	my $quit = 0;
 	my $ignore_winch;
-	my $quit_cb = sub { exit if $quit++; kill_workers($_[0]) };
 	my $sig = {
 		USR1 => sub { reopen_logs(); kill_workers($_[0]); },
 		USR2 => \&upgrade,
-		QUIT => $quit_cb,
-		INT => $quit_cb,
-		TERM => $quit_cb,
+		QUIT => \&master_quit,
+		INT => \&master_quit,
+		TERM => \&master_quit,
 		WINCH => sub {
-			return if $ignore_winch;
+			return if $ignore_winch || !@listeners;
 			if (-t STDIN || -t STDOUT || -t STDERR) {
 				$ignore_winch = 1;
 				warn <<EOF;
@@ -489,10 +493,12 @@ EOF
 			}
 		},
 		HUP => sub {
+			return unless @listeners;
 			$worker_processes = $set_workers;
 			kill_workers($_[0]);
 		},
 		TTIN => sub {
+			return unless @listeners;
 			if ($set_workers > $worker_processes) {
 				++$worker_processes;
 			} else {
@@ -509,7 +515,7 @@ EOF
 	sig_setmask($oldset) if !$sigfd;
 	while (1) { # main loop
 		my $n = scalar keys %pids;
-		if ($quit) {
+		unless (@listeners) {
 			exit if $n == 0;
 			$set_workers = $worker_processes = $n = 0;
 		}