]> Sergey Matveev's repositories - public-inbox.git/commitdiff
avoid calling waitpid from children in DESTROY
authorEric Wong <e@80x24.org>
Thu, 31 Dec 2020 13:51:52 +0000 (13:51 +0000)
committerEric Wong <e@80x24.org>
Fri, 1 Jan 2021 05:00:40 +0000 (05:00 +0000)
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
lib/PublicInbox/Gcf2Client.pm
lib/PublicInbox/Git.pm

index 01c9abd4baded2fa7b336398bb98e73417d8392b..4f1558c7c07a36c392c723fbc7c56c5a14608af8 100644 (file)
@@ -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, \$!=$!, \$?=$?";
                }
        }
 }
index 10820852b21d02761a49048b1dc31e8bc9f597cd..54957cf38621ec21e90b4cdb747b8ee2c109119f 100644 (file)
@@ -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
index fdfe126932038fba455ef09e7a9789a8f0796b60..47928c550298eff0d3a604ed96d508466df7f336 100644 (file)
@@ -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 ($) {