]> git.ipfire.org Git - thirdparty/public-inbox.git/commitdiff
treewide: avoid getpid() for OnDestroy checks
authorEric Wong <e@80x24.org>
Mon, 1 Apr 2024 06:49:37 +0000 (06:49 +0000)
committerEric Wong <e@80x24.org>
Wed, 3 Apr 2024 08:28:05 +0000 (08:28 +0000)
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.

29 files changed:
lib/PublicInbox/CodeSearchIdx.pm
lib/PublicInbox/DS.pm
lib/PublicInbox/Daemon.pm
lib/PublicInbox/IPC.pm
lib/PublicInbox/LEI.pm
lib/PublicInbox/LeiMirror.pm
lib/PublicInbox/LeiP2q.pm
lib/PublicInbox/LeiStore.pm
lib/PublicInbox/LeiTag.pm
lib/PublicInbox/Lock.pm
lib/PublicInbox/MHreader.pm
lib/PublicInbox/MboxLock.pm
lib/PublicInbox/OnDestroy.pm
lib/PublicInbox/SearchIdxShard.pm
lib/PublicInbox/SpawnPP.pm
lib/PublicInbox/TestCommon.pm
lib/PublicInbox/Umask.pm
lib/PublicInbox/ViewVCS.pm
lib/PublicInbox/Watch.pm
lib/PublicInbox/WwwCoderepo.pm
lib/PublicInbox/XapClient.pm
lib/PublicInbox/XapHelper.pm
lib/PublicInbox/Xapcmd.pm
script/public-inbox-init
t/lei-sigpipe.t
t/mbox_lock.t
t/mh_reader.t
t/on_destroy.t
xt/net_writer-imap.t

index 570ff64fe3546fff2a5b7f7cb25d01f48c80ac1a..6d777bf62a3e2f0b06bb9d3457810f377ed2aa74 100644 (file)
@@ -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 = [];
index 8bc8cfb77ba699ab1490455543d21259fea602a8..a6fec9546c571ec7e62495eebed7c70a5f73b789 100644 (file)
@@ -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;
index e578f2e8e9a57c6206b15db4fb5e961d5121e563..1cc0c9e69e316c6cb93ee310bdd1528a323f17b7 100644 (file)
@@ -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
index a5cae6f2e8b0113c67d394f3fee7d1afc6052b99..ed6d27fd489b13180afd4bd16a6dbe972c943d17 100644 (file)
@@ -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);
index 81f940fe13adb58c0b5f02c63dd4103edbe64237..7c31ab43ab3610fccae78b85d4d5b65277c82316 100644 (file)
@@ -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);
index 4e3e1d1c8a35b8b0b96417de8574061cd9f12ff6..08e61e4b2557fe3c98628c9c8413009524a4e787 100644 (file)
@@ -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->{''};
 
index 610adb78cb2a0b16edd4be71fd0fcb09a2f3a491..68faa0165ff7bf49f4ad7f3690ca4746927b4f2d 100644 (file)
@@ -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';
index a752174d3cd3476c2efbc7a3c56c5e4762d1c4a0..2eb09eca39e9f6ff311a3e8439f7b7acba70dcda 100644 (file)
@@ -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;
index 320b03558155d129b89b733fd7d18cb408536430..da8caeb74d55364bf0b1380143910edd6d583840 100644 (file)
@@ -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($$, \&note_unimported, $self);
+       on_destroy \&note_unimported, $self;
 }
 
 # Workaround bash word-splitting s to ['kw', ':', 'keyword' ...]
index 2a5a0f308295f812f55d72b162dc9ca64e6c7058..7162d80e5ee7b67b7ba2b420b3a8f2c46971bc92 100644 (file)
@@ -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;
index 3e7bbd5c0202f2511c8ad5151a362d9ed8d0d590..16e505a25fc6c04708edf1464ebbb0ac48e2117d 100644 (file)
@@ -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);
 }
index 9d7d4a32a13c45e64a237c953225b7c76ecaaaaa..5e37387348d4778fd3564f4c4da91341185bc559 100644 (file)
@@ -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?)";
index d9a6cd241becf48025f37f11389b94a4ee821396..4301edffc543becf45adb2bbef3001c33135d431 100644 (file)
@@ -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;
index 1630eb4aa2deba6b7a1908addc5d7707e0e91afa..ea261bdab7298408e50735a8ffe2fa3da61122e9 100644 (file)
@@ -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 {
index f89d37d4287d65748fd4e4249957c3049c452f1f..9ad4d0a11a8fb3fa3b8450b4b8c44694ce3e8888 100644 (file)
@@ -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 {
index 5f159683a1d11279807261af3f3ab4c1dccbcb2a..a7ec9b5b47d5028c494b734fedc24065154404e0 100644 (file)
@@ -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))
index 00772ce535a4e03560b418c335825d505ffa0cd3..2c859e65a6584fe31cb89b01fec5608f477f3036 100644 (file)
@@ -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;
 }
 
index 790b9a2c59089c2fd2e8b7d72ed7cc21b77c559c..f47c2703ebf9fb2fa3525a1d0be11a3abc927a7e 100644 (file)
@@ -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);
index 1ec574eadc8f35cb461fa198ad42be45a7da2205..eb90d353f3e7d39a50c23b7737b51ac5bb4cd676 100644 (file)
@@ -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;
index 61aa78620c90cbf1e1e335098bf8dadd4a8fadae..a5e2dc4a9930e25942240147938644d34841a745 100644 (file)
@@ -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'] ]) {
index 4dcbbe5dd1454b1e802121fe88f4c010a5d96faf..98034130c7b00f0acfdf4698f78c2244206968c9 100644 (file)
@@ -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) = @_;
index ed11a2f849982189864f8bce9d72164a667689e0..8c7732f564e50b6794406ea7c282683248ada07e 100644 (file)
@@ -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';
index 69f0af4329a6d3e08c27ea802770a1cd55510a1a..9a148ae45dabdf58349b792431fcbc3fe0aa1d81 100644 (file)
@@ -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);
index 8915cf31914ec75955f41a953bfb2c0979e15982..cf6443f778e2d42f3960e493ac291d4d6e1d93b2 100755 (executable)
@@ -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) {
index 72bc6c7de5040fb8948e7ce3989fe78cd104decc..b9fd88a672ee54b721b3c277351dd5b83afdd8ae 100644 (file)
@@ -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";
index c2fee0d4eb88d056f006480841c497c1ab3e5576..1fc828aabd666003fa7ce5991d20f0d709bd2c7b 100644 (file)
@@ -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');
index c81df32e369aa396cf74f15d0f111e53bf571082..95a7be4aa9f7b1dff6b69fada1bd0f0223c5bac1 100644 (file)
@@ -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;
index e79451005d52296b27fa3749be52873ff71a324b..e8fdf35e3f0db7a7a73ef296e86ec4d6bb541616 100644 (file)
@@ -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);
        }
 }
 
index f7796e8e420ee54f0efb5621b77639c0ee2155fe..176502ba357da5e955b6f311ba85eaddb063d2e8 100644 (file)
@@ -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'));