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, URIBL_BLOCKED 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 723B61F46D for ; Thu, 12 Dec 2019 21:16:50 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 3/3] daemon: use DESTROY for unlinking --pid-file Date: Thu, 12 Dec 2019 21:16:49 +0000 Message-Id: <20191212211649.7824-4-e@80x24.org> In-Reply-To: <20191212211649.7824-1-e@80x24.org> References: <20191212211649.7824-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: This gets rid of the last "END{}" block in our code and cleans up a (temporary) circular reference. Furthermore, ensure the cleanup code still works in all configurations by adding tests and testing both the -W1 (default, 1 worker) and -W0 (no workers) code paths. --- lib/PublicInbox/Daemon.pm | 25 ++++++++++++------------ t/httpd-unix.t | 41 ++++++++++++++++++++------------------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm index c2c05b96..dd9e7780 100644 --- a/lib/PublicInbox/Daemon.pm +++ b/lib/PublicInbox/Daemon.pm @@ -28,10 +28,8 @@ my %pids; my %listener_names; # sockname => IO::Handle my %tls_opt; # scheme://sockname => args for IO::Socket::SSL->start_SSL my $reexec_pid; -my $cleanup; my ($uid, $gid); my ($default_cert, $default_key); -END { $cleanup->() if $cleanup }; my %KNOWN_TLS = ( 443 => 'https', 563 => 'nntps' ); my %KNOWN_STARTTLS = ( 119 => 'nntp' ); @@ -245,14 +243,11 @@ sub daemonize () { die "could not fork: $!\n" unless defined $pid; exit if $pid; } - if (defined $pid_file) { - write_pid($pid_file); - my $unlink_pid = $$; - $cleanup = sub { - $cleanup = undef; # avoid cyclic reference - unlink_pid_file_safe_ish($unlink_pid, $pid_file); - }; - } + return unless defined $pid_file; + + write_pid($pid_file); + # for ->DESTROY: + bless { pid => $$, pid_file => $pid_file }, __PACKAGE__; } sub worker_quit { # $_[0] = signal name or number (unused) @@ -631,16 +626,16 @@ sub daemon_loop ($$$$) { PublicInbox::DS->SetLoopTimeout(1000); } PublicInbox::DS->EventLoop; - $parent_pipe = undef; } - sub run ($$$;$) { my ($default, $refresh, $post_accept, $nntpd) = @_; daemon_prepare($default); my $af_default = $default =~ /:8080\z/ ? 'httpready' : undef; - daemonize(); + my $for_destroy = daemonize(); daemon_loop($refresh, $post_accept, $nntpd, $af_default); + PublicInbox::DS->Reset; + # ->DESTROY runs when $for_destroy goes out-of-scope } sub do_chown ($) { @@ -656,4 +651,8 @@ sub write_pid ($) { do_chown($path); } +sub DESTROY { + unlink_pid_file_safe_ish($_[0]->{pid}, $_[0]->{pid_file}); +} + 1; diff --git a/t/httpd-unix.t b/t/httpd-unix.t index ceec127c..2c8f8d6b 100644 --- a/t/httpd-unix.t +++ b/t/httpd-unix.t @@ -22,7 +22,6 @@ my $td; my $spawn_httpd = sub { my (@args) = @_; - push @args, '-W0'; my $cmd = [ '-httpd', @args, "--stdout=$out", "--stderr=$err", $psgi ]; $td = start_script($cmd); }; @@ -36,7 +35,7 @@ my $spawn_httpd = sub { } ok(!-S $unix, 'UNIX socket does not exist, yet'); -$spawn_httpd->("-l$unix"); +$spawn_httpd->("-l$unix", '-W0'); my %o = (Peer => $unix, Type => SOCK_STREAM); for (1..1000) { last if -S $unix && IO::Socket::UNIX->new(%o); @@ -87,26 +86,28 @@ check_sock($unix); SKIP: { eval 'require Net::Server::Daemonize'; - skip('Net::Server missing for pid-file/daemonization test', 10) if $@; + skip('Net::Server missing for pid-file/daemonization test', 20) if $@; + my $pid_file = "$tmpdir/pid"; + for my $w (qw(-W0 -W1)) { + # wait for daemonization + $spawn_httpd->("-l$unix", '-D', '-P', $pid_file, $w); + $td->join; + is($?, 0, "daemonized $w process"); + check_sock($unix); - # wait for daemonization - $spawn_httpd->("-l$unix", '-D', '-P', "$tmpdir/pid"); - $td->join; - is($?, 0, 'daemonized process OK'); - check_sock($unix); - - ok(-f "$tmpdir/pid", 'pid file written'); - open my $fh, '<', "$tmpdir/pid" or die "open failed: $!"; - local $/ = "\n"; - my $rpid = <$fh>; - chomp $rpid; - like($rpid, qr/\A\d+\z/s, 'pid file looks like a pid'); - is(kill('TERM', $rpid), 1, 'signalled daemonized process'); - for (1..100) { - kill(0, $rpid) or last; - select undef, undef, undef, 0.02; + 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"); + ok(!-e $pid_file, "$w pid file unlinked at exit"); } - is(kill(0, $rpid), 0, 'daemonized process exited') } done_testing();