From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 4A5B11FA37 for ; Tue, 17 Jan 2023 07:19:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1673939953; bh=VyuBPBQQq5/JYgziIuUprI8Kw9b2Oj+mMflnRVKXFdk=; h=From:To:Subject:Date:In-Reply-To:References:From; b=ht3WP7rbu3H2SydU++yAPWMZZfhX47KuUNZaloK0raeINbk8QX0GuHujUYk9T8oFT KNieCUVDBS15AeYEb96feBfpztDbNgAcoorlzSP63s1/N+JH1hKY8M0XOaPYHO4bp+ naiPAtBBoKQlRQUiqUmpz0/5Xp1McnWU529/RSZ4= From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 12/12] ds: drop dwaitpid, switch to waitpid(-1) Date: Tue, 17 Jan 2023 07:19:11 +0000 Message-Id: <20230117071911.1577890-13-e@80x24.org> In-Reply-To: <20230117071911.1577890-1-e@80x24.org> References: <20230117071911.1577890-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: With no remaining users, we can drop dwaitpid and switch awaitpid to rely on waitpid(-1) to save syscalls. --- Documentation/technical/ds.txt | 2 +- lib/PublicInbox/DS.pm | 68 +++++++--------------------------- 2 files changed, 15 insertions(+), 55 deletions(-) diff --git a/Documentation/technical/ds.txt b/Documentation/technical/ds.txt index 5a1655a1..89cc05af 100644 --- a/Documentation/technical/ds.txt +++ b/Documentation/technical/ds.txt @@ -81,7 +81,7 @@ New features * IO::Socket::SSL support (for NNTPS, STARTTLS+NNTP, HTTPS) -* dwaitpid (waitpid wrapper) support for reaping dead children +* awaitpid (waitpid wrapper) support for reaping dead children * reliable signal wakeups are supported via signalfd on Linux, EVFILT_SIGNAL on *BSDs via IO::KQueue. diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm index 9563a1cb..c849f515 100644 --- a/lib/PublicInbox/DS.pm +++ b/lib/PublicInbox/DS.pm @@ -32,11 +32,10 @@ use PublicInbox::Syscall qw(:epoll); use PublicInbox::Tmpfile; use Errno qw(EAGAIN EINVAL); use Carp qw(carp croak); -our @EXPORT_OK = qw(now msg_more dwaitpid awaitpid add_timer add_uniq_timer); +our @EXPORT_OK = qw(now msg_more awaitpid add_timer add_uniq_timer); my %Stack; my $nextq; # queue for next_tick -my $wait_pids; # list of [ pid, callback, callback_arg ] my $AWAIT_PIDS; # pid => [ $callback, @args ] my $reap_armed; my $ToClose; # sockets to close when event loop is done @@ -75,11 +74,11 @@ sub Reset { # we may be iterating inside one of these on our stack my @q = delete @Stack{keys %Stack}; for my $q (@q) { @$q = () } - $AWAIT_PIDS = $wait_pids = $nextq = $ToClose = undef; + $AWAIT_PIDS = $nextq = $ToClose = undef; $ep_io = undef; # closes real $Epoll FD $Epoll = undef; # may call DSKQXS::DESTROY - } while (@Timers || keys(%Stack) || $nextq || $wait_pids || - $ToClose || keys(%DescriptorMap) || $AWAIT_PIDS || + } while (@Timers || keys(%Stack) || $nextq || $AWAIT_PIDS || + $ToClose || keys(%DescriptorMap) || $PostLoopCallback || keys(%UniqTimer)); $reap_armed = undef; @@ -209,43 +208,23 @@ sub await_cb ($;@) { warn "E: awaitpid($pid): $@" if $@; } -# We can't use waitpid(-1) safely here since it can hit ``, system(), -# and other things. So we scan the $wait_pids list, which is hopefully -# not too big. We keep $wait_pids small by not calling dwaitpid() -# until we've hit EOF when reading the stdout of the child. - +# This relies on our Perl process is single-threaded, or at least +# no threads are spawning and waiting on processes (``, system(), etc...) +# Threads are officially discouraged by the Perl5 team, and I expect +# that to remain the case. sub reap_pids { $reap_armed = undef; - my $tmp = $wait_pids // []; - $wait_pids = undef; - $Stack{reap_runq} = $tmp; my $oldset = block_signals(); - - # old API - foreach my $ary (@$tmp) { - my ($pid, $cb, $arg) = @$ary; - my $ret = waitpid($pid, WNOHANG); - if ($ret == 0) { - push @$wait_pids, $ary; # autovivifies @$wait_pids - } elsif ($ret == $pid) { - if ($cb) { - eval { $cb->($arg, $pid) }; - warn "E: dwaitpid($pid) in_loop: $@" if $@; - } + while (1) { + my $pid = waitpid(-1, WNOHANG) // last; + last if $pid <= 0; + if (defined(my $cb_args = delete $AWAIT_PIDS->{$pid})) { + await_cb($pid, @$cb_args) if $cb_args; } else { - warn "waitpid($pid, WNOHANG) = $ret, \$!=$!, \$?=$?"; + warn "W: reaped unknown PID=$pid: \$?=$?\n"; } } - - # new API TODO: convert to waitpid(-1) in the future as long - # as we don't use threads - for my $pid (keys %$AWAIT_PIDS) { - my $wpid = waitpid($pid, WNOHANG) // next; - my $cb_args = delete $AWAIT_PIDS->{$wpid} or next; - await_cb($pid, @$cb_args); - } sig_setmask($oldset); - delete $Stack{reap_runq}; } # reentrant SIGCHLD handler (since reap_pids is not reentrant) @@ -719,25 +698,6 @@ sub long_response ($$;@) { undef; } -sub dwaitpid ($;$$) { - my ($pid, $cb, $arg) = @_; - if ($in_loop) { - push @$wait_pids, [ $pid, $cb, $arg ]; - # We could've just missed our SIGCHLD, cover it, here: - enqueue_reap(); - } else { - my $ret = waitpid($pid, 0); - if ($ret == $pid) { - if ($cb) { - eval { $cb->($arg, $pid) }; - carp "E: dwaitpid($pid) !in_loop: $@" if $@; - } - } else { - carp "waitpid($pid, 0) = $ret, \$!=$!, \$?=$?"; - } - } -} - sub awaitpid { my ($pid, @cb_args) = @_; $AWAIT_PIDS->{$pid} //= @cb_args ? \@cb_args : 0;