* [PATCH] git: return upon self->close
@ 2023-11-20 8:46 7% Eric Wong
0 siblings, 0 replies; 3+ results
From: Eric Wong @ 2023-11-20 8:46 UTC (permalink / raw)
To: meta
I encountered the odd lack of `return' while chasing Gcf2 bugs
on CentOS 7.x which resulted in commit 7d06b126e939
("gcf2: fix autodie usage for older Perl") and commit e618c7654794
("gcf2client: add alias for PublicInbox::Git::fail") before
realizing the lack of `return' here wasn't the culprit behind
failures on CentOS 7.x.
However, the use of a `return' here appears required in case we
actually hit the error path, since falling through and
attempting my_readline with an undefined filehandle is always a
failure.
Fixes: e97a30e7624d ("lei: fix SIGPIPE on large result sets to pager")
---
lib/PublicInbox/Git.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 292c359a..bef524aa 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -276,7 +276,7 @@ sub cat_async_step ($$) {
sub cat_async_wait ($) {
my ($self) = @_;
- $self->close if !$self->{sock};
+ return $self->close if !$self->{sock};
my $inflight = $self->{inflight} or return;
while (scalar(@$inflight)) {
cat_async_step($self, $inflight);
@@ -332,7 +332,7 @@ sub check_async_wait ($) {
my ($self) = @_;
return cat_async_wait($self) if $self->{-bc};
my $ck = $self->{ck} or return;
- $ck->close if !$ck->{sock};
+ return $ck->close if !$ck->{sock};
my $inflight = $ck->{inflight} or return;
check_async_step($ck, $inflight) while (scalar(@$inflight));
}
^ permalink raw reply related [relevance 7%]
* [PATCH 01/13] lei_xsearch: put query in process title for debugging
@ 2023-11-09 10:09 6% ` Eric Wong
0 siblings, 0 replies; 3+ results
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 [relevance 6%]
* [PATCH] lei: fix SIGPIPE on large result sets to pager
@ 2023-11-07 13:01 6% Eric Wong
0 siblings, 0 replies; 3+ results
From: Eric Wong @ 2023-11-07 13:01 UTC (permalink / raw)
To: meta
When dealing with large search results, we need to deal with
EPIPE not just from the pager, but also EPIPE or ECONNRESET
between lei_xsearch and lei2mail processes.
Without this fix, lei_xsearch processes could linger and get
stuck writing to dead lei2mail processes if a user aborts the
pager early during a large result set.
To ensure lei_xsearch processes don't linger around after
lei2mail workers all die, we must close $l2m->{-wq_s2} before
spawning lei_xsearch processes, since $l2m->{-wq_s2} is only
used in lei2mail workers.
For `git cat-file' processes, we also need to trigger
PublicInbox::Git->close to handle unpredictable destructor
ordering to avoid using uninitialized IO refs. This combines
with the `git_to_mail' change to deal with process cleanup
handling from premature shutdowns.
To test all this, we can't just rely on a single message being
large, but also need to rely on the result set being large
enough to saturate the lei_xsearch -> lei2mail socket so we
rely on GIANT_INBOX_DIR once again.
---
lib/PublicInbox/Git.pm | 2 ++
lib/PublicInbox/LeiOverview.pm | 3 ++-
lib/PublicInbox/LeiToMail.pm | 8 +++++++-
lib/PublicInbox/LeiXSearch.pm | 3 ++-
t/lei-sigpipe.t | 27 ++++++++++++++++++++-------
5 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 11712db2..292c359a 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -276,6 +276,7 @@ sub cat_async_step ($$) {
sub cat_async_wait ($) {
my ($self) = @_;
+ $self->close if !$self->{sock};
my $inflight = $self->{inflight} or return;
while (scalar(@$inflight)) {
cat_async_step($self, $inflight);
@@ -331,6 +332,7 @@ sub check_async_wait ($) {
my ($self) = @_;
return cat_async_wait($self) if $self->{-bc};
my $ck = $self->{ck} or return;
+ $ck->close if !$ck->{sock};
my $inflight = $ck->{inflight} or return;
check_async_step($ck, $inflight) while (scalar(@$inflight));
}
diff --git a/lib/PublicInbox/LeiOverview.pm b/lib/PublicInbox/LeiOverview.pm
index 066c40bd..129dabf8 100644
--- a/lib/PublicInbox/LeiOverview.pm
+++ b/lib/PublicInbox/LeiOverview.pm
@@ -212,7 +212,8 @@ sub ovv_each_smsg_cb { # runs in wq worker usually
sub {
my ($smsg, $mitem, $eml) = @_;
$smsg->{pct} = get_pct($mitem) if $mitem;
- $l2m->wq_io_do('write_mail', [], $smsg, $eml);
+ eval { $l2m->wq_io_do('write_mail', [], $smsg, $eml) };
+ $lei->fail($@) if $@ && !$!{ECONNRESET} && !$!{EPIPE};
}
} elsif ($self->{fmt} =~ /\A(concat)?json\z/ && $lei->{opt}->{pretty}) {
my $EOR = ($1//'') eq 'concat' ? "\n}" : "\n},";
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index e80163e2..b73af68a 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -8,11 +8,13 @@ use v5.10.1;
use parent qw(PublicInbox::IPC);
use PublicInbox::Eml;
use PublicInbox::IO;
+use PublicInbox::Git;
use PublicInbox::Spawn qw(spawn);
use IO::Handle; # ->autoflush
use Fcntl qw(SEEK_SET SEEK_END O_CREAT O_EXCL O_WRONLY);
use PublicInbox::Syscall qw(rename_noreplace);
use autodie qw(open seek close);
+use Carp qw(croak);
my %kw2char = ( # Maildir characters
draft => 'D',
@@ -132,8 +134,12 @@ sub eml2mboxcl2 {
sub git_to_mail { # git->cat_async callback
my ($bref, $oid, $type, $size, $smsg) = @_;
- my $self = delete $smsg->{l2m} // die "BUG: no l2m";
$type // return; # called by PublicInbox::Git::close
+ my $self = delete $smsg->{l2m};
+ if (!defined($self)) {
+ return if $PublicInbox::Git::in_cleanup;
+ croak "BUG: no l2m (type=$type)";
+ }
eval {
if ($type eq 'missing' &&
($bref = $self->{-lms_rw}->local_blob($oid, 1))) {
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 5443188d..6c8dfe10 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -20,7 +20,7 @@ use PublicInbox::LEI;
use Fcntl qw(SEEK_SET F_SETFL O_APPEND O_RDWR);
use PublicInbox::ContentHash qw(git_sha);
use POSIX qw(strftime);
-use autodie qw(open read seek truncate);
+use autodie qw(close open read seek truncate);
use PublicInbox::Syscall qw($F_SETPIPE_SZ);
sub new {
@@ -543,6 +543,7 @@ sub do_query {
pipe($lei->{startq}, $lei->{au_done}) or die "pipe: $!";
fcntl($lei->{startq}, $F_SETPIPE_SZ, 4096) if $F_SETPIPE_SZ;
delete $l2m->{au_peers};
+ close(delete $l2m->{-wq_s2}); # share wq_s1 with lei_xsearch
}
$self->wq_workers_start('lei_xsearch', undef,
$lei->oldset, { lei => $lei },
diff --git a/t/lei-sigpipe.t b/t/lei-sigpipe.t
index 622598a4..1aa700e9 100644
--- a/t/lei-sigpipe.t
+++ b/t/lei-sigpipe.t
@@ -7,6 +7,19 @@ use PublicInbox::TestCommon;
use POSIX qw(WTERMSIG WIFSIGNALED SIGPIPE);
use PublicInbox::OnDestroy;
use PublicInbox::Syscall qw($F_SETPIPE_SZ);
+use autodie qw(close open pipe seek sysread);
+use PublicInbox::IO qw(write_file);
+my $inboxdir = $ENV{GIANT_INBOX_DIR};
+SKIP: {
+ $inboxdir // skip 'GIANT_INBOX_DIR unset to test large results';
+ require PublicInbox::Inbox;
+ my $ibx = PublicInbox::Inbox->new({
+ name => 'unconfigured-test',
+ address => [ "test\@example.com" ],
+ inboxdir => $inboxdir,
+ });
+ $ibx->search or xbail "GIANT_INBOX_DIR=$inboxdir has no search";
+}
# undo systemd (and similar) ignoring SIGPIPE, since lei expects to be run
# from an interactive terminal:
@@ -21,30 +34,30 @@ test_lei(sub {
my $f = "$ENV{HOME}/big.eml";
my $imported;
for my $out ([], [qw(-f mboxcl2)], [qw(-f text)]) {
- pipe(my ($r, $w)) or BAIL_OUT $!;
+ pipe(my $r, my $w);
my $size = $F_SETPIPE_SZ && fcntl($w, $F_SETPIPE_SZ, 4096) ?
4096 : 65536;
unless (-f $f) {
- open my $fh, '>', $f or xbail "open $f: $!";
- print $fh <<'EOM' or xbail;
+ my $fh = write_file '>', $f, <<'EOM';
From: big@example.com
Message-ID: <big@example.com>
EOM
print $fh 'Subject:';
print $fh (' '.('x' x 72)."\n") x (($size / 73) + 1);
print $fh "\nbody\n";
- close $fh or xbail "close: $!";
+ close $fh;
}
lei_ok(qw(import), $f) if $imported++ == 0;
- open my $errfh, '+>>', "$ENV{HOME}/stderr.log" or xbail $!;
+ open my $errfh, '+>>', "$ENV{HOME}/stderr.log";
my $opt = { run_mode => 0, 2 => $errfh, 1 => $w };
my $cmd = [qw(lei q -q -t), @$out, 'z:1..'];
+ push @$cmd, '--only='.$inboxdir if defined $inboxdir;
my $tp = start_script($cmd, undef, $opt);
close $w;
vec(my $rvec = '', fileno($r), 1) = 1;
if (!select($rvec, undef, undef, 30)) {
- seek($errfh, 0, 0) or xbail $!;
+ seek($errfh, 0, 0);
my $s = do { local $/; <$errfh> };
xbail "lei q had no output after 30s, stderr=$s";
}
@@ -53,7 +66,7 @@ EOM
$tp->join;
ok(WIFSIGNALED($?), "signaled @$out");
is(WTERMSIG($?), SIGPIPE, "got SIGPIPE @$out");
- seek($errfh, 0, 0) or xbail $!;
+ seek($errfh, 0, 0);
my $s = do { local $/; <$errfh> };
is($s, '', "quiet after sigpipe @$out");
}
^ permalink raw reply related [relevance 6%]
Results 1-3 of 3 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2023-11-07 13:01 6% [PATCH] lei: fix SIGPIPE on large result sets to pager Eric Wong
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
2023-11-09 10:09 6% ` [PATCH 01/13] lei_xsearch: put query in process title for debugging Eric Wong
2023-11-20 8:46 7% [PATCH] git: return upon self->close 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).