on_destroy: support PID owner guard

Since we'll be forking for Xapian indexing and maybe
other places, having a simple guard in place to ensure
OnDestroy doesn't unexpectedly unlink files or similar
is a safer option.

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index b84e24ef953da9800823278b678c46ca38e1f6fa..4af85d49bbb83a3b9b483fbab7f4b680771d3953 100644 (file)
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -745,9 +745,8 @@ sub lazy_start {
        # reading the <$daemon> pipe.
        openlog($path, 'pid', 'user');
        local $SIG{__WARN__} = sub { syslog('warning', "@_") };
-       my $owner_pid = $$;
-       my $on_destroy = PublicInbox::OnDestroy->new(sub {
-               syslog('crit', "$@") if $@ && $$ == $owner_pid;
+       my $on_destroy = PublicInbox::OnDestroy->new($$, sub {
+               syslog('crit', "$@") if $@;
        });
        open STDERR, '>&STDIN' or die "redirect stderr failed: $!";
        open STDOUT, '>&STDIN' or die "redirect stdout failed: $!";

diff --git a/lib/PublicInbox/Lock.pm b/lib/PublicInbox/Lock.pm
index f6eaa5ce1101128eef02885f444e09b08f5fcbb1..1d0b4f9cbcbb104741bb6cb09396184d71fcad53 100644 (file)
--- a/lib/PublicInbox/Lock.pm
+++ b/lib/PublicInbox/Lock.pm
@@ -36,9 +36,9 @@ sub lock_release {
 
 # caller must use return value
 sub lock_for_scope {
-       my ($self) = @_;
+       my ($self, @single_pid) = @_;
        $self->lock_acquire;
-       PublicInbox::OnDestroy->new(\&lock_release, $self);
+       PublicInbox::OnDestroy->new(@single_pid, \&lock_release, $self);
 }
 
 sub new_tmp {

diff --git a/lib/PublicInbox/OnDestroy.pm b/lib/PublicInbox/OnDestroy.pm
index 841f87d45eabe27ea676536f64268a88127a4953..65ebd7dcf574f5aa72243add2b0df55f355d794a 100644 (file)
--- a/lib/PublicInbox/OnDestroy.pm
+++ b/lib/PublicInbox/OnDestroy.pm
@@ -10,6 +10,11 @@ sub new {
 
 sub DESTROY {
        my ($cb, @args) = @{$_[0]};
+       if (!ref($cb)) {
+               my $pid = $cb;
+               return if $pid != $$;
+               $cb = shift @args;
+       }
        $cb->(@args) if $cb;
 }
 

diff --git a/script/public-inbox-init b/script/public-inbox-init
index 693f5ca17ced941ac936ba4e43895fc739628d2e..222d0c603f6b1e93384b1ab369a99552ba8aa630 100755 (executable)
--- a/script/public-inbox-init
+++ b/script/public-inbox-init
@@ -92,7 +92,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->new($$, sub { unlink $lockfile });
 my ($perm, %seen);
 if (-e $pi_config) {
        open(my $oh, '<', $pi_config) or die "unable to read $pi_config: $!\n";

diff --git a/t/on_destroy.t b/t/on_destroy.t
index 8b85b48ecb7b361ff2b056d53f3e45a197f6c4ee..0de67d0bcde10c116e49270dd20a7dff3b1ea72e 100644 (file)
--- a/t/on_destroy.t
+++ b/t/on_destroy.t
@@ -16,6 +16,15 @@ $od = PublicInbox::OnDestroy->new(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;
+is(-s $tmp, 0, '$tmp is empty on pid mismatch');
+$od = PublicInbox::OnDestroy->new($$, sub { $tmp = $$ });
+undef $od;
+is($tmp, $$, '$tmp set to $$ by callback');
+
 if (my $nr = $ENV{TEST_LEAK_NR}) {
        for (0..$nr) {
                $od = PublicInbox::OnDestroy->new(sub { @x = @_ }, qw(x y));