From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 31/36] use PublicInbox::DS for dwaitpid
Date: Thu, 31 Dec 2020 13:51:49 +0000 [thread overview]
Message-ID: <20201231135154.6070-32-e@80x24.org> (raw)
In-Reply-To: <20201231135154.6070-1-e@80x24.org>
This simplifies our code and provides a more consistent API for
error handling. PublicInbox::DS can be loaded nowadays on all
*BSDs and Linux distros easily without extra packages to
install.
The downside is possibly increased startup time, but it's
probably not as a big problem with lei being a daemon
(and -mda possibly following suite).
---
lib/PublicInbox/DS.pm | 39 ++++++++++++++++++++----------
lib/PublicInbox/Gcf2Client.pm | 16 ++++---------
lib/PublicInbox/Git.pm | 6 ++---
lib/PublicInbox/LEI.pm | 4 ++--
lib/PublicInbox/ProcessPipe.pm | 17 ++++++++------
lib/PublicInbox/Qspawn.pm | 43 ++++++++--------------------------
6 files changed, 56 insertions(+), 69 deletions(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 97a6f6ef..01c9abd4 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -21,19 +21,19 @@
# (tmpio = [ GLOB, offset, [ length ] ])
package PublicInbox::DS;
use strict;
+use v5.10.1;
+use parent qw(Exporter);
use bytes;
use POSIX qw(WNOHANG);
use IO::Handle qw();
use Fcntl qw(SEEK_SET :DEFAULT O_APPEND);
use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC);
-use parent qw(Exporter);
-our @EXPORT_OK = qw(now msg_more);
-use 5.010_001;
use Scalar::Util qw(blessed);
use PublicInbox::Syscall qw(:epoll);
use PublicInbox::Tmpfile;
use Errno qw(EAGAIN EINVAL);
use Carp qw(confess carp);
+our @EXPORT_OK = qw(now msg_more dwaitpid);
my $nextq; # queue for next_tick
my $wait_pids; # list of [ pid, callback, callback_arg ]
@@ -215,8 +215,13 @@ sub reap_pids {
my $ret = waitpid($pid, WNOHANG);
if ($ret == 0) {
push @$wait_pids, $ary; # autovivifies @$wait_pids
- } elsif ($cb) {
- eval { $cb->($arg, $pid) };
+ } elsif ($ret == $pid) {
+ if ($cb) {
+ eval { $cb->($arg, $pid) };
+ warn "E: dwaitpid($pid) in_loop: $@" if $@;
+ }
+ } else {
+ warn "waitpid($pid, WNOHANG) = $ret, \$!=$!, \$?=$?";
}
}
# we may not be done, yet, and could've missed/masked a SIGCHLD:
@@ -608,13 +613,23 @@ sub shutdn ($) {
}
}
-# must be called with eval, PublicInbox::DS may not be loaded (see t/qspawn.t)
-sub dwaitpid ($$$) {
- die "Not in EventLoop\n" unless $in_loop;
- push @$wait_pids, [ @_ ]; # [ $pid, $cb, $arg ]
-
- # We could've just missed our SIGCHLD, cover it, here:
- enqueue_reap();
+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) };
+ warn "E: dwaitpid($pid) !in_loop: $@" if $@;
+ }
+ } else {
+ warn "waitpid($pid, 0) = $ret, \$!=$!, \$?=$?";
+ }
+ }
}
sub _run_later () {
diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index 4bda5520..10820852 100644
--- a/lib/PublicInbox/Gcf2Client.pm
+++ b/lib/PublicInbox/Gcf2Client.pm
@@ -9,6 +9,7 @@ use PublicInbox::Git;
use PublicInbox::Spawn qw(popen_rd);
use IO::Handle ();
use PublicInbox::Syscall qw(EPOLLONESHOT);
+use PublicInbox::DS qw(dwaitpid);
# fields:
# async_cat => GitAsyncCat ref (read-only pipe)
# sock => writable pipe to Gcf2::loop
@@ -65,18 +66,11 @@ no warnings 'once';
sub DESTROY {
my ($self) = @_;
- my $pid = delete $self->{pid};
delete $self->{in};
- return unless $pid;
- eval {
- PublicInbox::DS::dwaitpid($pid, undef, undef);
- $self->close; # we're still in the event loop
- };
- if ($@) { # wait synchronously if not in event loop
- my $sock = delete $self->{sock};
- close $sock if $sock;
- waitpid($pid, 0);
- }
+ # 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;
}
# used by GitAsyncCat
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 73dc7d3e..fdfe1269 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -21,6 +21,7 @@ use PublicInbox::Tmpfile;
use IO::Poll qw(POLLIN);
use Carp qw(croak);
use Digest::SHA ();
+use PublicInbox::DS qw(dwaitpid);
our @EXPORT_OK = qw(git_unquote git_quote);
our $PIPE_BUFSIZ = 65536; # Linux default
our $in_cleanup;
@@ -326,10 +327,7 @@ sub _destroy {
# GitAsyncCat::event_step may delete {pid}
my $p = delete $self->{$pid} or return;
-
- # PublicInbox::DS may not be loaded
- eval { PublicInbox::DS::dwaitpid($p, undef, undef) };
- waitpid($p, 0) if $@; # wait synchronously if not in event loop
+ dwaitpid $p;
}
sub cat_async_abort ($) {
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 1ba9eff3..7b7f45de 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -18,7 +18,7 @@ use Sys::Syslog qw(syslog openlog);
use PublicInbox::Config;
use PublicInbox::Syscall qw($SFD_NONBLOCK EPOLLIN EPOLLONESHOT);
use PublicInbox::Sigfd;
-use PublicInbox::DS qw(now);
+use PublicInbox::DS qw(now dwaitpid);
use PublicInbox::Spawn qw(spawn run_die);
use PublicInbox::OnDestroy;
use Text::Wrap qw(wrap);
@@ -604,7 +604,7 @@ sub lei_git { # support passing through random git commands
my ($self, @argv) = @_;
my %rdr = map { $_ => $self->{$_} } (0..2);
my $pid = spawn(['git', @argv], $self->{env}, \%rdr);
- PublicInbox::DS::dwaitpid($pid, \&reap_exec, $self);
+ dwaitpid($pid, \&reap_exec, $self);
}
sub accept_dispatch { # Listener {post_accept} callback
diff --git a/lib/PublicInbox/ProcessPipe.pm b/lib/PublicInbox/ProcessPipe.pm
index c9234f42..afbb048d 100644
--- a/lib/PublicInbox/ProcessPipe.pm
+++ b/lib/PublicInbox/ProcessPipe.pm
@@ -5,6 +5,7 @@
package PublicInbox::ProcessPipe;
use strict;
use v5.10.1;
+use PublicInbox::DS qw(dwaitpid);
sub TIEHANDLE {
my ($class, $pid, $fh, $cb, $arg) = @_;
@@ -25,19 +26,21 @@ sub PRINT {
print { $self->{fh} } @_;
}
+sub adjust_ret { # dwaitpid callback
+ my ($retref, $pid) = @_;
+ $$retref = '' if $?
+}
+
sub CLOSE {
my $fh = delete($_[0]->{fh});
my $ret = defined $fh ? close($fh) : '';
my ($pid, $cb, $arg) = delete @{$_[0]}{qw(pid cb arg)};
if (defined $pid) {
- # PublicInbox::DS may not be loaded
- eval { PublicInbox::DS::dwaitpid($pid, $cb, $arg) };
-
- if ($@) { # ok, not in the event loop, work synchronously
- waitpid($pid, 0);
- $ret = '' if $?;
- $cb->($arg, $pid) if $cb;
+ unless ($cb) {
+ $cb = \&adjust_ret;
+ $arg = \$ret;
}
+ dwaitpid $pid, $cb, $arg;
}
$ret;
}
diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm
index 2aa2042a..5bbbb027 100644
--- a/lib/PublicInbox/Qspawn.pm
+++ b/lib/PublicInbox/Qspawn.pm
@@ -12,12 +12,13 @@
# operate in. This can be useful to ensure smaller inboxes can
# be cloned while cloning of large inboxes is maxed out.
#
-# This does not depend on PublicInbox::DS or any other external
-# scheduling mechanism, you just need to call start() and finish()
-# appropriately. However, public-inbox-httpd (which uses PublicInbox::DS)
-# will be able to schedule this based on readability of stdout from
-# the spawned process. See GitHTTPBackend.pm and SolverGit.pm for
-# usage examples. It does not depend on any form of threading.
+# This does not depend on the PublicInbox::DS->EventLoop or any
+# other external scheduling mechanism, you just need to call
+# start() and finish() appropriately. However, public-inbox-httpd
+# (which uses PublicInbox::DS) will be able to schedule this
+# based on readability of stdout from the spawned process.
+# See GitHTTPBackend.pm and SolverGit.pm for usage examples.
+# It does not depend on any form of threading.
#
# This is useful for scheduling CGI execution of both long-lived
# git-http-backend(1) process (for "git clone") as well as short-lived
@@ -27,6 +28,7 @@ package PublicInbox::Qspawn;
use strict;
use PublicInbox::Spawn qw(popen_rd);
use PublicInbox::GzipFilter;
+use PublicInbox::DS qw(dwaitpid); # doesn't need event loop
# n.b.: we get EAGAIN with public-inbox-httpd, and EINTR on other PSGI servers
use Errno qw(EAGAIN EINTR);
@@ -116,37 +118,12 @@ sub finalize ($$) {
}
# callback for dwaitpid
-sub waitpid_err ($$) {
- my ($self, $pid) = @_;
- my $xpid = delete $self->{pid};
- my $err;
- if (defined $pid) {
- if ($pid > 0) { # success!
- $err = child_err($?);
- } elsif ($pid < 0) { # ??? does this happen in our case?
- $err = "W: waitpid($xpid, 0) => $pid: $!";
- } # else should not be called with pid == 0
- }
- finalize($self, $err);
-}
-
-sub do_waitpid ($) {
- my ($self) = @_;
- my $pid = $self->{pid};
- # PublicInbox::DS may not be loaded
- eval { PublicInbox::DS::dwaitpid($pid, \&waitpid_err, $self) };
- # done if we're running in PublicInbox::DS::EventLoop
- if ($@) {
- # non public-inbox-{httpd,nntpd} callers may block:
- my $ret = waitpid($pid, 0);
- waitpid_err($self, $ret);
- }
-}
+sub waitpid_err { finalize($_[0], child_err($?)) }
sub finish ($;$) {
my ($self, $err) = @_;
if (delete $self->{rpipe}) {
- do_waitpid($self);
+ dwaitpid $self->{pid}, \&waitpid_err, $self;
} else {
finalize($self, $err);
}
next prev parent reply other threads:[~2020-12-31 13:51 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-31 13:51 [PATCH 00/36] another round of lei stuff Eric Wong
2020-12-31 13:51 ` [PATCH 01/36] import: respect init.defaultBranch Eric Wong
2020-12-31 13:51 ` [PATCH 02/36] lei_store: use per-machine refname as git HEAD Eric Wong
2020-12-31 13:51 ` [PATCH 03/36] revert "lei_store: use per-machine refname as git HEAD" Eric Wong
2020-12-31 13:51 ` [PATCH 04/36] lei_to_mail: initial implementation for writing mbox formats Eric Wong
2020-12-31 13:51 ` [PATCH 05/36] sharedkv: fork()-friendly key-value store Eric Wong
2020-12-31 13:51 ` [PATCH 06/36] sharedkv: split out index_values Eric Wong
2020-12-31 13:51 ` [PATCH 07/36] lei_to_mail: start atomic and compressed mbox writing Eric Wong
2020-12-31 13:51 ` [PATCH 08/36] mboxreader: new class for reading various mbox formats Eric Wong
2020-12-31 13:51 ` [PATCH 09/36] lei_to_mail: start --augment, dedupe, bz2 and xz Eric Wong
2020-12-31 13:51 ` [PATCH 10/36] lei: implement various deduplication strategies Eric Wong
2020-12-31 13:51 ` [PATCH 11/36] lei_to_mail: lazy-require LeiDedupe Eric Wong
2020-12-31 13:51 ` [PATCH 12/36] lei_to_mail: support for non-seekable outputs Eric Wong
2020-12-31 13:51 ` [PATCH 13/36] lei_to_mail: support Maildir, fix+test --augment Eric Wong
2020-12-31 13:51 ` [PATCH 14/36] ipc: generic IPC dispatch based on Storable Eric Wong
2020-12-31 13:51 ` [PATCH 15/36] ipc: support Sereal Eric Wong
2020-12-31 13:51 ` [PATCH 16/36] lei_store: add ->set_eml, ->add_eml can return smsg Eric Wong
2020-12-31 13:51 ` [PATCH 17/36] lei: rename "extinbox" => "external" Eric Wong
2020-12-31 13:51 ` [PATCH 18/36] mid: use defined-or with `push' for uniqueness check Eric Wong
2020-12-31 13:51 ` [PATCH 19/36] mid: hoist out mids_in sub Eric Wong
2020-12-31 13:51 ` [PATCH 20/36] lei_store: handle messages without Message-ID at all Eric Wong
2020-12-31 13:51 ` [PATCH 21/36] ipc: use shutdown(2), base atfork* callback Eric Wong
2020-12-31 13:51 ` [PATCH 22/36] lei_to_mail: unlink mboxes if not augmenting Eric Wong
2020-12-31 13:51 ` [PATCH 23/36] lei: add --mfolder as an --output alias Eric Wong
2020-12-31 13:51 ` [PATCH 24/36] spawn: move run_die here from PublicInbox::Import Eric Wong
2020-12-31 13:51 ` [PATCH 25/36] init: remove embedded UnlinkMe package Eric Wong
2020-12-31 13:51 ` [PATCH 26/36] t/run: avoid uninitialized var on incomplete test Eric Wong
2020-12-31 13:51 ` [PATCH 27/36] gcf2client: reap process on DESTROY Eric Wong
2020-12-31 13:51 ` [PATCH 28/36] lei_to_mail: open FIFOs O_WRONLY so we block Eric Wong
2020-12-31 13:51 ` [PATCH 29/36] searchidxshard: call DS->Reset at worker start Eric Wong
2020-12-31 13:51 ` [PATCH 30/36] t/ipc.t: test for references via `die' Eric Wong
2020-12-31 13:51 ` Eric Wong [this message]
2020-12-31 13:51 ` [PATCH 32/36] syscall: SFD_NONBLOCK can be a constant, again Eric Wong
2020-12-31 13:51 ` [PATCH 33/36] lei: avoid Spawn package when starting daemon Eric Wong
2020-12-31 13:51 ` [PATCH 34/36] avoid calling waitpid from children in DESTROY Eric Wong
2020-12-31 13:51 ` [PATCH 35/36] ds: clobber $in_loop first at reset Eric Wong
2020-12-31 13:51 ` [PATCH 36/36] on_destroy: support PID owner guard Eric Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://public-inbox.org/README
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201231135154.6070-32-e@80x24.org \
--to=e@80x24.org \
--cc=meta@public-inbox.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).