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 3DBFA20084 for ; Thu, 31 Dec 2020 13:51:58 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 34/36] avoid calling waitpid from children in DESTROY Date: Thu, 31 Dec 2020 13:51:52 +0000 Message-Id: <20201231135154.6070-35-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: Objects with DESTROY callbacks get propagated to children, so we must be careful to not invoke waitpid from children on their sibling processes. Only parents (and their parents...) can reap child processes. --- lib/PublicInbox/DS.pm | 4 ++-- lib/PublicInbox/Gcf2Client.pm | 8 ++++++-- lib/PublicInbox/Git.pm | 3 ++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm index 01c9abd4..4f1558c7 100644 --- a/lib/PublicInbox/DS.pm +++ b/lib/PublicInbox/DS.pm @@ -624,10 +624,10 @@ sub dwaitpid ($;$$) { if ($ret == $pid) { if ($cb) { eval { $cb->($arg, $pid) }; - warn "E: dwaitpid($pid) !in_loop: $@" if $@; + carp "E: dwaitpid($pid) !in_loop: $@" if $@; } } else { - warn "waitpid($pid, 0) = $ret, \$!=$!, \$?=$?"; + carp "waitpid($pid, 0) = $ret, \$!=$!, \$?=$?"; } } } diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm index 10820852..54957cf3 100644 --- a/lib/PublicInbox/Gcf2Client.pm +++ b/lib/PublicInbox/Gcf2Client.pm @@ -15,6 +15,7 @@ use PublicInbox::DS qw(dwaitpid); # sock => writable pipe to Gcf2::loop # in => pipe we read from # pid => PID of Gcf2::loop process +# owner_pid => process which spawned {pid} sub new { my ($rdr) = @_; my $self = bless {}, __PACKAGE__; @@ -25,6 +26,7 @@ sub new { $rdr //= {}; $rdr->{0} = $out_r; my $cmd = [$^X, qw[-MPublicInbox::Gcf2 -e PublicInbox::Gcf2::loop()]]; + $self->{owner_pid} = $$; @$self{qw(in pid)} = popen_rd($cmd, $env, $rdr); fcntl($out_w, 1031, 4096) if $^O eq 'linux'; # 1031: F_SETPIPE_SZ $out_w->autoflush(1); @@ -69,8 +71,10 @@ sub DESTROY { delete $self->{in}; # GitAsyncCat::event_step may reap us with WNOHANG, too my $pid = delete $self->{pid} or return; - PublicInbox::DS->in_loop ? $self->close : delete($self->{sock}); - dwaitpid $pid; + if ($$ == $self->{owner_pid}) { + PublicInbox::DS->in_loop ? $self->close : delete($self->{sock}); + dwaitpid $pid; + } } # used by GitAsyncCat diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm index fdfe1269..47928c55 100644 --- a/lib/PublicInbox/Git.pm +++ b/lib/PublicInbox/Git.pm @@ -126,6 +126,7 @@ sub _bidi_pipe { } my ($in_r, $p) = popen_rd(\@cmd, undef, $redir); $self->{$pid} = $p; + $self->{"$pid.owner"} = $$; $out_w->autoflush(1); if ($^O eq 'linux') { # 1031: F_SETPIPE_SZ fcntl($out_w, 1031, 4096); @@ -327,7 +328,7 @@ sub _destroy { # GitAsyncCat::event_step may delete {pid} my $p = delete $self->{$pid} or return; - dwaitpid $p; + dwaitpid($p) if $$ == $self->{"$pid.owner"}; } sub cat_async_abort ($) {