From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 6E2E420086 for ; Thu, 31 Dec 2020 13:51:58 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 36/36] on_destroy: support PID owner guard Date: Thu, 31 Dec 2020 13:51:54 +0000 Message-Id: <20201231135154.6070-37-e@80x24.org> In-Reply-To: <20201231135154.6070-1-e@80x24.org> References: <20201231135154.6070-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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. --- lib/PublicInbox/LEI.pm | 5 ++--- lib/PublicInbox/Lock.pm | 4 ++-- lib/PublicInbox/OnDestroy.pm | 5 +++++ script/public-inbox-init | 2 +- t/on_destroy.t | 9 +++++++++ 5 files changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index b84e24ef..4af85d49 100644 --- 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 f6eaa5ce..1d0b4f9c 100644 --- 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 841f87d4..65ebd7dc 100644 --- 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 693f5ca1..222d0c60 100755 --- 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 8b85b48e..0de67d0b 100644 --- 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));