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-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 5C1012007F for ; Tue, 2 Feb 2021 11:47:03 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 09/16] lei q: do not leave temporary files after oneshot exit Date: Tue, 2 Feb 2021 11:46:55 +0000 Message-Id: <20210202114702.29886-10-e@80x24.org> In-Reply-To: <20210202114702.29886-1-e@80x24.org> References: <20210202114702.29886-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Avoid on-stack shortcuts which may prevent destructors from firing since we're not inside the event loop. We'll also tidy up the unlink mechanism in LeiOverview while we're at it. --- lib/PublicInbox/LEI.pm | 20 +++++++++++--------- lib/PublicInbox/LeiOverview.pm | 7 +++---- lib/PublicInbox/LeiQuery.pm | 4 ++-- lib/PublicInbox/LeiXSearch.pm | 5 +++-- xt/lei-sigpipe.t | 27 +++++++++++++++++++++++++-- 5 files changed, 44 insertions(+), 19 deletions(-) diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index d6fa814c..44afced3 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -284,20 +284,22 @@ sub x_it ($$) { dump_and_clear_log(); if (my $sock = $self->{sock}) { send($sock, "x_it $code", MSG_EOR); - } elsif (!$self->{oneshot}) { - return; # client disconnected, noop - } elsif (my $signum = ($code & 127)) { # usually SIGPIPE (13) - $SIG{PIPE} = 'DEFAULT'; # $SIG{$signum} doesn't work - kill $signum, $$; - sleep; # wait for signal - } else { + } elsif ($self->{oneshot}) { # don't want to end up using $? from child processes for my $f (qw(lxs l2m)) { my $wq = delete $self->{$f} or next; $wq->DESTROY; } - $quit->($code >> 8); - } + # cleanup anything that has tempfiles + delete @$self{qw(ovv dedupe)}; + if (my $signum = ($code & 127)) { # usually SIGPIPE (13) + $SIG{PIPE} = 'DEFAULT'; # $SIG{$signum} doesn't work + kill $signum, $$; + sleep; # wait for signal + } else { + $quit->($code >> 8); + } + } # else ignore if client disconnected } sub err ($;@) { diff --git a/lib/PublicInbox/LeiOverview.pm b/lib/PublicInbox/LeiOverview.pm index 1d62ffe2..31cc67f1 100644 --- a/lib/PublicInbox/LeiOverview.pm +++ b/lib/PublicInbox/LeiOverview.pm @@ -26,16 +26,15 @@ sub _iso8601 ($) { strftime('%Y-%m-%dT%H:%M:%SZ', gmtime($_[0])) } # we open this in the parent process before ->wq_do handoff sub ovv_out_lk_init ($) { my ($self) = @_; - $self->{tmp_lk_id} = "$self.$$"; my $tmp = File::Temp->new("lei-ovv.dst.$$.lock-XXXXXX", TMPDIR => 1, UNLINK => 0); - $self->{lock_path} = $tmp->filename; + $self->{"lk_id.$self.$$"} = $self->{lock_path} = $tmp->filename; } sub ovv_out_lk_cancel ($) { my ($self) = @_; - ($self->{tmp_lk_id}//'') eq "$self.$$" and - unlink(delete($self->{lock_path})); + my $lock_path = delete $self->{"lk_id.$self.$$"} or return; + unlink($lock_path); } sub detect_fmt ($$) { diff --git a/lib/PublicInbox/LeiQuery.pm b/lib/PublicInbox/LeiQuery.pm index 7c1e3606..ca214ca1 100644 --- a/lib/PublicInbox/LeiQuery.pm +++ b/lib/PublicInbox/LeiQuery.pm @@ -54,7 +54,7 @@ sub lei_q { return $self->fail('no local or remote inboxes to search'); } my $xj = $lxs->concurrency($opt); - my $ovv = PublicInbox::LeiOverview->new($self) or return; + PublicInbox::LeiOverview->new($self) or return; $self->atfork_prepare_wq($lxs); $lxs->wq_workers_start('lei_xsearch', $xj, $self->oldset); delete $lxs->{-ipc_atfork_child_close}; @@ -90,7 +90,7 @@ sub lei_q { # descending docid order $mset_opt{relevance} //= -2 if $opt->{thread}; $self->{mset_opt} = \%mset_opt; - $ovv->ovv_begin($self); + $self->{ovv}->ovv_begin($self); $lxs->do_query($self); } diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm index e997431f..b3cace74 100644 --- a/lib/PublicInbox/LeiXSearch.pm +++ b/lib/PublicInbox/LeiXSearch.pm @@ -387,8 +387,9 @@ sub query_prepare { # called by wq_do sub fail_handler ($;$$) { my ($lei, $code, $io) = @_; - if (my $lxs = delete $lei->{lxs}) { - $lxs->wq_wait_old($lei) if $lxs->wq_kill_old; # lei-daemon + for my $f (qw(lxs l2m)) { + my $wq = delete $lei->{$f} or next; + $wq->wq_wait_old($lei) if $wq->wq_kill_old; # lei-daemon } close($io) if $io; # needed to avoid warnings on SIGPIPE $lei->x_it($code // (1 >> 8)); diff --git a/xt/lei-sigpipe.t b/xt/lei-sigpipe.t index 1aa9ed07..ba2d23c8 100644 --- a/xt/lei-sigpipe.t +++ b/xt/lei-sigpipe.t @@ -29,7 +29,30 @@ my $do_test = sub { } }; -$do_test->(); -$do_test->({XDG_RUNTIME_DIR => '/dev/null'}); +my ($tmp, $for_destroy) = tmpdir(); +my $pid; +my $opt = { run_mode => 0, 1 => \(my $out = '') }; +if (run_script([qw(lei daemon-pid)], undef, $opt)) { + chomp($pid = $out); + mkdir "$tmp/d" or BAIL_OUT $!; + local $ENV{TMPDIR} = "$tmp/d"; + $do_test->(); + $out = ''; + ok(run_script([qw(lei daemon-pid)], undef, $opt), 'daemon-pid again'); + chomp($out); + is($out, $pid, 'daemon-pid unchanged'); + ok(kill(0, $pid), 'daemon still running'); + $out = ''; +} +{ + mkdir "$tmp/1" or BAIL_OUT $!; + local $ENV{TMPDIR} = "$tmp/1"; + $do_test->({XDG_RUNTIME_DIR => '/dev/null'}); + is(unlink(glob("$tmp/1/*")), 0, 'nothing left over w/ oneshot'); +} + +# the one-shot test should be slow enough that the daemon has cleaned +# up in the background: +is_deeply([glob("$tmp/d/*")], [], 'nothing left over with daemon'); done_testing;