* [PATCH 10/19] Revert "ipc: add support for asynchronous callbacks"
2021-02-07 8:51 7% [PATCH 00/19] lei import Maildir, remote mboxrd fixes Eric Wong
@ 2021-02-07 8:51 5% ` Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2021-02-07 8:51 UTC (permalink / raw)
To: meta
This reverts commit a7e6a8cd68fb6d700337d8dbc7ee2c65ff3d2fc1.
It turns out to be unworkable in the face of multiple producer
processes, since the lock we make has no effect when calculating
pipe capacity.
---
lib/PublicInbox/IPC.pm | 52 +++---------------------------------------
t/ipc.t | 25 --------------------
2 files changed, 3 insertions(+), 74 deletions(-)
diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index 3713b56b..7e5a0b16 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -18,7 +18,6 @@ use PublicInbox::OnDestroy;
use PublicInbox::WQWorker;
use Socket qw(AF_UNIX MSG_EOR SOCK_STREAM);
my $SEQPACKET = eval { Socket::SOCK_SEQPACKET() }; # portable enough?
-use constant PIPE_BUF => $^O eq 'linux' ? 4096 : POSIX::_POSIX_PIPE_BUF();
our @EXPORT_OK = qw(ipc_freeze ipc_thaw);
my $WQ_MAX_WORKERS = 4096;
my ($enc, $dec);
@@ -59,15 +58,10 @@ sub _get_rec ($) {
ipc_thaw($buf);
}
-sub _pack_rec ($) {
- my ($ref) = @_;
- my $buf = ipc_freeze($ref);
- length($buf) . "\n" . $buf;
-}
-
sub _send_rec ($$) {
my ($w, $ref) = @_;
- print $w _pack_rec($ref) or croak "print: $!";
+ my $buf = ipc_freeze($ref);
+ print $w length($buf), "\n", $buf or croak "print: $!";
}
sub ipc_return ($$$) {
@@ -188,21 +182,6 @@ sub ipc_lock_init {
$self->{-ipc_lock} //= bless { lock_path => $f }, 'PublicInbox::Lock'
}
-sub ipc_async_wait ($$) {
- my ($self, $max) = @_; # max == -1 to wait for all
- my $aif = $self->{-async_inflight} or return;
- my $r_res = $self->{-ipc_res} or die 'BUG: no ipc_res';
- while (my ($sub, $bytes, $cb, $cb_arg) = splice(@$aif, 0, 4)) {
- my $ret = _get_rec($r_res) //
- die "no response on $sub (req.size=$bytes)";
- $self->{-async_inflight_bytes} -= $bytes;
-
- eval { $cb->($cb_arg, $ret) };
- warn "E: $sub callback error: $@\n" if $@;
- return if --$max == 0;
- }
-}
-
# call $self->$sub(@args), on a worker if ipc_worker_spawn was used
sub ipc_do {
my ($self, $sub, @args) = @_;
@@ -210,8 +189,7 @@ sub ipc_do {
my $ipc_lock = $self->{-ipc_lock};
my $lock = $ipc_lock ? $ipc_lock->lock_for_scope : undef;
if (defined(wantarray)) {
- my $r_res = $self->{-ipc_res} or die 'BUG: no ipc_res';
- ipc_async_wait($self, -1);
+ my $r_res = $self->{-ipc_res} or die 'no ipc_res';
_send_rec($w_req, [ wantarray, $sub, @args ]);
my $ret = _get_rec($r_res) // die "no response on $sub";
die $$ret if ref($ret) eq 'PublicInbox::IPC::Die';
@@ -224,30 +202,6 @@ sub ipc_do {
}
}
-sub ipc_async {
- my ($self, $sub, $sub_args, $cb, $cb_arg) = @_;
- if (my $w_req = $self->{-ipc_req}) { # run in worker
- my $rec = _pack_rec([ 1, $sub, @$sub_args ]);
- my $cur_bytes = \($self->{-async_inflight_bytes} //= 0);
- while (($$cur_bytes + length($rec)) > PIPE_BUF) {
- ipc_async_wait($self, 1);
- }
- my $ipc_lock = $self->{-ipc_lock};
- my $lock = $ipc_lock ? $ipc_lock->lock_for_scope : undef;
- print $w_req $rec or croak "print: $!";
- $$cur_bytes += length($rec);
- push @{$self->{-async_inflight}},
- $sub, length($rec), $cb, $cb_arg;
- } else {
- my $ret = [ eval { $self->$sub(@$sub_args) } ];
- if (my $exc = $@) {
- $ret = ( bless(\$exc, 'PublicInbox::IPC::Die') );
- }
- eval { $cb->($cb_arg, $ret) };
- warn "E: $sub callback error: $@\n" if $@;
- }
-}
-
# needed when there's multiple IPC workers and the parent forking
# causes newer siblings to inherit older siblings sockets
sub ipc_sibling_atfork_child {
diff --git a/t/ipc.t b/t/ipc.t
index 5801c760..face5726 100644
--- a/t/ipc.t
+++ b/t/ipc.t
@@ -37,7 +37,6 @@ my $ipc = bless {}, 'PublicInbox::IPC';
my @t = qw(array scalar scalarref undef);
my $test = sub {
my $x = shift;
- my @res;
for my $type (@t) {
my $m = "test_$type";
my @ret = $ipc->ipc_do($m);
@@ -46,34 +45,10 @@ my $test = sub {
$ipc->ipc_do($m);
- $ipc->ipc_async($m, [], sub { push @res, \@_ }, \$m);
-
my $ret = $ipc->ipc_do($m);
my $exp = $ipc->$m;
is_deeply($ret, $exp, "!wantarray $m $x");
-
- is_deeply(\@res, [ [ \$m, \@exp ] ], "async $m $x");
- @res = ();
}
- $ipc->ipc_async_wait(-1);
- is_deeply(\@res, [], 'no leftover results');
- $ipc->ipc_async('test_die', ['die test'],
- sub { push @res, \@_ }, 'die arg');
- $ipc->ipc_async_wait(1);
- is(scalar(@res), 1, 'only one result');
- is(scalar(@{$res[0]}), 2, 'result has 2-element array');
- is($res[0]->[0], 'die arg', 'got async die arg '.$x);
- is(ref($res[0]->[1]), 'PublicInbox::IPC::Die',
- "exception type $x");
- {
- my $nr = PublicInbox::IPC::PIPE_BUF();
- my $count = 0;
- my $cb = sub { ++$count };
- $ipc->ipc_async('test_undef', [], $cb) for (1..$nr);
- $ipc->ipc_async_wait(-1);
- is($count, $nr, "$x async runs w/o deadlock");
- }
-
my $ret = eval { $ipc->test_die('phail') };
my $exp = $@;
$ret = eval { $ipc->ipc_do('test_die', 'phail') };
^ permalink raw reply related [relevance 5%]
* [PATCH 00/19] lei import Maildir, remote mboxrd fixes
@ 2021-02-07 8:51 7% Eric Wong
2021-02-07 8:51 5% ` [PATCH 10/19] Revert "ipc: add support for asynchronous callbacks" Eric Wong
0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2021-02-07 8:51 UTC (permalink / raw)
To: meta
"lei q" with remote mboxrd + early MUA spawning is
nicer, too. Several risky constructs eliminated,
Interrupting "add-external --mirror" is less bad, now;
though it could probably support indexlevel=none in
case somebody wants to run index themselves.
Eric Wong (19):
spawn: pi_fork_exec: restore parent sigmask in child
spawn: pi_fork_exec: support "pgid"
lei add-external: handle interrupts with --mirror
spawn_pp: die more consistently in child
ipc: do not die inside wq_worker child process
ipc: trim down the Storable checks
Makefile.PL: depend on IO::Uncompress::Gunzip
xapcmd: avoid potential die surprise in children
tests: guard setup_public_inboxes for SQLite and Xapian
Revert "ipc: add support for asynchronous callbacks"
ipc: wq_do => wq_io_do
lei: more consistent IPC exit and error handling
lei: remove --mua-cmd alias for --mua
lei: replace --thread with --threads
lei q: improve remote mboxrd UX
lei q: SIGWINCH process group with the terminal
lei import: support Maildirs
imap: avoid unnecessary delete on stack
httpd/async: avoid unnecessary on-stack delete
Documentation/lei-q.pod | 4 +-
MANIFEST | 1 +
Makefile.PL | 1 +
lib/PublicInbox/HTTPD/Async.pm | 2 +-
lib/PublicInbox/IMAP.pm | 6 +-
lib/PublicInbox/IPC.pm | 105 +++++++-----------------
lib/PublicInbox/LEI.pm | 49 +++++++----
lib/PublicInbox/LeiCurl.pm | 11 ++-
lib/PublicInbox/LeiHelp.pm | 6 +-
lib/PublicInbox/LeiImport.pm | 38 ++++++---
lib/PublicInbox/LeiMirror.pm | 75 ++++++++++-------
lib/PublicInbox/LeiOverview.pm | 7 +-
lib/PublicInbox/LeiQuery.pm | 4 +-
lib/PublicInbox/LeiStore.pm | 8 +-
lib/PublicInbox/LeiToMail.pm | 37 ++++-----
lib/PublicInbox/LeiXSearch.pm | 143 ++++++++++++++++++++-------------
lib/PublicInbox/Mbox.pm | 2 +-
lib/PublicInbox/OnDestroy.pm | 2 +-
lib/PublicInbox/Search.pm | 2 +-
lib/PublicInbox/SearchView.pm | 2 +-
lib/PublicInbox/Spawn.pm | 63 +++++++++------
lib/PublicInbox/SpawnPP.pm | 44 +++++-----
lib/PublicInbox/Xapcmd.pm | 11 +--
script/lei | 8 +-
t/ipc.t | 39 ++-------
t/lei-externals.t | 2 +
t/lei-import-maildir.t | 33 ++++++++
t/lei-mirror.t | 14 ++++
t/lei.t | 2 +-
t/lei_to_mail.t | 6 +-
t/spawn.t | 18 +++++
xt/stress-sharedkv.t | 6 +-
32 files changed, 433 insertions(+), 318 deletions(-)
create mode 100644 t/lei-import-maildir.t
^ permalink raw reply [relevance 7%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2021-02-07 8:51 7% [PATCH 00/19] lei import Maildir, remote mboxrd fixes Eric Wong
2021-02-07 8:51 5% ` [PATCH 10/19] Revert "ipc: add support for asynchronous callbacks" 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).