From: Eric Wong Date: Mon, 1 Apr 2024 06:49:37 +0000 (+0000) Subject: treewide: avoid getpid() for OnDestroy checks X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=108196adad5e70b6dd40dc431cd1033d44679483;p=thirdparty%2Fpublic-inbox.git treewide: avoid getpid() for OnDestroy checks 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. --- diff --git a/lib/PublicInbox/CodeSearchIdx.pm b/lib/PublicInbox/CodeSearchIdx.pm index 570ff64fe..6d777bf62 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 8bc8cfb77..a6fec9546 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 e578f2e8e..1cc0c9e69 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 a5cae6f2e..ed6d27fd4 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 81f940fe1..7c31ab43a 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 4e3e1d1c8..08e61e4b2 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 610adb78c..68faa0165 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 a752174d3..2eb09eca3 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 320b03558..da8caeb74 100644 --- a/lib/PublicInbox/LeiTag.pm +++ b/lib/PublicInbox/LeiTag.pm @@ -1,12 +1,12 @@ -# Copyright (C) 2021 all contributors +# Copyright (C) all contributors # License: AGPL-3.0+ # 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 2a5a0f308..7162d80e5 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 3e7bbd5c0..16e505a25 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 9d7d4a32a..5e3738734 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 d9a6cd241..4301edffc 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 1630eb4aa..ea261bdab 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 f89d37d42..9ad4d0a11 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 5f159683a..a7ec9b5b4 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 00772ce53..2c859e65a 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 790b9a2c5..f47c2703e 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 1ec574ead..eb90d353f 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 61aa78620..a5e2dc4a9 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 4dcbbe5dd..98034130c 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 ed11a2f84..8c7732f56 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 69f0af432..9a148ae45 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 8915cf319..cf6443f77 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 72bc6c7de..b9fd88a67 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 c2fee0d4e..1fc828aab 100644 --- a/t/mbox_lock.t +++ b/t/mbox_lock.t @@ -2,6 +2,7 @@ # Copyright (C) 2021 all contributors # License: AGPL-3.0+ 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 c81df32e3..95a7be4aa 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 e79451005..e8fdf35e3 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 f7796e8e4..176502ba3 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'));