From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id BAB911F487 for ; Sun, 22 Mar 2020 08:58:49 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 1/2] daemon: fix SIGUSR2 upgrade with -W0 (no workers) Date: Sun, 22 Mar 2020 08:58:48 +0000 Message-Id: <20200322085849.14360-2-e@yhbt.net> In-Reply-To: <20200322085849.14360-1-e@yhbt.net> References: <20200322085849.14360-1-e@yhbt.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Disabling workers via `-W0' blesses the contents of the @listeners array, so we need to ensure we call fcntl on the GLOB ref in ->{sock}. Add tests to ensure USR2 works regardless of whether workers are enabled or not. --- lib/PublicInbox/Daemon.pm | 3 ++ t/httpd-unix.t | 99 +++++++++++++++++++++++++++++++++++---- 2 files changed, 92 insertions(+), 10 deletions(-) diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm index 43ef2691..3d582e35 100644 --- a/lib/PublicInbox/Daemon.pm +++ b/lib/PublicInbox/Daemon.pm @@ -403,6 +403,9 @@ sub upgrade { # $_[0] = signal name or number (unused) $ENV{LISTEN_FDS} = scalar @listeners; $ENV{LISTEN_PID} = $$; foreach my $s (@listeners) { + # @listeners are globs with workers, PI::L w/o workers + $s = $s->{sock} if ref($s) eq 'PublicInbox::Listener'; + my $fl = fcntl($s, F_GETFD, 0); fcntl($s, F_SETFD, $fl &= ~FD_CLOEXEC); } diff --git a/t/httpd-unix.t b/t/httpd-unix.t index b321c789..425ecfff 100644 --- a/t/httpd-unix.t +++ b/t/httpd-unix.t @@ -6,6 +6,8 @@ use warnings; use Test::More; use PublicInbox::TestCommon; use Errno qw(EADDRINUSE); +use Cwd qw(abs_path); +use Carp qw(croak); require_mods(qw(Plack::Util Plack::Builder HTTP::Date HTTP::Status)); use IO::Socket::UNIX; my ($tmpdir, $for_destroy) = tmpdir(); @@ -79,9 +81,26 @@ check_sock($unix); ok(-S $unix, 'unix socket still exists'); } +sub delay_until { + my $cond = shift; + for (1..1000) { + return if $cond->(); + select undef, undef, undef, 0.012; + } + Carp::croak('condition failed'); +} + SKIP: { require_mods('Net::Server::Daemonize', 20); my $pid_file = "$tmpdir/pid"; + my $read_pid = sub { + my $f = shift; + open my $fh, '<', $f or die "open $f failed: $!"; + my $pid = do { local $/; <$fh> }; + chomp $pid; + $pid || 0; + }; + for my $w (qw(-W0 -W1)) { # wait for daemonization $spawn_httpd->("-l$unix", '-D', '-P', $pid_file, $w); @@ -90,18 +109,78 @@ SKIP: { check_sock($unix); ok(-f $pid_file, "$w pid file written"); - open my $fh, '<', "$tmpdir/pid" or die "open failed: $!"; - my $rpid = do { local $/; <$fh> }; - chomp $rpid; - like($rpid, qr/\A\d+\z/s, "$w pid file looks like a pid"); - is(kill('TERM', $rpid), 1, "signaled daemonized $w process"); - for (1..100) { - kill(0, $rpid) or last; - select undef, undef, undef, 0.02; - } - is(kill(0, $rpid), 0, "daemonized $w process exited"); + my $pid = $read_pid->($pid_file); + is(kill('TERM', $pid), 1, "signaled daemonized $w process"); + delay_until(sub { !kill(0, $pid) }); + is(kill(0, $pid), 0, "daemonized $w process exited"); ok(!-e $pid_file, "$w pid file unlinked at exit"); } + + # try a USR2 upgrade with workers: + my $httpd = abs_path('blib/script/public-inbox-httpd'); + $psgi = abs_path($psgi); + my $opt = { run_mode => 0 }; + + my @args = ("-l$unix", '-D', '-P', $pid_file, -1, $out, -2, $err); + $td = start_script([$httpd, @args, $psgi], undef, $opt); + $td->join; + is($?, 0, "daemonized process again"); + check_sock($unix); + my $pid = $read_pid->($pid_file); + kill('USR2', $pid) or die "USR2 failed: $!"; + delay_until(sub { + $pid != (eval { $read_pid->($pid_file) } // $pid) + }); + my $new_pid = $read_pid->($pid_file); + isnt($new_pid, $pid, 'new child started'); + my $old_pid = $read_pid->("$pid_file.oldbin"); + is($old_pid, $pid, '.oldbin pid file written'); + + # first, back out of the upgrade + kill('QUIT', $new_pid) or die "kill new PID failed: $!"; + delay_until(sub { + $pid == (eval { $read_pid->($pid_file) } // 0) + }); + is($read_pid->($pid_file), $pid, 'old PID file restored'); + ok(!-f "$pid_file.oldbin", '.oldbin PID file gone'); + + # retry USR2 upgrade + kill('USR2', $pid) or die "USR2 failed: $!"; + delay_until(sub { + $pid != (eval { $read_pid->($pid_file) } // $pid) + }); + $new_pid = $read_pid->($pid_file); + isnt($new_pid, $pid, 'new child started again'); + $old_pid = $read_pid->("$pid_file.oldbin"); + is($old_pid, $pid, '.oldbin pid file written'); + + # drop the old parent + kill('QUIT', $old_pid) or die "QUIT failed: $!"; + delay_until(sub { !kill(0, $old_pid) }); + + # drop the new child + check_sock($unix); + kill('QUIT', $new_pid) or die "QUIT failed: $!"; + delay_until(sub { !kill(0, $new_pid) }); + ok(!-f $pid_file, 'PID file is gone'); + + + # try USR2 without workers (-W0) + $td = start_script([$httpd, @args, '-W0', $psgi], undef, $opt); + $td->join; + is($?, 0, 'daemonized w/o workers'); + check_sock($unix); + $pid = $read_pid->($pid_file); + + # replace running process + kill('USR2', $pid) or die "USR2 failed: $!"; + delay_until(sub { !kill(0, $pid) }); + + check_sock($unix); + $pid = $read_pid->($pid_file); + kill('QUIT', $pid) or die "USR2 failed: $!"; + delay_until(sub { !kill(0, $pid) }); + ok(!-f $pid_file, 'PID file is gone'); } done_testing();