[PATCH 0/4] daemon-related cleanups

[PATCH 0/4] daemon-related cleanups

[Eric Wong]

Nothing earth-shattering, but we need to start documenting
this for users if we expect people to start using it :p.

Eric Wong (4):
      t/httpd-corner: additional callback test
      daemon: drop listener sockets ASAP on termination
      daemon: avoid cyclic references for once-used callbacks
      daemon: simplify parent death handling

 lib/PublicInbox/Daemon.pm | 9 +++++++--
 t/httpd-corner.psgi       | 9 +++++++++
 t/httpd-corner.t          | 9 +++++++++
 3 files changed, 25 insertions(+), 2 deletions(-)

[PATCH 1/4] t/httpd-corner: additional callback test

[Eric Wong]

Just to ensure we hit the code path independently of
WWW code.
---
 t/httpd-corner.psgi | 9 +++++++++
 t/httpd-corner.t    | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/t/httpd-corner.psgi b/t/httpd-corner.psgi
index 349b35d..da8a2ee 100644
--- a/t/httpd-corner.psgi
+++ b/t/httpd-corner.psgi
@@ -47,6 +47,15 @@ my $app = sub {
 	} elsif ($path eq '/host-port') {
 		$code = 200;
 		push @$body, "$env->{REMOTE_ADDR}:$env->{REMOTE_PORT}";
+	} elsif ($path eq '/callback') {
+		return sub {
+			my ($res) = @_;
+			my $buf = "hello world\n";
+			push @$h, 'Content-Length', length($buf);
+			my $fh = $res->([200, $h]);
+			$fh->write($buf);
+			$fh->close;
+		}
 	}
 
 	[ $code, $h, $body ]
diff --git a/t/httpd-corner.t b/t/httpd-corner.t
index 1956407..a6238e4 100644
--- a/t/httpd-corner.t
+++ b/t/httpd-corner.t
@@ -76,6 +76,15 @@ my $spawn_httpd = sub {
 	$spawn_httpd->('-W0');
 }
 
+{
+	my $conn = conn_for($sock, 'streaming callback');
+	$conn->write("GET /callback HTTP/1.0\r\n\r\n");
+	ok($conn->read(my $buf, 8192), 'read response');
+	my ($head, $body) = split(/\r\n\r\n/, $buf);
+	is($body, "hello world\n", 'callback body matches expected');
+}
+
+
 # Unix domain sockets
 {
 	my $u = IO::Socket::UNIX->new(Type => SOCK_STREAM, Peer => $upath);
-- 
EW

[PATCH 2/4] daemon: drop listener sockets ASAP on termination

[Eric Wong]

We do not want to be accepting connections during graceful
shutdown because another new process is likely taking over.
This also allows us to free up the listener case another
(independent) process wants to claim it.
---
 lib/PublicInbox/Daemon.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index f0be034..c3199cd 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -137,6 +137,7 @@ sub worker_quit () {
 	# killing again terminates immediately:
 	exit unless @listeners;
 
+	$_->close foreach @listeners; # call Danga::Socket::close
 	@listeners = ();
 
 	# give slow clients 30s to finish reading/writing whatever
@@ -401,7 +402,9 @@ sub daemon_loop ($$) {
 	$SIG{USR1} = *reopen_logs;
 	$SIG{HUP} = $refresh;
 	# this calls epoll_create:
-	PublicInbox::Listener->new($_, $post_accept) for @listeners;
+	@listeners = map {
+		PublicInbox::Listener->new($_, $post_accept)
+	} @listeners;
 	Danga::Socket->EventLoop;
 	$parent_pipe = undef;
 }
-- 
EW

[PATCH 3/4] daemon: avoid cyclic references for once-used callbacks

[Eric Wong]

Not that these subs are repeatedly created, but this makes
the code easier-to-review and these callbacks are idempotent
anyways.
---
 lib/PublicInbox/Daemon.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index c3199cd..30411e1 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -109,6 +109,7 @@ sub daemonize () {
 	# The upgrade will create the ".oldbin" pid file in the
 	# same directory as the given pid file.
 	$uid and $set_user = sub {
+		$set_user = undef;
 		Net::Server::Daemonize::set_user($uid, $gid);
 	};
 
@@ -128,6 +129,7 @@ sub daemonize () {
 		write_pid($pid_file);
 		my $unlink_pid = $$;
 		$cleanup = sub {
+			$cleanup = undef; # avoid cyclic reference
 			unlink_pid_file_safe_ish($unlink_pid, $pid_file);
 		};
 	}
-- 
EW

[PATCH 4/4] daemon: simplify parent death handling

[Eric Wong]

No need to create a new sub which kill ourselves $$ when we can
invoke worker_quit directly.
---
 lib/PublicInbox/Daemon.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 30411e1..8a0af8d 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -391,7 +391,7 @@ sub daemon_loop ($$) {
 		$refresh->(); # preload by default
 		$parent_pipe = master_loop(); # returns if in child process
 		my $fd = fileno($parent_pipe);
-		Danga::Socket->AddOtherFds($fd => sub { kill('TERM', $$) } );
+		Danga::Socket->AddOtherFds($fd => *worker_quit);
 	} else {
 		reopen_logs();
 		$set_user->() if $set_user;
-- 
EW