* [PATCH 2/3] treewide: avoid getpid() for OnDestroy checks
2024-04-01 6:49 [PATCH 0/3] treewide: getpid() syscall reduction Eric Wong
2024-04-01 6:49 ` [PATCH 1/3] lock: get rid of PID guard Eric Wong
@ 2024-04-01 6:49 ` Eric Wong
2024-04-01 6:49 ` [PATCH 3/3] treewide: avoid getpid for more ownership checks Eric Wong
2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2024-04-01 6:49 UTC (permalink / raw)
To: meta
getpid() isn't cached by glibc nowadays and system calls are
more expensive due to CPU vulnerability mitigations. To
ensure we switch to the new semantics properly, introduce
a new `on_destroy' function to simplify callers.
Furthermore, most OnDestroy correctness is often tied to the
process which creates it, so make the new API default to
guarded against running in subprocesses.
For cases which require running in all children, a new
PublicInbox::OnDestroy::all call is provided.
---
lib/PublicInbox/CodeSearchIdx.pm | 29 +++++++++++++----------------
lib/PublicInbox/DS.pm | 9 +++++----
lib/PublicInbox/Daemon.pm | 12 ++++++------
lib/PublicInbox/IPC.pm | 12 +++++++-----
lib/PublicInbox/LEI.pm | 8 ++++----
lib/PublicInbox/LeiMirror.pm | 25 +++++++++++--------------
lib/PublicInbox/LeiP2q.pm | 2 +-
lib/PublicInbox/LeiStore.pm | 3 ++-
lib/PublicInbox/LeiTag.pm | 8 ++++----
lib/PublicInbox/Lock.pm | 4 ++--
lib/PublicInbox/MHreader.pm | 4 ++--
lib/PublicInbox/MboxLock.pm | 6 +++---
lib/PublicInbox/OnDestroy.pm | 27 +++++++++++++++++----------
lib/PublicInbox/SearchIdxShard.pm | 2 +-
lib/PublicInbox/SpawnPP.pm | 5 +++--
lib/PublicInbox/TestCommon.pm | 4 ++--
lib/PublicInbox/Umask.pm | 2 +-
lib/PublicInbox/ViewVCS.pm | 5 +++--
lib/PublicInbox/Watch.pm | 4 ++--
lib/PublicInbox/WwwCoderepo.pm | 2 +-
lib/PublicInbox/XapClient.pm | 2 +-
lib/PublicInbox/XapHelper.pm | 2 +-
lib/PublicInbox/Xapcmd.pm | 2 +-
script/public-inbox-init | 2 +-
t/lei-sigpipe.t | 4 +---
t/mbox_lock.t | 7 ++++---
t/mh_reader.t | 1 -
t/on_destroy.t | 25 ++++++++++++++++---------
xt/net_writer-imap.t | 4 ++--
29 files changed, 117 insertions(+), 105 deletions(-)
diff --git a/lib/PublicInbox/CodeSearchIdx.pm b/lib/PublicInbox/CodeSearchIdx.pm
index 570ff64f..6d777bf6 100644
--- a/lib/PublicInbox/CodeSearchIdx.pm
+++ b/lib/PublicInbox/CodeSearchIdx.pm
@@ -368,7 +368,7 @@ sub repo_stored {
$did > 0 or die "BUG: $repo_ctx->{repo}->{git_dir}: docid=$did";
my ($c, $p) = PublicInbox::PktOp->pair;
$c->{ops}->{shard_done} = [ $self, $repo_ctx,
- PublicInbox::OnDestroy->new(\&next_repos, $repo_ctx, $drs)];
+ on_destroy(\&next_repos, $repo_ctx, $drs)];
# shard_done fires when all shards are committed
my @active = keys %{$repo_ctx->{active}};
$IDX_SHARDS[$_]->wq_io_do('shard_commit', [ $p->{op_p} ]) for @active;
@@ -425,7 +425,7 @@ sub fp_start ($$) {
open my $refs, '+>', undef;
$git->{-repo}->{refs} = $refs;
my ($c, $p) = PublicInbox::PktOp->pair;
- my $next_on_err = PublicInbox::OnDestroy->new(\&index_next, $self);
+ my $next_on_err = on_destroy \&index_next, $self;
$c->{ops}->{fp_done} = [ $self, $git, $next_on_err ];
$IDX_SHARDS[++$ANY_SHARD % scalar(@IDX_SHARDS)]->wq_io_do('fp_async',
[ $p->{op_p}, $refs ], $git->{git_dir})
@@ -664,8 +664,7 @@ sub index_repo {
my $repo_ctx = $REPO_CTX = { self => $self, repo => $repo };
delete $git->{-cidx_gits_fini}; # may fire gits_fini
my $drs = delete $git->{-cidx_dump_roots_start};
- my $index_done = PublicInbox::OnDestroy->new(\&index_done,
- $repo_ctx, $drs);
+ my $index_done = on_destroy \&index_done, $repo_ctx, $drs;
my ($c, $p) = PublicInbox::PktOp->pair;
$c->{ops}->{shard_done} = [ $self, $repo_ctx, $index_done ];
for my $n (0..$#shard_in) {
@@ -690,7 +689,7 @@ sub ct_fini { # run_git cb
sub prep_repo ($$) {
my ($self, $git) = @_;
return if $DO_QUIT;
- my $index_repo = PublicInbox::OnDestroy->new(\&index_repo, $self, $git);
+ my $index_repo = on_destroy \&index_repo, $self, $git;
my $refs = $git->{-repo}->{refs} // die 'BUG: no {-repo}->{refs}';
sysseek($refs, 0, SEEK_SET);
open my $roots_fh, '+>', undef;
@@ -787,7 +786,7 @@ sub scan_git_dirs ($) {
my ($self) = @_;
@$SCANQ = () unless $self->{-opt}->{scan};
$GITS_NR = @$SCANQ or return;
- my $gits_fini = PublicInbox::OnDestroy->new(\&gits_fini);
+ my $gits_fini = on_destroy \&gits_fini;
$_->{-cidx_gits_fini} = $gits_fini for @$SCANQ;
if (my $drs = $TODO{dump_roots_start}) {
$_->{-cidx_dump_roots_start} = $drs for @$SCANQ;
@@ -859,7 +858,7 @@ sub prep_umask ($) {
umask == $um or progress($self, 'using umask from ',
$self->{cidx_dir}, ': ',
sprintf('0%03o', $um));
- PublicInbox::OnDestroy->new(\&CORE::umask, umask($um));
+ on_destroy \&CORE::umask, umask($um);
} else {
$self->{umask} = umask; # for SearchIdx->with_umask
undef;
@@ -1083,12 +1082,12 @@ EOM
($JOIN_DT[1]) = ($QRY_STR =~ /\.\.([0-9]{14})\z/); # YYYYmmddHHMMSS
($JOIN_DT[0]) = ($QRY_STR =~ /\Adt:([0-9]{14})/); # YYYYmmddHHMMSS
$JOIN_DT[0] //= '19700101'.'000000'; # git uses unsigned times
- $TODO{do_join} = PublicInbox::OnDestroy->new(\&do_join, $self);
+ $TODO{do_join} = on_destroy \&do_join, $self;
$TODO{joining} = 1; # keep shards_active() happy
- $TODO{dump_ibx_start} = PublicInbox::OnDestroy->new(\&dump_ibx_start,
- $self, $TODO{do_join});
- $TODO{dump_roots_start} = PublicInbox::OnDestroy->new(
- \&dump_roots_start, $self, $TODO{do_join});
+ $TODO{dump_ibx_start} = on_destroy \&dump_ibx_start,
+ $self, $TODO{do_join};
+ $TODO{dump_roots_start} = on_destroy \&dump_roots_start,
+ $self, $TODO{do_join};
progress($self, "will join in $QRY_STR date range...");
my $id = -1;
@IBXQ = map { ++$id } @IBX;
@@ -1110,8 +1109,7 @@ sub init_prune ($) {
require_progs('prune', 'xapian-delve' => \@delve, sed => \@sed,
comm => \@COMM, awk => \@AWK);
for (0..$#IDX_SHARDS) { push @delve, "$self->{xpfx}/$_" }
- my $run_prune = PublicInbox::OnDestroy->new(\&run_prune, $self,
- $TODO{dump_roots_start});
+ my $run_prune = on_destroy \&run_prune, $self, $TODO{dump_roots_start};
my ($sort_opt, $sed_opt, $delve_opt);
pipe(local $sed_opt->{0}, local $delve_opt->{1});
pipe(local $sort_opt->{0}, local $sed_opt->{1});
@@ -1279,8 +1277,7 @@ sub cidx_run { # main entry point
my $restore_umask = prep_umask($self);
local $SIGSET = PublicInbox::DS::block_signals(
POSIX::SIGTSTP, POSIX::SIGCONT);
- my $restore = PublicInbox::OnDestroy->new($$,
- \&PublicInbox::DS::sig_setmask, $SIGSET);
+ my $restore = on_destroy \&PublicInbox::DS::sig_setmask, $SIGSET;
local $PRUNE_DONE = [];
local $IDXQ = [];
local $SCANQ = [];
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 8bc8cfb7..a6fec954 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -32,9 +32,9 @@ use PublicInbox::Syscall qw(%SIGNUM
EPOLLIN EPOLLOUT EPOLLONESHOT EPOLLEXCLUSIVE);
use PublicInbox::Tmpfile;
use PublicInbox::Select;
+use PublicInbox::OnDestroy;
use Errno qw(EAGAIN EINVAL ECHILD);
use Carp qw(carp croak);
-use autodie qw(fork);
our @EXPORT_OK = qw(now msg_more awaitpid add_timer add_uniq_timer);
my $nextq; # queue for next_tick
@@ -679,12 +679,13 @@ sub awaitpid {
}
}
-sub do_fork () {
+# for persistent child process
+sub fork_persist () {
my $seed = rand(0xffffffff);
- my $pid = fork;
+ my $pid = PublicInbox::OnDestroy::fork_tmp;
if ($pid == 0) {
srand($seed);
- eval { Net::SSLeay::randomize() };
+ eval { Net::SSLeay::randomize() }; # may not be loaded
Reset();
}
$pid;
diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index e578f2e8..1cc0c9e6 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -21,6 +21,7 @@ use PublicInbox::Git;
use PublicInbox::GitAsyncCat;
use PublicInbox::Eml;
use PublicInbox::Config;
+use PublicInbox::OnDestroy;
our $SO_ACCEPTFILTER = 0x1000;
my @CMD;
my ($set_user, $oldset);
@@ -338,15 +339,14 @@ EOF
};
if ($daemonize) {
- my $pid = fork // die "fork: $!";
+ my $pid = PublicInbox::OnDestroy::fork_tmp;
exit if $pid;
-
open(STDIN, '+<', '/dev/null') or
die "redirect stdin failed: $!\n";
open STDOUT, '>&STDIN' or die "redirect stdout failed: $!\n";
open STDERR, '>&STDIN' or die "redirect stderr failed: $!\n";
POSIX::setsid();
- $pid = fork // die "fork: $!";
+ $pid = PublicInbox::OnDestroy::fork_tmp;
exit if $pid;
}
return unless defined $pid_file;
@@ -480,9 +480,9 @@ sub upgrade { # $_[0] = signal name or number (unused)
$pid_file .= '.oldbin';
write_pid($pid_file);
}
- my $pid = fork;
+ my $pid = eval { PublicInbox::OnDestroy::fork_tmp };
if (!defined($pid)) {
- warn "fork failed: $!\n";
+ warn "fork failed: $! $@\n";
} elsif ($pid == 0) {
$ENV{LISTEN_FDS} = scalar @listeners;
$ENV{LISTEN_PID} = $$;
@@ -545,7 +545,7 @@ sub reap_worker { # awaitpid CB
sub start_worker ($) {
my ($nr) = @_;
return unless @listeners;
- my $pid = PublicInbox::DS::do_fork;
+ my $pid = PublicInbox::DS::fork_persist;
if ($pid == 0) {
undef %WORKERS;
local $PublicInbox::DS::Poller; # allow epoll/kqueue
diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index a5cae6f2..ed6d27fd 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -10,7 +10,7 @@
package PublicInbox::IPC;
use v5.12;
use parent qw(Exporter);
-use autodie qw(close fork pipe read socketpair sysread);
+use autodie qw(close pipe read socketpair sysread);
use Carp qw(croak);
use PublicInbox::DS qw(awaitpid);
use PublicInbox::Spawn;
@@ -93,6 +93,8 @@ sub ipc_worker_loop ($$$) {
}
}
+sub exit_exception { exit(!!$@) }
+
# starts a worker if Sereal or Storable is installed
sub ipc_worker_spawn {
my ($self, $ident, $oldset, $fields, @cb_args) = @_;
@@ -102,7 +104,7 @@ sub ipc_worker_spawn {
pipe(my $r_res, my $w_res);
my $sigset = $oldset // PublicInbox::DS::block_signals();
$self->ipc_atfork_prepare;
- my $pid = PublicInbox::DS::do_fork;
+ my $pid = PublicInbox::DS::fork_persist;
if ($pid == 0) {
delete @$self{qw(-wq_s1 -wq_s2 -wq_workers -wq_ppid)};
$w_req = $r_res = undef;
@@ -110,7 +112,7 @@ sub ipc_worker_spawn {
$SIG{$_} = 'IGNORE' for (qw(TERM INT QUIT));
local $0 = $ident;
# ensure we properly exit even if warn() dies:
- my $end = PublicInbox::OnDestroy->new($$, sub { exit(!!$@) });
+ my $end = on_destroy \&exit_exception;
eval {
$fields //= {};
local @$self{keys %$fields} = values(%$fields);
@@ -330,7 +332,7 @@ sub _wq_worker_start {
my ($self, $oldset, $fields, $one, @cb_args) = @_;
my ($bcast1, $bcast2);
$one or socketpair($bcast1, $bcast2, AF_UNIX, SOCK_SEQPACKET, 0);
- my $pid = PublicInbox::DS::do_fork;
+ my $pid = PublicInbox::DS::fork_persist;
if ($pid == 0) {
undef $bcast1;
delete @$self{qw(-wq_s1 -wq_ppid)};
@@ -340,7 +342,7 @@ sub _wq_worker_start {
local $0 = $one ? $self->{-wq_ident} :
"$self->{-wq_ident} $self->{-wq_worker_nr}";
# ensure we properly exit even if warn() dies:
- my $end = PublicInbox::OnDestroy->new($$, sub { exit(!!$@) });
+ my $end = on_destroy \&exit_exception;
eval {
$fields //= {};
local @$self{keys %$fields} = values(%$fields);
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 81f940fe..7c31ab43 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -9,7 +9,7 @@ package PublicInbox::LEI;
use v5.12;
use parent qw(PublicInbox::DS PublicInbox::LeiExternal
PublicInbox::LeiQuery);
-use autodie qw(bind chdir fork open pipe socket socketpair syswrite unlink);
+use autodie qw(bind chdir open pipe socket socketpair syswrite unlink);
use Getopt::Long ();
use Socket qw(AF_UNIX SOCK_SEQPACKET pack_sockaddr_un);
use Errno qw(EPIPE EAGAIN ECONNREFUSED ENOENT ECONNRESET);
@@ -24,6 +24,7 @@ use PublicInbox::Lock;
use PublicInbox::Eml;
use PublicInbox::Import;
use PublicInbox::ContentHash qw(git_sha);
+use PublicInbox::OnDestroy;
use PublicInbox::IPC;
use Time::HiRes qw(stat); # ctime comparisons for config cache
use File::Path ();
@@ -631,9 +632,8 @@ sub _delete_pkt_op { # OnDestroy callback to prevent leaks on die
sub pkt_op_pair {
my ($self) = @_;
- require PublicInbox::OnDestroy;
require PublicInbox::PktOp;
- my $end = PublicInbox::OnDestroy->new($$, \&_delete_pkt_op, $self);
+ my $end = on_destroy \&_delete_pkt_op, $self;
@$self{qw(pkt_op_c pkt_op_p)} = PublicInbox::PktOp->pair;
$end;
}
@@ -1357,7 +1357,7 @@ sub lazy_start {
STDIN->autoflush(1);
dump_and_clear_log();
POSIX::setsid() > 0 or die "setsid: $!";
- my $pid = fork;
+ my $pid = PublicInbox::OnDestroy::fork_tmp;
return if $pid;
$0 = "lei-daemon $path";
local (%PATH2CFG, $MDIR2CFGPATH);
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 4e3e1d1c..08e61e4b 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -293,7 +293,7 @@ sub start_update_ref {
pipe(my $r, my $w);
my $cmd = [ 'git', "--git-dir=$fgrp->{cur_dst}",
qw(update-ref --stdin -z) ];
- my $pack = PublicInbox::OnDestroy->new($$, \&satellite_done, $fgrp);
+ my $pack = on_destroy \&satellite_done, $fgrp;
start_cmd($fgrp, $cmd, { 0 => $r, 2 => $fgrp->{lei}->{2} }, $pack);
close $r;
$fgrp->{dry_run} ? undef : $w;
@@ -373,10 +373,7 @@ sub fgrpv_done {
for my $fgrp (@$fgrpv) {
my $rn = $fgrp->{-remote};
my %opt = ( 2 => $fgrp->{lei}->{2} );
-
- my $update_ref = PublicInbox::OnDestroy->new($$,
- \&fgrp_update, $fgrp);
-
+ my $update_ref = on_destroy \&fgrp_update, $fgrp;
my $src = [ 'git', "--git-dir=$fgrp->{-osdir}", 'for-each-ref',
"--format=refs/%(refname:lstrip=3)%00%(objectname)",
"refs/remotes/$rn/" ];
@@ -467,7 +464,7 @@ sub fgrp_fetch_all {
}
$cmd = [ @git, "--git-dir=$osdir", @fetch, $grp ];
push @$old, @$new;
- my $end = PublicInbox::OnDestroy->new($$, \&fgrpv_done, $old);
+ my $end = on_destroy \&fgrpv_done, $old;
start_cmd($self, $cmd, $opt, $end);
}
}
@@ -567,7 +564,7 @@ sub resume_fetch {
my $cmd = [ @{$self->{-torsocks}}, @git,
fetch_args($self->{lei}, $opt), $rn ];
push @$cmd, '-P' if $self->{lei}->{prune}; # --prune-tags implied
- my $run_puh = PublicInbox::OnDestroy->new($$, \&run_puh, $self, $fini);
+ my $run_puh = on_destroy \&run_puh, $self, $fini;
++$self->{chg}->{nr_chg};
start_cmd($self, $cmd, $opt, $run_puh);
}
@@ -599,7 +596,7 @@ sub clone_v1 {
return;
}
}
- my $fini = PublicInbox::OnDestroy->new($$, \&v1_done, $self);
+ my $fini = on_destroy \&v1_done, $self;
if (my $fgrp = forkgroup_prep($self, $uri)) {
$fgrp->{-fini} = $fini;
if ($resume) {
@@ -621,8 +618,8 @@ sub clone_v1 {
}
}
++$self->{chg}->{nr_chg};
- start_cmd($self, $cmd, $opt, PublicInbox::OnDestroy->new($$,
- \&run_puh, $self, $fini));
+ start_cmd($self, $cmd, $opt,
+ on_destroy(\&run_puh, $self, $fini));
}
if (!$self->{-is_epoch} && $lei->{opt}->{'inbox-config'} =~
/\A(?:always|v1)\z/s &&
@@ -737,7 +734,7 @@ sub atomic_write ($$$) {
sub run_next_puh {
my ($self) = @_;
my $puh = shift @{$self->{-puh_todo}} // return delete($self->{-fini});
- my $fini = PublicInbox::OnDestroy->new($$, \&run_next_puh, $self);
+ my $fini = on_destroy \&run_next_puh, $self;
my $cmd = [ @$puh, ($self->{cur_dst} // $self->{dst}) ];
my $opt = +{ map { $_ => $self->{lei}->{$_} } (0..2) };
start_cmd($self, $cmd, undef, $opt, $fini);
@@ -762,7 +759,7 @@ sub update_ent {
my $opt = { 2 => $self->{lei}->{2} };
open($opt->{1}, '+>', undef);
$self->{-show_ref_up} = $opt->{1};
- my $done = PublicInbox::OnDestroy->new($$, \&up_fp_done, $self);
+ my $done = on_destroy \&up_fp_done, $self;
start_cmd($self, $cmd, $opt, $done);
}
$new = $self->{-ent}->{head};
@@ -883,7 +880,7 @@ sub clone_v2_prep ($$;$) {
my $want = parse_epochs($lei->{opt}->{epoch}, $v2_epochs);
my $task = $m ? bless { %$self }, __PACKAGE__ : $self;
my (@skip, $desc);
- my $fini = PublicInbox::OnDestroy->new($$, \&v2_done, $task);
+ my $fini = on_destroy \&v2_done, $task;
for my $nr (sort { $a <=> $b } keys %$v2_epochs) {
my ($uri, $key) = @{$v2_epochs->{$nr}};
my $src = $uri->as_string;
@@ -1018,7 +1015,7 @@ sub clone_all {
my ($self, $m) = @_;
my $todo = $TODO;
$TODO = \'BUG on further use';
- my $end = PublicInbox::OnDestroy->new($$, \&fgrp_fetch_all, $self);
+ my $end = on_destroy \&fgrp_fetch_all, $self;
{
my $nodep = delete $todo->{''};
diff --git a/lib/PublicInbox/LeiP2q.pm b/lib/PublicInbox/LeiP2q.pm
index 610adb78..68faa016 100644
--- a/lib/PublicInbox/LeiP2q.pm
+++ b/lib/PublicInbox/LeiP2q.pm
@@ -189,7 +189,7 @@ sub lei_p2q { # the "lei patch-to-query" entry point
sub ipc_atfork_child {
my ($self) = @_;
PublicInbox::LeiInput::input_only_atfork_child($self);
- PublicInbox::OnDestroy->new($$, \&emit_query, $self);
+ on_destroy \&emit_query, $self;
}
no warnings 'once';
diff --git a/lib/PublicInbox/LeiStore.pm b/lib/PublicInbox/LeiStore.pm
index a752174d..2eb09eca 100644
--- a/lib/PublicInbox/LeiStore.pm
+++ b/lib/PublicInbox/LeiStore.pm
@@ -28,6 +28,7 @@ use PublicInbox::Spawn qw(spawn);
use PublicInbox::MdirReader;
use PublicInbox::LeiToMail;
use PublicInbox::Compat qw(uniqstr);
+use PublicInbox::OnDestroy;
use File::Temp qw(tmpnam);
use POSIX ();
use IO::Handle (); # ->autoflush
@@ -135,7 +136,7 @@ sub eidx_init {
my ($self) = @_;
my $eidx = $self->{priv_eidx};
my $tl = wantarray && $self->{-err_wr} ?
- PublicInbox::OnDestroy->new($$, \&_tail_err, $self) :
+ on_destroy(\&_tail_err, $self) :
undef;
$eidx->idx_init({-private => 1}); # acquires lock
wantarray ? ($eidx, $tl) : $eidx;
diff --git a/lib/PublicInbox/LeiTag.pm b/lib/PublicInbox/LeiTag.pm
index 320b0355..da8caeb7 100644
--- a/lib/PublicInbox/LeiTag.pm
+++ b/lib/PublicInbox/LeiTag.pm
@@ -1,12 +1,12 @@
-# Copyright (C) 2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
# handles "lei tag" command
package PublicInbox::LeiTag;
-use strict;
-use v5.10.1;
+use v5.12;
use parent qw(PublicInbox::IPC PublicInbox::LeiInput);
use PublicInbox::InboxWritable qw(eml_from_path);
+use PublicInbox::OnDestroy;
sub input_eml_cb { # used by PublicInbox::LeiInput::input_fh
my ($self, $eml) = @_;
@@ -49,7 +49,7 @@ sub ipc_atfork_child {
PublicInbox::LeiInput::input_only_atfork_child($self);
$self->{lse} = $self->{lei}->{sto}->search;
# this goes out-of-scope at worker process exit:
- PublicInbox::OnDestroy->new($$, \¬e_unimported, $self);
+ on_destroy \¬e_unimported, $self;
}
# Workaround bash word-splitting s to ['kw', ':', 'keyword' ...]
diff --git a/lib/PublicInbox/Lock.pm b/lib/PublicInbox/Lock.pm
index 2a5a0f30..7162d80e 100644
--- a/lib/PublicInbox/Lock.pm
+++ b/lib/PublicInbox/Lock.pm
@@ -43,7 +43,7 @@ sub lock_release {
sub lock_for_scope {
my ($self) = @_;
lock_acquire($self) or return; # lock_path not set
- PublicInbox::OnDestroy->new(\&lock_release, $self);
+ on_destroy \&lock_release, $self;
}
sub lock_acquire_fast {
@@ -60,7 +60,7 @@ sub lock_release_fast {
sub lock_for_scope_fast {
my ($self) = @_;
lock_acquire_fast($self) or return; # lock_path not set
- PublicInbox::OnDestroy->new(\&lock_release_fast, $self);
+ on_destroy \&lock_release_fast, $self;
}
1;
diff --git a/lib/PublicInbox/MHreader.pm b/lib/PublicInbox/MHreader.pm
index 3e7bbd5c..16e505a2 100644
--- a/lib/PublicInbox/MHreader.pm
+++ b/lib/PublicInbox/MHreader.pm
@@ -52,7 +52,7 @@ EOM
sub mh_each_file {
my ($self, $efcb, @arg) = @_;
opendir(my $dh, my $dir = $self->{dir});
- my $restore = PublicInbox::OnDestroy->new($$, \&chdir, $self->{cwdfh});
+ my $restore = on_destroy \&chdir, $self->{cwdfh};
chdir($dh);
my $sort = $self->{sort};
if (defined $sort && "@$sort" ne 'none') {
@@ -96,7 +96,7 @@ sub mh_each_eml {
sub mh_read_one {
my ($self, $n, $ucb, @arg) = @_;
- my $restore = PublicInbox::OnDestroy->new($$, \&chdir, $self->{cwdfh});
+ my $restore = on_destroy \&chdir, $self->{cwdfh};
chdir(my $dir = $self->{dir});
_file2eml($dir, $n, $self, $ucb, @arg);
}
diff --git a/lib/PublicInbox/MboxLock.pm b/lib/PublicInbox/MboxLock.pm
index 9d7d4a32..5e373873 100644
--- a/lib/PublicInbox/MboxLock.pm
+++ b/lib/PublicInbox/MboxLock.pm
@@ -4,7 +4,7 @@
# Various mbox locking methods
package PublicInbox::MboxLock;
use v5.12;
-use PublicInbox::OnDestroy;
+use PublicInbox::OnDestroy ();
use Fcntl qw(:flock F_SETLK F_SETLKW F_RDLCK F_WRLCK
O_CREAT O_EXCL O_WRONLY SEEK_SET);
use Carp qw(croak);
@@ -122,10 +122,10 @@ sub acq {
sub DESTROY {
my ($self) = @_;
my $f = $self->{".lock$$"} or return;
- my $x;
+ my $od;
if (my $dh = delete $self->{dh}) {
opendir my $c, '.';
- $x = PublicInbox::OnDestroy->new(\&chdir, $c);
+ $od = PublicInbox::OnDestroy::all \&chdir, $c;
chdir($dh);
}
CORE::unlink($f) or die "unlink($f): $! (lock stolen?)";
diff --git a/lib/PublicInbox/OnDestroy.pm b/lib/PublicInbox/OnDestroy.pm
index d9a6cd24..4301edff 100644
--- a/lib/PublicInbox/OnDestroy.pm
+++ b/lib/PublicInbox/OnDestroy.pm
@@ -3,22 +3,29 @@
package PublicInbox::OnDestroy;
use v5.12;
+use parent qw(Exporter);
+use autodie qw(fork);
+our @EXPORT = qw(on_destroy);
+our $fork_gen = 0;
-sub new {
- shift; # ($class, $cb, @args)
- bless [ @_ ], __PACKAGE__;
+# either parent or child is expected to exit or exec shortly after this:
+sub fork_tmp () {
+ my $pid = fork;
+ ++$fork_gen if $pid == 0;
+ $pid;
}
+# all children
+sub all (@) { bless [ undef, @_ ], __PACKAGE__ }
+
+# same process
+sub on_destroy (@) { bless [ $fork_gen, @_ ], __PACKAGE__ }
+
sub cancel { @{$_[0]} = () }
sub DESTROY {
- my ($cb, @args) = @{$_[0]};
- if (!ref($cb) && $cb) {
- my $pid = $cb;
- return if $pid != $$;
- $cb = shift @args;
- }
- $cb->(@args) if $cb;
+ my ($fgen, $cb, @args) = @{$_[0]};
+ $cb->(@args) if ($cb && ($fgen // $fork_gen) == $fork_gen);
}
1;
diff --git a/lib/PublicInbox/SearchIdxShard.pm b/lib/PublicInbox/SearchIdxShard.pm
index 1630eb4a..ea261bda 100644
--- a/lib/PublicInbox/SearchIdxShard.pm
+++ b/lib/PublicInbox/SearchIdxShard.pm
@@ -45,7 +45,7 @@ sub ipc_atfork_child { # called automatically before ipc_worker_loop
$v2w->{current_info} = "[$self->{shard}]"; # for $SIG{__WARN__}
$self->begin_txn_lazy;
# caller (ipc_worker_spawn) must capture this:
- PublicInbox::OnDestroy->new($$, \&_worker_done, $self);
+ on_destroy \&_worker_done, $self;
}
sub index_eml {
diff --git a/lib/PublicInbox/SpawnPP.pm b/lib/PublicInbox/SpawnPP.pm
index f89d37d4..9ad4d0a1 100644
--- a/lib/PublicInbox/SpawnPP.pm
+++ b/lib/PublicInbox/SpawnPP.pm
@@ -7,7 +7,8 @@
package PublicInbox::SpawnPP;
use v5.12;
use POSIX qw(dup2 _exit setpgid :signal_h);
-use autodie qw(chdir close fork pipe);
+use autodie qw(chdir close pipe);
+use PublicInbox::OnDestroy;
# this is loaded by PublicInbox::Spawn, so we can't use/require it, here
# Pure Perl implementation for folks that do not use Inline::C
@@ -22,7 +23,7 @@ sub pi_fork_exec ($$$$$$$) {
}
sigprocmask(SIG_SETMASK, $set, $old) or die "SIG_SETMASK(set): $!";
pipe(my $r, my $w);
- my $pid = fork;
+ my $pid = PublicInbox::OnDestroy::fork_tmp;
if ($pid == 0) {
close $r;
$SIG{__DIE__} = sub {
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 5f159683..a7ec9b5b 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -588,9 +588,9 @@ sub start_script {
require PublicInbox::DS;
my $oset = PublicInbox::DS::block_signals();
require PublicInbox::OnDestroy;
- my $tmp_mask = PublicInbox::OnDestroy->new(
+ my $tmp_mask = PublicInbox::OnDestroy::all(
\&PublicInbox::DS::sig_setmask, $oset);
- my $pid = PublicInbox::DS::do_fork();
+ my $pid = PublicInbox::DS::fork_persist();
if ($pid == 0) {
close($_) for (@{delete($opt->{-CLOFORK}) // []});
# pretend to be systemd (cf. sd_listen_fds(3))
diff --git a/lib/PublicInbox/Umask.pm b/lib/PublicInbox/Umask.pm
index 00772ce5..2c859e65 100644
--- a/lib/PublicInbox/Umask.pm
+++ b/lib/PublicInbox/Umask.pm
@@ -58,7 +58,7 @@ sub _umask_for {
sub with_umask {
my ($self, $cb, @arg) = @_;
my $old = umask($self->{umask} //= umask_prepare($self));
- my $restore = PublicInbox::OnDestroy->new($$, \&CORE::umask, $old);
+ my $restore = on_destroy \&CORE::umask, $old;
$cb ? $cb->(@arg) : $restore;
}
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 790b9a2c..f47c2703 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -25,6 +25,7 @@ use PublicInbox::Tmpfile;
use PublicInbox::ViewDiff qw(flush_diff uri_escape_path);
use PublicInbox::View;
use PublicInbox::Eml;
+use PublicInbox::OnDestroy;
use Text::Wrap qw(wrap);
use PublicInbox::Hval qw(ascii_html to_filename prurl utf8_maybe);
use POSIX qw(strftime);
@@ -388,7 +389,7 @@ sub show_commit ($$) {
qw(--encoding=UTF-8 -z --no-notes --no-patch), $oid),
undef, { 1 => $ctx->{patch_fh} });
$qsp_h->{qsp_err} = \($ctx->{-qsp_err_h} = '');
- my $cmt_fin = PublicInbox::OnDestroy->new($$, \&cmt_fin, $ctx);
+ my $cmt_fin = on_destroy \&cmt_fin, $ctx;
$ctx->{git} = $git;
$ctx->{oid} = $oid;
$qsp_h->psgi_qx($ctx->{env}, undef, \&cmt_hdr_prep, $ctx, $cmt_fin);
@@ -624,7 +625,7 @@ sub start_solver ($) {
my $v = $ctx->{qp}->{$from} // next;
$ctx->{hints}->{$to} = $v if $v ne '';
}
- $ctx->{-next_solver} = PublicInbox::OnDestroy->new($$, \&next_solver);
+ $ctx->{-next_solver} = on_destroy \&next_solver;
++$solver_nr;
$ctx->{-tmp} = File::Temp->newdir("solver.$ctx->{oid_b}-XXXX",
TMPDIR => 1);
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index 1ec574ea..eb90d353 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -445,7 +445,7 @@ sub imap_idle_reap { # awaitpid callback
sub imap_idle_fork {
my ($self, $uri, $intvl) = @_;
return if $self->{quit};
- my $pid = PublicInbox::DS::do_fork;
+ my $pid = PublicInbox::DS::fork_persist;
if ($pid == 0) {
watch_atfork_child($self);
watch_imap_idle_1($self, $uri, $intvl);
@@ -506,7 +506,7 @@ sub poll_fetch_fork { # DS::add_timer callback
my @imap = grep { # push() always returns > 0
$_->scheme =~ m!\Aimaps?!i ? 1 : (push(@nntp, $_) < 0)
} @$uris;
- my $pid = PublicInbox::DS::do_fork;
+ my $pid = PublicInbox::DS::fork_persist;
if ($pid == 0) {
watch_atfork_child($self);
watch_imap_fetch_all($self, \@imap) if @imap;
diff --git a/lib/PublicInbox/WwwCoderepo.pm b/lib/PublicInbox/WwwCoderepo.pm
index 61aa7862..a5e2dc4a 100644
--- a/lib/PublicInbox/WwwCoderepo.pm
+++ b/lib/PublicInbox/WwwCoderepo.pm
@@ -253,7 +253,7 @@ sub summary ($$) {
push(@log, $tip) if defined $tip;
# limit scope for MockHTTP test (t/solver_git.t)
- my $END = PublicInbox::OnDestroy->new($$, \&summary_END, $ctx);
+ my $END = on_destroy \&summary_END, $ctx;
for (['log', \@log],
[ 'heads', [@EACH_REF, "--count=$nb", 'refs/heads'] ],
[ 'tags', [@EACH_REF, "--count=$nt", 'refs/tags'] ]) {
diff --git a/lib/PublicInbox/XapClient.pm b/lib/PublicInbox/XapClient.pm
index 4dcbbe5d..98034130 100644
--- a/lib/PublicInbox/XapClient.pm
+++ b/lib/PublicInbox/XapClient.pm
@@ -11,7 +11,7 @@ use v5.12;
use PublicInbox::Spawn qw(spawn);
use Socket qw(AF_UNIX SOCK_SEQPACKET);
use PublicInbox::IPC;
-use autodie qw(fork pipe socketpair);
+use autodie qw(pipe socketpair);
sub mkreq {
my ($self, $ios, @arg) = @_;
diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index ed11a2f8..8c7732f5 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -244,7 +244,7 @@ sub reap_worker { # awaitpid CB
sub start_worker ($) {
my ($nr) = @_;
- my $pid = eval { PublicInbox::DS::do_fork } // return(warn($@));
+ my $pid = eval { PublicInbox::DS::fork_persist } // return(warn($@));
if ($pid == 0) {
undef %WORKERS;
$SIG{TTIN} = $SIG{TTOU} = 'IGNORE';
diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index 69f0af43..9a148ae4 100644
--- a/lib/PublicInbox/Xapcmd.pm
+++ b/lib/PublicInbox/Xapcmd.pm
@@ -103,7 +103,7 @@ sub commit_changes ($$$$) {
sub cb_spawn {
my ($cb, $args, $opt) = @_; # $cb = cpdb() or compact()
- my $pid = PublicInbox::DS::do_fork;
+ my $pid = PublicInbox::DS::fork_persist;
return $pid if $pid > 0;
$SIG{__DIE__} = sub { warn @_; _exit(1) }; # don't jump up stack
$cb->($args, $opt);
diff --git a/script/public-inbox-init b/script/public-inbox-init
index 8915cf31..cf6443f7 100755
--- a/script/public-inbox-init
+++ b/script/public-inbox-init
@@ -122,7 +122,7 @@ sysopen($lockfh, $lockfile, O_RDWR|O_CREAT|O_EXCL) or do {
exit(255);
};
require PublicInbox::OnDestroy;
-my $auto_unlink = PublicInbox::OnDestroy->new($$, sub { unlink $lockfile });
+my $auto_unlink = PublicInbox::OnDestroy::on_destroy(sub { unlink $lockfile });
my $perm = 0644 & ~umask;
my %seen;
if (-e $pi_config) {
diff --git a/t/lei-sigpipe.t b/t/lei-sigpipe.t
index 72bc6c7d..b9fd88a6 100644
--- a/t/lei-sigpipe.t
+++ b/t/lei-sigpipe.t
@@ -26,9 +26,7 @@ SKIP: {
# https://public-inbox.org/meta/20220227080422.gyqowrxomzu6gyin@sourcephile.fr/
my $oldSIGPIPE = $SIG{PIPE};
$SIG{PIPE} = 'DEFAULT';
-my $cleanup = PublicInbox::OnDestroy->new($$, sub {
- $SIG{PIPE} = $oldSIGPIPE;
-});
+my $cleanup = on_destroy(sub { $SIG{PIPE} = $oldSIGPIPE });
test_lei(sub {
my $f = "$ENV{HOME}/big.eml";
diff --git a/t/mbox_lock.t b/t/mbox_lock.t
index c2fee0d4..1fc828aa 100644
--- a/t/mbox_lock.t
+++ b/t/mbox_lock.t
@@ -2,6 +2,7 @@
# Copyright (C) 2021 all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
use strict; use v5.10.1; use PublicInbox::TestCommon;
+use autodie qw(chdir);
use POSIX qw(_exit);
use PublicInbox::DS qw(now);
use Errno qw(EAGAIN);
@@ -18,11 +19,11 @@ ok(!-f "$f.lock", 'no dotlock with none');
undef $mbl;
{
opendir my $cur, '.' or BAIL_OUT $!;
- my $od = PublicInbox::OnDestroy->new(sub { chdir $cur });
- chdir $tmpdir or BAIL_OUT;
+ my $od = on_destroy \&chdir, $cur;
+ chdir $tmpdir;
my $abs = "$tmpdir/rel.lock";
my $rel = PublicInbox::MboxLock->acq('rel', 1, ['dotlock']);
- chdir '/' or BAIL_OUT;
+ chdir '/';
ok(-f $abs, 'lock with abs path created');
undef $rel;
ok(!-f $abs, 'lock gone despite being in the wrong dir');
diff --git a/t/mh_reader.t b/t/mh_reader.t
index c81df32e..95a7be4a 100644
--- a/t/mh_reader.t
+++ b/t/mh_reader.t
@@ -5,7 +5,6 @@ use PublicInbox::TestCommon;
require_ok 'PublicInbox::MHreader';
use PublicInbox::IO qw(write_file);
use PublicInbox::Lock;
-use PublicInbox::OnDestroy;
use PublicInbox::Eml;
use File::Path qw(remove_tree);
use autodie;
diff --git a/t/on_destroy.t b/t/on_destroy.t
index e7945100..e8fdf35e 100644
--- a/t/on_destroy.t
+++ b/t/on_destroy.t
@@ -1,37 +1,44 @@
#!perl -w
use v5.12;
use Test::More;
-require_ok 'PublicInbox::OnDestroy';
+use PublicInbox::OnDestroy;
+use POSIX qw(_exit);
my @x;
-my $od = PublicInbox::OnDestroy->new(sub { push @x, 'hi' });
+my $od = on_destroy sub { push @x, 'hi' };
is_deeply(\@x, [], 'not called, yet');
undef $od;
is_deeply(\@x, [ 'hi' ], 'no args works');
-$od = PublicInbox::OnDestroy->new(sub { $x[0] = $_[0] }, 'bye');
+$od = on_destroy sub { $x[0] = $_[0] }, 'bye';
is_deeply(\@x, [ 'hi' ], 'nothing changed while alive');
undef $od;
is_deeply(\@x, [ 'bye' ], 'arg passed');
-$od = PublicInbox::OnDestroy->new(sub { @x = @_ }, qw(x y));
+$od = on_destroy sub { @x = @_ }, qw(x y);
undef $od;
is_deeply(\@x, [ 'x', 'y' ], '2 args passed');
open my $tmp, '+>>', undef or BAIL_OUT $!;
$tmp->autoflush(1);
-$od = PublicInbox::OnDestroy->new(1, sub { print $tmp "$$ DESTROY\n" });
-undef $od;
+$od = on_destroy sub { print $tmp "$$ DESTROY\n" };
+my $pid = PublicInbox::OnDestroy::fork_tmp;
+if ($pid == 0) { undef $od; _exit 0; };
+waitpid($pid, 0);
+is $?, 0, 'test process exited';
is(-s $tmp, 0, '$tmp is empty on pid mismatch');
-$od = PublicInbox::OnDestroy->new($$, sub { $tmp = $$ });
+$od->cancel;
+undef $od;
+is(-s $tmp, 0, '$tmp is empty after ->cancel');
+$od = on_destroy sub { $tmp = $$ };
undef $od;
is($tmp, $$, '$tmp set to $$ by callback');
-$od = PublicInbox::OnDestroy->new($$, sub { $tmp = 'foo' });
+$od = on_destroy sub { $tmp = 'foo' };
$od->cancel;
$od = undef;
isnt($tmp, 'foo', '->cancel');
if (my $nr = $ENV{TEST_LEAK_NR}) {
for (0..$nr) {
- $od = PublicInbox::OnDestroy->new(sub { @x = @_ }, qw(x y));
+ $od = on_destroy sub { @x = @_ }, qw(x y);
}
}
diff --git a/xt/net_writer-imap.t b/xt/net_writer-imap.t
index f7796e8e..176502ba 100644
--- a/xt/net_writer-imap.t
+++ b/xt/net_writer-imap.t
@@ -82,7 +82,7 @@ my $mics = do {
$nwr->imap_common_init;
};
my $mic = (values %$mics)[0];
-my $cleanup = PublicInbox::OnDestroy->new($$, sub {
+my $cleanup = on_destroy sub {
if (defined($folder)) {
my $mic = $nwr->mic_get($uri);
$mic->delete($folder) or
@@ -92,7 +92,7 @@ my $cleanup = PublicInbox::OnDestroy->new($$, sub {
local $ENV{HOME} = $tmpdir;
system(qw(git credential-cache exit));
}
-});
+};
my $imap_append = $nwr->can('imap_append');
my $smsg = bless { kw => [ 'seen' ] }, 'PublicInbox::Smsg';
$imap_append->($mic, $folder, undef, $smsg, eml_load('t/plack-qp.eml'));
^ permalink raw reply related [flat|nested] 4+ messages in thread