user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
* [PATCH] ds: awaitpid: do not clobber entries for reaped processes
@ 2023-01-21  8:58 Eric Wong
  0 siblings, 0 replies; only message in thread
From: Eric Wong @ 2023-01-21  8:58 UTC (permalink / raw)
  To: meta

We must only write to $AWAIT_PIDS on the initial reap attempt.
While we're at it, avoid triggering an extra wakeup if we're
doing synchronous awaitpid.  This seems to eliminate most
reliance on Qspawn->DESTROY to call Qspawn->finalize.
---
 lib/PublicInbox/DS.pm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 523d47e4..0a763d0e 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -699,8 +699,8 @@ sub long_response ($$;@) {
 }
 
 sub awaitpid {
-	my ($pid, @cb_args) = @_;
-	$AWAIT_PIDS->{$pid} //= @cb_args ? \@cb_args : 0;
+	my ($pid, @cb_args) = @_; # @cb_args = ($cb, @args), $cb may be undef
+	$AWAIT_PIDS->{$pid} = \@cb_args if @cb_args;
 	# provide synchronous API
 	if (defined(wantarray) || (!$in_loop && !@cb_args)) {
 		my $ret;
@@ -716,9 +716,9 @@ again:
 			delete $AWAIT_PIDS->{$pid};
 		}
 		return $ret;
+	} elsif ($in_loop) { # We could've just missed our SIGCHLD, cover it, here:
+		enqueue_reap();
 	}
-	# We could've just missed our SIGCHLD, cover it, here:
-	enqueue_reap() if $in_loop;
 }
 
 1;

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2023-01-21  8:58 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-21  8:58 [PATCH] ds: awaitpid: do not clobber entries for reaped processes Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).