* [PATCH 01/13] lei_xsearch: put query in process title for debugging
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
2023-11-09 10:09 ` [PATCH 02/13] lei: use cached $daemon_pid when possible Eric Wong
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
Having queries in the process titles makes it easier to diagnose
stuck queries due to IPC problems. This was used to diagnose
commit e97a30e7624d (lei: fix SIGPIPE on large result sets to pager)).
---
lib/PublicInbox/LeiXSearch.pm | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 6c8dfe10..ba8ff293 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -148,13 +148,13 @@ sub mset_progress {
sub query_one_mset { # for --threads and l2m w/o sort
my ($self, $ibxish) = @_;
- local $0 = "$0 query_one_mset";
my $lei = $self->{lei};
my ($srch, $over) = ($ibxish->search, $ibxish->over);
my $dir = $ibxish->{inboxdir} // $ibxish->{topdir};
return warn("$dir not indexed by Xapian\n") unless ($srch && $over);
bless $srch, 'PublicInbox::LeiSearch'; # for ->qparse_new
my $mo = { %{$lei->{mset_opt}} }; # copy
+ local $0 = "$0 1 $mo->{qstr}";
my $mset;
my $each_smsg = $lei->{ovv}->ovv_each_smsg_cb($lei);
my $can_kw = !!$ibxish->can('msg_keywords');
@@ -219,9 +219,9 @@ sub query_one_mset { # for --threads and l2m w/o sort
sub query_combined_mset { # non-parallel for non-"--threads" users
my ($self) = @_;
- local $0 = "$0 query_combined_mset";
my $lei = $self->{lei};
my $mo = { %{$lei->{mset_opt}} };
+ local $0 = "$0 C $mo->{qstr}";
my $mset;
for my $loc (locals($self)) {
attach_external($self, $loc);
@@ -312,12 +312,11 @@ sub fudge_qstr_time ($$$) {
sub query_remote_mboxrd {
my ($self, $uris) = @_;
- local $0 = "$0 query_remote_mboxrd";
local $SIG{TERM} = sub { exit(0) }; # for DESTROY (File::Temp, $reap)
my $lei = $self->{lei};
my $opt = $lei->{opt};
- chomp(my $qstr = $lei->{mset_opt}->{qstr});
- $qstr =~ s/[ \n\t]+/ /sg; # make URLs less ugly
+ my $qstr = $lei->{mset_opt}->{qstr};
+ local $0 = "$0 R $qstr";
my @qform = (x => 'm');
push(@qform, t => 1) if $opt->{threads};
open my $cerr, '+>', undef;
@@ -504,6 +503,9 @@ sub ipc_atfork_child {
sub do_query {
my ($self, $lei) = @_;
my $l2m = $lei->{l2m};
+ my $qstr = \($lei->{mset_opt}->{qstr});
+ chomp $$qstr;
+ $$qstr =~ s/[ \n\t]+/ /sg; # make URLs and $0 less ugly
my $ops = {
sigpipe_handler => [ $lei ],
fail_handler => [ $lei ],
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 02/13] lei: use cached $daemon_pid when possible
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
2023-11-09 10:09 ` [PATCH 01/13] lei_xsearch: put query in process title for debugging Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
2023-11-09 10:09 ` [PATCH 03/13] lei: reuse FDs atfork and close explicitly Eric Wong
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
->lei_daemon_pid can only be called in the top-level daemon
process when $daemon_pid is valid, so avoid a getpid(2) syscall
in those cases.
---
lib/PublicInbox/LEI.pm | 2 +-
lib/PublicInbox/LeiUp.pm | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 2832db63..f32e5bbc 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -927,7 +927,7 @@ sub _config {
run_wait($cmd, \%env, \%opt) ? ($err_ok ? undef : fail($self, $?)) : 1;
}
-sub lei_daemon_pid { puts shift, $$ }
+sub lei_daemon_pid { puts shift, $daemon_pid }
sub lei_daemon_kill {
my ($self) = @_;
diff --git a/lib/PublicInbox/LeiUp.pm b/lib/PublicInbox/LeiUp.pm
index cd2337b4..0faa180d 100644
--- a/lib/PublicInbox/LeiUp.pm
+++ b/lib/PublicInbox/LeiUp.pm
@@ -11,6 +11,7 @@ use PublicInbox::LeiSavedSearch; # OverIdx
use PublicInbox::DS;
use PublicInbox::PktOp;
use PublicInbox::LeiFinmsg;
+use PublicInbox::LEI;
my $REMOTE_RE = qr!\A(?:imap|http)s?://!i; # http(s) will be for JMAP
sub up1 ($$) {
@@ -92,7 +93,6 @@ sub redispatch_all ($$) {
$op_c->{ops} = { '' => [ $lei->can('dclose'), $lei ] };
my @first_batch = splice(@$upq, 0, $j); # initial parallelism
$lei->{-upq} = $upq;
- $lei->{daemon_pid} = $$;
$lei->event_step_init; # wait for client disconnects
for my $out (@first_batch) {
PublicInbox::DS::requeue(
@@ -212,8 +212,8 @@ sub event_step { # runs via PublicInbox::DS::requeue
sub DESTROY {
my ($self) = @_;
+ return if ($PublicInbox::LEI::daemon_pid // -1) != $$;
my $lei = $self->{lei}; # the original, from lei_up
- return if $lei->{daemon_pid} != $$;
my $sock = delete $self->{unref_on_destroy};
my $s = $lei->{-socks} // [];
@$s = grep { $_ != $sock } @$s;
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 03/13] lei: reuse FDs atfork and close explicitly
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
2023-11-09 10:09 ` [PATCH 01/13] lei_xsearch: put query in process title for debugging Eric Wong
2023-11-09 10:09 ` [PATCH 02/13] lei: use cached $daemon_pid when possible Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
2023-11-09 10:09 ` [PATCH 04/13] lei_up: use v5.12 Eric Wong
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
We'll avoid having a redundant STDERR FD open in lei workers,
and some explicit close() on `lei up' sockets reduces the
likelyhood of inadvertantly open FDs causing processes to
linger.
---
lib/PublicInbox/LEI.pm | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index f32e5bbc..681044c8 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -574,19 +574,20 @@ sub _lei_atfork_child {
my ($self, $persist) = @_;
# we need to explicitly close things which are on stack
my $cfg = $self->{cfg};
+ delete @$cfg{qw(-watches -lei_note_event)};
if ($persist) {
open $self->{3}, '<', '/';
fchdir($self);
close($_) for (grep(defined, delete @$self{qw(0 1 2 sock)}));
- delete @$cfg{qw(-lei_store -watches -lei_note_event)};
+ delete $cfg->{-lei_store};
} else { # worker, Net::NNTP (Net::Cmd) uses STDERR directly
- open STDERR, '+>&='.fileno($self->{2});
+ open STDERR, '+>&', $self->{2};
STDERR->autoflush(1);
+ $self->{2} = \*STDERR;
POSIX::setpgid(0, $$) // die "setpgid(0, $$): $!";
- delete @$cfg{qw(-watches -lei_note_event)};
}
close($_) for (grep(defined, delete @$self{qw(old_1 au_done)}));
- delete $self->{-socks};
+ close($_) for (@{delete($self->{-socks}) // []});
if (my $op_c = delete $self->{pkt_op_c}) {
close(delete $op_c->{sock});
}
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 04/13] lei_up: use v5.12
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
` (2 preceding siblings ...)
2023-11-09 10:09 ` [PATCH 03/13] lei: reuse FDs atfork and close explicitly Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
2023-11-09 10:09 ` [PATCH 05/13] net_nntp_socks: more comments around how it works Eric Wong
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
No unicode_strings dependencies here, AFAIK
---
lib/PublicInbox/LeiUp.pm | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/PublicInbox/LeiUp.pm b/lib/PublicInbox/LeiUp.pm
index 0faa180d..9931f017 100644
--- a/lib/PublicInbox/LeiUp.pm
+++ b/lib/PublicInbox/LeiUp.pm
@@ -3,8 +3,7 @@
# "lei up" - updates the result of "lei q --save"
package PublicInbox::LeiUp;
-use strict;
-use v5.10.1;
+use v5.12;
# n.b. we use LeiInput to setup IMAP auth
use parent qw(PublicInbox::IPC PublicInbox::LeiInput);
use PublicInbox::LeiSavedSearch; # OverIdx
@@ -174,8 +173,7 @@ no warnings 'once';
*ipc_atfork_child = \&PublicInbox::LeiInput::input_only_atfork_child;
package PublicInbox::LeiUp1; # for redispatch_all
-use strict;
-use v5.10.1;
+use v5.12;
sub nxt ($$$) {
my ($lei, $out, $op_p) = @_;
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 05/13] net_nntp_socks: more comments around how it works
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
` (3 preceding siblings ...)
2023-11-09 10:09 ` [PATCH 04/13] lei_up: use v5.12 Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
2023-11-09 10:09 ` [PATCH 06/13] lei ls-mail-source: gracefully handle network failures Eric Wong
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
This is convoluted as hell but I can't figure out a better way
to make Net::NNTP work with SOCKS.
---
lib/PublicInbox/NetNNTPSocks.pm | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/lib/PublicInbox/NetNNTPSocks.pm b/lib/PublicInbox/NetNNTPSocks.pm
index 5b15dd59..306dcacb 100644
--- a/lib/PublicInbox/NetNNTPSocks.pm
+++ b/lib/PublicInbox/NetNNTPSocks.pm
@@ -1,13 +1,13 @@
# Copyright (C) all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-# wrap Net::NNTP client with SOCKS support
+# wrap Net::NNTP client with SOCKS support. Convoluted, but AFAIK this
+# is the only way to get SOCKS working with Net::NNTP w/o LD_PRELOAD.
package PublicInbox::NetNNTPSocks;
use v5.12;
use Net::NNTP;
-our %OPT;
+our %OPT; # used to pass options between ->new_socks and our ->new
our @ISA = qw(IO::Socket::Socks);
-my @SOCKS_KEYS = qw(ProxyAddr ProxyPort SocksVersion SocksDebug SocksResolve);
# use this instead of Net::NNTP->new if using Proxy*
sub new_socks {
@@ -16,9 +16,10 @@ sub new_socks {
local @Net::NNTP::ISA = (qw(Net::Cmd), __PACKAGE__);
local %OPT = map {;
defined($opt{$_}) ? ($_ => $opt{$_}) : ()
- } @SOCKS_KEYS;
+ } qw(ProxyAddr ProxyPort SocksVersion SocksDebug SocksResolve);
no warnings 'uninitialized'; # needed for $SOCKS_ERROR
- Net::NNTP->new(%opt) // die "errors: \$!=$! SOCKS=",
+ my $ret = Net::NNTP->new(%opt); # calls PublicInbox::NetNNTPSocks::new
+ $ret // die "errors: \$!=$! SOCKS=",
eval('$IO::Socket::Socks::SOCKS_ERROR // ""'),
', SSL=',
(eval('IO::Socket::SSL->errstr') // ''), "\n";
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 06/13] lei ls-mail-source: gracefully handle network failures
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
` (4 preceding siblings ...)
2023-11-09 10:09 ` [PATCH 05/13] net_nntp_socks: more comments around how it works Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
2023-11-09 10:09 ` [PATCH 07/13] net: retry on EINTR and check for {quit} flag Eric Wong
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
All network connections may fail, so try to emit a helpful
error message instead of attempting to dispatch methods off
`undef'.
---
lib/PublicInbox/LeiLsMailSource.pm | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/LeiLsMailSource.pm b/lib/PublicInbox/LeiLsMailSource.pm
index 50799270..4b427b26 100644
--- a/lib/PublicInbox/LeiLsMailSource.pm
+++ b/lib/PublicInbox/LeiLsMailSource.pm
@@ -19,7 +19,8 @@ sub input_path_url { # overrides LeiInput version
if ($url =~ m!\Aimaps?://!i) {
my $uri = PublicInbox::URIimap->new($url);
my $sec = $lei->{net}->can('uri_section')->($uri);
- my $mic = $lei->{net}->mic_get($uri);
+ my $mic = $lei->{net}->mic_get($uri) or
+ return $lei->err("E: $uri");
my $l = $mic->folders_hash($uri->path); # server-side filter
@$l = map { $_->[2] } # undo Schwartzian transform below:
sort { $a->[0] cmp $b->[0] || $a->[1] <=> $b->[1] }
@@ -39,7 +40,8 @@ sub input_path_url { # overrides LeiInput version
}
} elsif ($url =~ m!\A(?:nntps?|s?news)://!i) {
my $uri = PublicInbox::URInntps->new($url);
- my $nn = $lei->{net}->nn_get($uri);
+ my $nn = $lei->{net}->nn_get($uri) or
+ return $lei->err("E: $uri");
my $l = $nn->newsgroups($uri->group); # name => description
my $sec = $lei->{net}->can('uri_section')->($uri);
if ($json) {
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 07/13] net: retry on EINTR and check for {quit} flag
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
` (5 preceding siblings ...)
2023-11-09 10:09 ` [PATCH 06/13] lei ls-mail-source: gracefully handle network failures Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
2023-11-09 10:09 ` [PATCH 08/13] lei_mirror: note missing local manifests are non-fatal Eric Wong
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
This should allow us to detect shutdown signals in -watch
more quickly and not unnecessarily fail on inconsequential
signals such as SIGWINCH.
---
lib/PublicInbox/NetNNTPSocks.pm | 1 +
lib/PublicInbox/NetReader.pm | 53 +++++++++++++++++++++++----------
lib/PublicInbox/Watch.pm | 2 +-
3 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/lib/PublicInbox/NetNNTPSocks.pm b/lib/PublicInbox/NetNNTPSocks.pm
index 306dcacb..d27efba1 100644
--- a/lib/PublicInbox/NetNNTPSocks.pm
+++ b/lib/PublicInbox/NetNNTPSocks.pm
@@ -19,6 +19,7 @@ sub new_socks {
} qw(ProxyAddr ProxyPort SocksVersion SocksDebug SocksResolve);
no warnings 'uninitialized'; # needed for $SOCKS_ERROR
my $ret = Net::NNTP->new(%opt); # calls PublicInbox::NetNNTPSocks::new
+ return $ret if $ret || $!{EINTR};
$ret // die "errors: \$!=$! SOCKS=",
eval('$IO::Socket::Socks::SOCKS_ERROR // ""'),
', SSL=',
diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm
index 95cf1de8..e3e5d596 100644
--- a/lib/PublicInbox/NetReader.pm
+++ b/lib/PublicInbox/NetReader.pm
@@ -60,6 +60,7 @@ sub mic_new ($$$$) {
my ($self, $mic_arg, $sec, $uri) = @_;
my %mic_arg = (%$mic_arg, Keepalive => 1);
my $sa = $self->{cfg_opt}->{$sec}->{-proxy_cfg} || $self->{-proxy_cli};
+ my ($mic, $s, $t);
if ($sa) {
# this `require' needed for worker[1..Inf], since socks_args
# only got called in worker[0]
@@ -68,24 +69,37 @@ sub mic_new ($$$$) {
$opt{SocksDebug} = 1 if $mic_arg{Debug};
$opt{ConnectAddr} = delete $mic_arg{Server};
$opt{ConnectPort} = delete $mic_arg{Port};
- my $s = IO::Socket::Socks->new(%opt) or die
- "E: <$uri> ".eval('$IO::Socket::Socks::SOCKS_ERROR');
+ do {
+ $! = 0;
+ $s = IO::Socket::Socks->new(%opt);
+ } until ($s || !$!{EINTR} || $self->{quit});
+ return if $self->{quit};
+ $s or die "E: <$uri> ".eval('$IO::Socket::Socks::SOCKS_ERROR');
+ $mic_arg{Socket} = $s;
if (my $o = delete $mic_arg{Ssl}) { # for imaps://
$o = mic_tls_opt($o, $opt{ConnectAddr});
- $s = IO::Socket::SSL->start_SSL($s, @$o) or die
- "E: <$uri> ".(IO::Socket::SSL->errstr // '');
+ do {
+ $! = 0;
+ $t = IO::Socket::SSL->start_SSL($s, @$o);
+ } until ($t || !$!{EINTR} || $self->{quit});
+ return if $self->{quit};
+ $t or die "E: <$uri> ".(IO::Socket::SSL->errstr // '');
+ $mic_arg{Socket} = $t;
} elsif ($o = $mic_arg{Starttls}) {
# Mail::IMAPClient will use this:
$mic_arg{Starttls} = mic_tls_opt($o, $opt{ConnectAddr});
}
- $mic_arg{Socket} = $s;
} elsif ($mic_arg{Ssl} || $mic_arg{Starttls}) {
for my $f (qw(Ssl Starttls)) {
my $o = $mic_arg{$f} or next;
$mic_arg{$f} = mic_tls_opt($o, $mic_arg{Server});
}
}
- PublicInbox::IMAPClient->new(%mic_arg);
+ do {
+ $! = 0;
+ $mic = PublicInbox::IMAPClient->new(%mic_arg);
+ } until ($mic || !$!{EINTR} || $self->{quit});
+ $mic;
}
sub auth_anon_cb { '' }; # for Mail::IMAPClient::Authcallback
@@ -152,6 +166,7 @@ sub mic_for ($$$$) { # mic = Mail::IMAPClient
};
require PublicInbox::IMAPClient;
my $mic = mic_new($self, $mic_arg, $sec, $uri);
+ return if $self->{quit};
($mic && $mic->IsConnected) or
die "E: <$uri> new: $@".onion_hint($lei, $uri);
@@ -202,16 +217,20 @@ sub mic_for ($$$$) { # mic = Mail::IMAPClient
$mic;
}
-sub nn_new ($$$) {
- my ($nn_arg, $nntp_cfg, $uri) = @_;
+sub nn_new ($$$$) {
+ my ($self, $nn_arg, $nntp_cfg, $uri) = @_;
my $nn;
+ my ($Net_NNTP, $new) = qw(Net::NNTP new);
if (defined $nn_arg->{ProxyAddr}) {
require PublicInbox::NetNNTPSocks;
+ ($Net_NNTP, $new) = qw(PublicInbox::NetNNTPSocks new_socks);
$nn_arg->{SocksDebug} = 1 if $nn_arg->{Debug};
- $nn = PublicInbox::NetNNTPSocks->new_socks(%$nn_arg) or return;
- } else {
- $nn = Net::NNTP->new(%$nn_arg) or return;
}
+ do {
+ $! = 0;
+ $nn = $Net_NNTP->$new(%$nn_arg);
+ } until ($nn || !$!{EINTR} || $self->{quit});
+ $nn // return;
setsockopt($nn, Socket::SOL_SOCKET(), Socket::SO_KEEPALIVE(), 1);
# default to using STARTTLS if it's available, but allow
@@ -268,8 +287,9 @@ sub nn_for ($$$$) { # nn = Net::NNTP
$nn_arg->{SSL} = 1 if $uri->secure; # snews == nntps
my $sa = $self->{-proxy_cli};
%$nn_arg = (%$nn_arg, %$sa) if $sa;
- my $nn = nn_new($nn_arg, $nntp_cfg, $uri) or
- die "E: <$uri> new: $@".onion_hint($lei, $uri);
+ my $nn = nn_new($self, $nn_arg, $nntp_cfg, $uri);
+ return if $self->{quit};
+ $nn // die "E: <$uri> new: $@".onion_hint($lei, $uri);
if ($cred) {
$cred->fill($lei) unless defined($p); # may prompt user here
if ($nn->authinfo($u, $p)) {
@@ -382,8 +402,9 @@ sub imap_common_init ($;$) {
my $sec = uri_section($orig_uri);
my $uri = PublicInbox::URIimap->new("$sec/");
my $mic = $mics->{$sec} //=
- mic_for($self, $uri, $mic_common, $lei) //
- die "Unable to continue\n";
+ mic_for($self, $uri, $mic_common, $lei);
+ return if $self->{quit};
+ $mic // die "Unable to continue\n";
next unless $self->isa('PublicInbox::NetWriter');
next if $self->{-skip_creat};
my $dst = $orig_uri->mailbox // next;
@@ -751,7 +772,7 @@ sub nn_get {
my $nn_arg = $self->{net_arg}->{$sec} or
die "BUG: no Net::NNTP->new arg for $sec";
my $nntp_cfg = $self->{cfg_opt}->{$sec};
- $nn = nn_new($nn_arg, $nntp_cfg, $uri) or return;
+ $nn = nn_new($self, $nn_arg, $nntp_cfg, $uri) or return;
if (my $postconn = $nntp_cfg->{-postconn}) {
for my $m_arg (@$postconn) {
my ($method, @args) = @$m_arg;
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index c2b1312a..1cdf12a5 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -494,7 +494,7 @@ sub poll_fetch_reap { # awaitpid callback
sub watch_imap_init ($$) {
my ($self, $poll) = @_;
- my $mics = PublicInbox::NetReader::imap_common_init($self);
+ my $mics = PublicInbox::NetReader::imap_common_init($self) or return;
my $idle = []; # [ uri1, intvl1, uri2, intvl2 ]
for my $uri (@{$self->{imap_order}}) {
my $sec = uri_section($uri);
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 08/13] lei_mirror: note missing local manifests are non-fatal
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
` (6 preceding siblings ...)
2023-11-09 10:09 ` [PATCH 07/13] net: retry on EINTR and check for {quit} flag Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
2023-11-09 10:09 ` [PATCH 09/13] ipc: simplify partial sendmsg fallback Eric Wong
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
Sometimes seeing that warning is alarming.
---
lib/PublicInbox/LeiMirror.pm | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 49febe9e..84266d03 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -950,10 +950,11 @@ sub load_current_manifest ($) {
if (CORE::open(my $fh, '<', $fn)) {
decode_manifest($fh, $fn, $fn);
} elsif ($!{ENOENT}) { # non-fatal, we can just do it slowly
- warn "open($fn): $!\n" if !$self->{-initial_clone};
+ $self->{-initial_clone} or
+ warn "W: open($fn): $! (non-fatal)\n";
undef;
} else {
- die "open($fn): $!\n";
+ die "E: open($fn): $!\n";
}
}
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 09/13] ipc: simplify partial sendmsg fallback
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
` (7 preceding siblings ...)
2023-11-09 10:09 ` [PATCH 08/13] lei_mirror: note missing local manifests are non-fatal Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
2023-11-09 10:09 ` [PATCH 10/13] lei_input: always close single `eml' inputs Eric Wong
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
In the rare case sendmsg(2) isn't able to send the full amount
(due to buffers >=2GB on Linux), use print + (autodie)close
to send the remainder and retry on EINTR. `substr' should be
able to avoid a large malloc via offsets and CoW on modern Perl.
---
lib/PublicInbox/IPC.pm | 13 +++----------
t/ipc.t | 7 +++++++
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index 3292d960..a5cae6f2 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -10,7 +10,7 @@
package PublicInbox::IPC;
use v5.12;
use parent qw(Exporter);
-use autodie qw(fork pipe read socketpair sysread);
+use autodie qw(close fork pipe read socketpair sysread);
use Carp qw(croak);
use PublicInbox::DS qw(awaitpid);
use PublicInbox::Spawn;
@@ -266,15 +266,8 @@ sub stream_in_full ($$$) {
0) // croak "sendmsg: $!";
undef $r;
$n = $send_cmd->($w, $fds, $buf, 0) // croak "sendmsg: $!";
- while ($n < length($buf)) {
- my $x = syswrite($w, $buf, length($buf) - $n, $n);
- if (!defined($n)) {
- next if $!{EINTR};
- croak "syswrite: $!";
- }
- $x or croak "syswrite wrote 0 bytes";
- $n += $x;
- }
+ print $w substr($buf, $n) if $n < length($buf); # need > 2G on Linux
+ close $w; # autodies
}
sub wq_io_do { # always async
diff --git a/t/ipc.t b/t/ipc.t
index 519ef089..23ae2e7b 100644
--- a/t/ipc.t
+++ b/t/ipc.t
@@ -132,6 +132,13 @@ for my $t ('worker', 'worker again') {
$exp = sha1_hex($bigger)."\n";
is(readline($rb), $exp, "SHA WQWorker limit ($t)");
}
+ SKIP: {
+ $ENV{TEST_EXPENSIVE} or skip 'TEST_EXPENSIVE not set', 1;
+ my $bigger = $big x 75000; # over 2G to trigger partial sendmsg
+ $ipc->wq_io_do('test_sha', [ $wa, $wb ], $bigger);
+ my $exp = sha1_hex($bigger)."\n";
+ is(readline($rb), $exp, "SHA WQWorker sendmsg limit ($t)");
+ }
}
# wq_io_do works across fork (siblings can feed)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 10/13] lei_input: always close single `eml' inputs
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
` (8 preceding siblings ...)
2023-11-09 10:09 ` [PATCH 09/13] ipc: simplify partial sendmsg fallback Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
2023-11-09 10:09 ` [PATCH 11/13] xapcmd: get rid of scalar wantarray popen_rd Eric Wong
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
This matches the behavior we have for multi-message mbox files
since we rely on ->close to detect errors on bad mboxes. This
ensures we'll notice errors reading single messages from stdin.
We'll also start relying more on strace error injection to test
error handling.
---
lib/PublicInbox/LeiInput.pm | 13 +++++++------
lib/PublicInbox/TestCommon.pm | 23 ++++++++++++++++++++++-
t/io.t | 8 +-------
t/lei-import.t | 27 +++++++++++++++++++++++++++
4 files changed, 57 insertions(+), 14 deletions(-)
diff --git a/lib/PublicInbox/LeiInput.pm b/lib/PublicInbox/LeiInput.pm
index 4cd18c09..adb356c9 100644
--- a/lib/PublicInbox/LeiInput.pm
+++ b/lib/PublicInbox/LeiInput.pm
@@ -81,9 +81,11 @@ sub input_fh {
my ($self, $ifmt, $fh, $name, @args) = @_;
if ($ifmt eq 'eml') {
my $buf = do { local $/; <$fh> };
- (defined($buf) && eof($fh) && close($fh)) or
- return $self->{lei}->child_error(0, <<"");
-error reading $name: $!
+ my $ok = defined($buf) ? 1 : 0;
+ ++$ok if eof($fh);
+ ++$ok if $fh->close;
+ $ok == 3 or return $self->{lei}->child_error($?, <<"");
+error reading $name: $! (\$?=$?)
PublicInbox::Eml::strip_from($buf);
@@ -246,9 +248,8 @@ sub input_path_url {
my $rdr = { 2 => $lei->{2} };
my $fh = popen_rd($fp, undef, $rdr);
eval { $self->input_fh('eml', $fh, $input, @args) };
- my @err = ($@ ? $@ : ());
- $fh->close or push @err, "\$?=$?";
- $lei->child_error($?, "@$fp failed: @err") if @err;
+ my $err = $@ ? ": $@" : '';
+ $lei->child_error($?, "@$fp failed$err") if $err || $?;
} else {
$self->folder_missing("$ifmt:$input");
}
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 83e99b42..46e6a538 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -28,7 +28,8 @@ BEGIN {
quit_waiter_pipe wait_for_eof
tcp_host_port test_lei lei lei_ok $lei_out $lei_err $lei_opt
test_httpd xbail require_cmd is_xdeeply tail_f
- ignore_inline_c_missing no_pollerfd no_coredump cfg_new);
+ ignore_inline_c_missing no_pollerfd no_coredump cfg_new
+ strace strace_inject);
require Test::More;
my @methods = grep(!/\W/, @Test::More::EXPORT);
eval(join('', map { "*$_=\\&Test::More::$_;" } @methods));
@@ -933,6 +934,26 @@ sub cfg_new ($;@) {
PublicInbox::Config->new($f);
}
+our $strace_cmd;
+sub strace () {
+ skip 'linux only test' if $^O ne 'linux';
+ require_cmd('strace', 1);
+}
+
+sub strace_inject () {
+ my $cmd = strace;
+ state $ver = do {
+ require PublicInbox::Spawn;
+ my $v = PublicInbox::Spawn::run_qx([$cmd, '--version']);
+ $v =~ m!version\s+([1-9]+\.[0-9]+)! or
+ xbail "no strace --version: $v";
+ eval("v$1");
+ };
+ $ver ge v4.16 or skip "$cmd too old for syscall injection (".
+ sprintf('v%vd', $ver). ' < v4.16)';
+ $cmd
+}
+
package PublicInbox::TestCommon::InboxWakeup;
use strict;
sub on_inbox_unlock { ${$_[0]}->($_[1]) }
diff --git a/t/io.t b/t/io.t
index 4c7a97a3..3ea00ed8 100644
--- a/t/io.t
+++ b/t/io.t
@@ -9,13 +9,7 @@ use PublicInbox::Spawn qw(which run_qx);
# only test failures
SKIP: {
-skip 'linux only test' if $^O ne 'linux';
-my $strace = which('strace') or skip 'strace missing for test';
-my $v = run_qx([$strace, '--version']);
-$v =~ m!version\s+([1-9]+\.[0-9]+)! or xbail "no strace --version: $v";
-$v = eval("v$1");
-$v ge v4.16 or skip "$strace too old for syscall injection (".
- sprintf('v%vd', $v). ' < v4.16)';
+my $strace = strace_inject;
my $env = { PERL5LIB => join(':', @INC) };
my $opt = { 1 => \my $out, 2 => \my $err };
my $dst = "$tmpdir/dst";
diff --git a/t/lei-import.t b/t/lei-import.t
index b2c1de9b..6ad4c97b 100644
--- a/t/lei-import.t
+++ b/t/lei-import.t
@@ -154,6 +154,33 @@ do {
} until (!lei('ls-label') || $lei_out =~ /\bbin\b/ || now > $end);
like($lei_out, qr/\bbin\b/, 'commit-delay eventually commits');
+SKIP: {
+ my $strace = strace_inject; # skips if strace is old or non-Linux
+ my $tmpdir = tmpdir;
+ my $tr = "$tmpdir/tr";
+ my $cmd = [ $strace, "-o$tr", '-f',
+ "-P", File::Spec->rel2abs('t/plack-qp.eml'),
+ '-e', 'inject=readv,read:error=EIO'];
+ lei_ok qw(daemon-pid);
+ chomp(my $daemon_pid = $lei_out);
+ push @$cmd, '-p', $daemon_pid;
+ my $strace_opt = { 1 => \my $out, 2 => \my $err };
+ require PublicInbox::Spawn;
+ require PublicInbox::AutoReap;
+ my $pid = PublicInbox::Spawn::spawn($cmd, \%ENV, $strace_opt);
+ my $ar = PublicInbox::AutoReap->new($pid);
+ tick; # wait for strace to attach
+ ok(!lei(qw(import -F eml t/plack-qp.eml)),
+ '-F eml import fails on pathname error injection');
+ like($lei_err, qr!error reading t/plack-qp\.eml: Input/output error!,
+ 'EIO noted in stderr');
+ open $fh, '<', 't/plack-qp.eml';
+ ok(!lei(qw(import -F eml -), undef, { %$lei_opt, 0 => $fh }),
+ '-F eml import fails on stdin error injection');
+ like($lei_err, qr!error reading .*?: Input/output error!,
+ 'EIO noted in stderr');
+}
+
# see t/lei_to_mail.t for "import -F mbox*"
});
done_testing;
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 11/13] xapcmd: get rid of scalar wantarray popen_rd
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
` (9 preceding siblings ...)
2023-11-09 10:09 ` [PATCH 10/13] lei_input: always close single `eml' inputs Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
2023-11-09 10:09 ` [PATCH 12/13] lei: get rid of autoreap usage Eric Wong
2023-11-09 10:09 ` [PATCH 13/13] spawn: get rid of wantarray popen_rd/popen_wr Eric Wong
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
We can rely on Process::IO->attached_pid and work towards
simplifying popen_rd.
---
lib/PublicInbox/Xapcmd.pm | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index c2b66e69..69f0af43 100644
--- a/lib/PublicInbox/Xapcmd.pm
+++ b/lib/PublicInbox/Xapcmd.pm
@@ -329,8 +329,8 @@ sub progress_pfx ($) {
}
sub kill_compact { # setup_signals callback
- my ($sig, $pidref) = @_;
- kill($sig, $$pidref) if defined($$pidref);
+ my ($sig, $ioref) = @_;
+ kill($sig, $$ioref->attached_pid // return) if defined($$ioref);
}
# xapian-compact wrapper
@@ -358,18 +358,16 @@ sub compact ($$) { # cb_spawn callback
}
$pr->("$pfx `".join(' ', @$cmd)."'\n") if $pr;
push @$cmd, $src, $dst;
- my ($rd, $pid);
local @SIG{keys %SIG} = values %SIG;
- setup_signals(\&kill_compact, \$pid);
- ($rd, $pid) = popen_rd($cmd, undef, $rdr);
+ setup_signals(\&kill_compact, \my $rd);
+ $rd = popen_rd($cmd, undef, $rdr);
while (<$rd>) {
if ($pr) {
s/\r/\r$pfx /g;
$pr->("$pfx $_");
}
}
- waitpid($pid, 0);
- die "@$cmd failed: \$?=$?\n" if $?;
+ $rd->close or die "@$cmd failed: \$?=$?\n";
}
sub cpdb_loop ($$$;$$) {
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 12/13] lei: get rid of autoreap usage
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
` (10 preceding siblings ...)
2023-11-09 10:09 ` [PATCH 11/13] xapcmd: get rid of scalar wantarray popen_rd Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
2023-11-09 10:09 ` [PATCH 13/13] spawn: get rid of wantarray popen_rd/popen_wr Eric Wong
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
We can rely on Process::IO->DESTROY to close and reap
in these cases. This is the final step in eliminating
the wantarray invocations of popen_rd (and popen_wr).
---
lib/PublicInbox/LeiInput.pm | 13 +++++--------
lib/PublicInbox/LeiRemote.pm | 14 ++++++--------
lib/PublicInbox/LeiXSearch.pm | 15 +++++++++------
3 files changed, 20 insertions(+), 22 deletions(-)
diff --git a/lib/PublicInbox/LeiInput.pm b/lib/PublicInbox/LeiInput.pm
index adb356c9..68c3c459 100644
--- a/lib/PublicInbox/LeiInput.pm
+++ b/lib/PublicInbox/LeiInput.pm
@@ -7,7 +7,6 @@ use v5.12;
use PublicInbox::DS;
use PublicInbox::Spawn qw(which popen_rd);
use PublicInbox::InboxWritable qw(eml_from_path);
-use PublicInbox::AutoReap;
# JMAP RFC 8621 4.1.1
# https://www.iana.org/assignments/imap-jmap-keywords/imap-jmap-keywords.xhtml
@@ -114,15 +113,13 @@ sub handle_http_input ($$@) {
push @$curl, '-s', @$curl_opt;
my $cmd = $curl->for_uri($lei, $uri);
$lei->qerr("# $cmd");
- my ($fh, $pid) = popen_rd($cmd, undef, { 2 => $lei->{2} });
- my $ar = PublicInbox::AutoReap->new($pid);
+ my $fh = popen_rd($cmd, undef, { 2 => $lei->{2} });
grep(/\A--compressed\z/, @$curl) or
- $fh = IO::Uncompress::Gunzip->new($fh, MultiStream => 1);
+ $fh = IO::Uncompress::Gunzip->new($fh,
+ MultiStream => 1, AutoClose => 1);
eval { $self->input_fh('mboxrd', $fh, $url, @args) };
- my @err = ($@ ? $@ : ());
- $ar->join;
- push(@err, "\$?=$?") if $?;
- $lei->child_error($?, "@$cmd failed: @err") if @err;
+ my $err = $@ ? ": $@" : '';
+ $lei->child_error($?, "@$cmd failed$err") if $err || $?;
}
sub oid2eml { # git->cat_async cb
diff --git a/lib/PublicInbox/LeiRemote.pm b/lib/PublicInbox/LeiRemote.pm
index 54750062..559fb8d5 100644
--- a/lib/PublicInbox/LeiRemote.pm
+++ b/lib/PublicInbox/LeiRemote.pm
@@ -12,7 +12,6 @@ use IO::Uncompress::Gunzip;
use PublicInbox::MboxReader;
use PublicInbox::Spawn qw(popen_rd);
use PublicInbox::LeiCurl;
-use PublicInbox::AutoReap;
use PublicInbox::ContentHash qw(git_sha);
sub new {
@@ -22,7 +21,7 @@ sub new {
sub isrch { $_[0] } # SolverGit expcets this
-sub _each_mboxrd_eml { # callback for MboxReader->mboxrd
+sub each_mboxrd_eml { # callback for MboxReader->mboxrd
my ($eml, $self) = @_;
my $lei = $self->{lei};
my $xoids = $lei->{ale}->xoids_for($eml, 1);
@@ -47,14 +46,13 @@ sub mset {
$uri->query_form(q => $qstr, x => 'm', r => 1); # r=1: relevance
my $cmd = $curl->for_uri($self->{lei}, $uri);
$self->{lei}->qerr("# $cmd");
- my ($fh, $pid) = popen_rd($cmd, undef, { 2 => $lei->{2} });
- my $ar = PublicInbox::AutoReap->new($pid);
$self->{smsg} = [];
- $fh = IO::Uncompress::Gunzip->new($fh, MultiStream => 1);
- PublicInbox::MboxReader->mboxrd($fh, \&_each_mboxrd_eml, $self);
+ my $fh = popen_rd($cmd, undef, { 2 => $lei->{2} });
+ $fh = IO::Uncompress::Gunzip->new($fh, MultiStream=>1, AutoClose=>1);
+ eval { PublicInbox::MboxReader->mboxrd($fh, \&each_mboxrd_eml, $self) };
+ my $err = $@ ? ": $@" : '';
my $wait = $self->{lei}->{sto}->wq_do('done');
- $ar->join;
- $lei->child_error($?) if $?;
+ $lei->child_error($?, "@$cmd failed$err") if $err || $?;
$self; # we are the mset (and $ibx, and $self)
}
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index ba8ff293..b09c2462 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -346,14 +346,17 @@ print STDERR $_;
my $cmd = $curl->for_uri($lei, $uri);
$lei->qerr("# $cmd");
$rdr->{2} //= popen_wr(@lbf_tee) if @lbf_tee;
- my $cfh = popen_rd($cmd, undef, $rdr);
- my $fh = IO::Uncompress::Gunzip->new($cfh, MultiStream => 1);
- PublicInbox::MboxReader->mboxrd($fh, \&each_remote_eml, $self,
- $lei, $each_smsg);
+ my $fh = popen_rd($cmd, undef, $rdr);
+ $fh = IO::Uncompress::Gunzip->new($fh,
+ MultiStream => 1, AutoClose => 1);
+ eval {
+ PublicInbox::MboxReader->mboxrd($fh, \&each_remote_eml,
+ $self, $lei, $each_smsg);
+ };
+ my ($exc, $code) = ($@, $?);
$lei->sto_done_request if delete($self->{-sto_imported});
+ die "E: $exc" if $exc && !$code;
my $nr = delete $lei->{-nr_remote_eml} // 0;
- $cfh->close;
- my $code = $?;
if (!$code) { # don't update if no results, maybe MTA is down
$lei->{lss}->cfg_set($key, $start) if $key && $nr;
mset_progress($lei, $lei->{-current_url}, $nr, $nr);
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 13/13] spawn: get rid of wantarray popen_rd/popen_wr
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
` (11 preceding siblings ...)
2023-11-09 10:09 ` [PATCH 12/13] lei: get rid of autoreap usage Eric Wong
@ 2023-11-09 10:09 ` Eric Wong
12 siblings, 0 replies; 14+ messages in thread
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
We've updated all of our users to use Process::IO (and avoiding
tied handles) so the trade-off for using the array context
no longer exists.
---
lib/PublicInbox/Spawn.pm | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index 8c798b39..8cc4dfaf 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -384,16 +384,14 @@ sub spawn ($;$$) {
sub popen_rd {
my ($cmd, $env, $opt, @cb_arg) = @_;
pipe(my $r, local $opt->{1});
- my $pid = spawn($cmd, $env, $opt);
- wantarray ? ($r, $pid) : PublicInbox::IO::attach_pid($r, $pid, @cb_arg)
+ PublicInbox::IO::attach_pid($r, spawn($cmd, $env, $opt), @cb_arg);
}
sub popen_wr {
my ($cmd, $env, $opt, @cb_arg) = @_;
pipe(local $opt->{0}, my $w);
$w->autoflush(1);
- my $pid = spawn($cmd, $env, $opt);
- wantarray ? ($w, $pid) : PublicInbox::IO::attach_pid($w, $pid, @cb_arg)
+ PublicInbox::IO::attach_pid($w, spawn($cmd, $env, $opt), @cb_arg);
}
sub read_out_err ($) {
^ permalink raw reply related [flat|nested] 14+ messages in thread