Most worrying was the the bug fixed in 4/5; but at least there wasn't data loss involved... While the bug fixed in 4/5 didn't cause data loss, it was dumping core files and filling up my disk while polluting the kernel log buffer. Eric Wong (5): lei: do not access {sock} after SIGPIPE lei_to_mail: limit workers for text, reply and v2 outputs lei_xsearch: quiet error message on SIG{PIPE,TERM} lei_to_mail: avoid SEGV on worker exit via SIGTERM doc: lei-security: add a note about core dumps Documentation/lei-security.pod | 6 ++++++ lib/PublicInbox/LEI.pm | 2 +- lib/PublicInbox/LeiQuery.pm | 2 +- lib/PublicInbox/LeiToMail.pm | 10 +++++++++- lib/PublicInbox/LeiXSearch.pm | 5 ++++- t/lei-sigpipe.t | 7 +++++-- 6 files changed, 26 insertions(+), 6 deletions(-)
It's possible for this to break out of the event loop if note_sigpipe fires via PktOp in the same iteration. --- lib/PublicInbox/LEI.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index 96f7c5e315a9..78b49a3bc1af 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -1127,7 +1127,7 @@ sub event_step { local %ENV = %{$self->{env}}; local $current_lei = $self; eval { - my @fds = $recv_cmd->($self->{sock}, my $buf, 4096); + my @fds = $recv_cmd->($self->{sock} // return, my $buf, 4096); if (scalar(@fds) == 1 && !defined($fds[0])) { return if $! == EAGAIN; die "recvmsg: $!" if $! != ECONNRESET;
"text" and "reply" outputs are intended for the pager, so parallelizing them is a waste of resources. v2 has shards, of course, so parallelizing writes to it is also a waste since the deduplication work is a bit more complex. --- lib/PublicInbox/LeiQuery.pm | 2 +- lib/PublicInbox/LeiToMail.pm | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/PublicInbox/LeiQuery.pm b/lib/PublicInbox/LeiQuery.pm index 3d0e5b144d3a..352ee60131aa 100644 --- a/lib/PublicInbox/LeiQuery.pm +++ b/lib/PublicInbox/LeiQuery.pm @@ -37,7 +37,7 @@ sub _start_query { # used by "lei q" and "lei up" $lms->lms_write_prepare->lms_pause; # just create } } - $l2m and $l2m->{-wq_nr_workers} = $mj // + $l2m and $l2m->{-wq_nr_workers} //= $mj // int($nproc * 0.75 + 0.5); # keep some CPU for git # descending docid order is cheapest, MUA controls sorting order diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm index 83f58a29405b..90db30e53089 100644 --- a/lib/PublicInbox/LeiToMail.pm +++ b/lib/PublicInbox/LeiToMail.pm @@ -415,11 +415,13 @@ sub new { require PublicInbox::LeiViewText; $lei->{lvt} = PublicInbox::LeiViewText->new($lei, $fmt); $self->{base_type} = 'text'; + $self->{-wq_nr_workers} = 1; # for pager @conflict = qw(mua save); } elsif ($fmt eq 'v2') { die "--dedupe=oid and v2 are incompatible\n" if ($lei->{opt}->{dedupe}//'') eq 'oid'; $self->{base_type} = 'v2'; + $self->{-wq_nr_workers} = 1; # v2 has shards $lei->{opt}->{save} = \1; $dst = $lei->{ovv}->{dst} = $lei->abs_path($dst); @conflict = qw(mua sort);
SIGPIPE and SIGTERM are common and user-induced, so they're not worth warning on. Add the value of "$?", though, since it can help users notice other errors (e.g. SIGSEGV). --- lib/PublicInbox/LeiXSearch.pm | 5 ++++- t/lei-sigpipe.t | 7 +++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm index 2a037f2bd79b..29df07e0c8a8 100644 --- a/lib/PublicInbox/LeiXSearch.pm +++ b/lib/PublicInbox/LeiXSearch.pm @@ -409,7 +409,10 @@ sub git { $_[0]->{git} // die 'BUG: git uninitialized' } sub xsearch_done_wait { # dwaitpid callback my ($arg, $pid) = @_; my ($wq, $lei) = @$arg; - $lei->child_error($?, 'non-fatal error from '.ref($wq)) if $?; + return if !$?; + my $s = $? & 127; + return $lei->child_error($?) if $s == 13 || $s == 15; + $lei->child_error($?, 'non-fatal error from '.ref($wq)." \$?=$?"); } sub query_done { # EOF callback for main daemon diff --git a/t/lei-sigpipe.t b/t/lei-sigpipe.t index f84d6d22ec4d..d9738b07bd8e 100644 --- a/t/lei-sigpipe.t +++ b/t/lei-sigpipe.t @@ -8,7 +8,7 @@ use POSIX qw(WTERMSIG WIFSIGNALED SIGPIPE); test_lei(sub { my $f = "$ENV{HOME}/big.eml"; my $imported; - for my $out ([], [qw(-f mboxcl2)]) { + for my $out ([], [qw(-f mboxcl2)], [qw(-f text)]) { pipe(my ($r, $w)) or BAIL_OUT $!; my $size = 65536; if ($^O eq 'linux' && fcntl($w, 1031, 4096)) { @@ -27,7 +27,7 @@ EOM } lei_ok(qw(import), $f) if $imported++ == 0; - open my $errfh, '>>', "$ENV{HOME}/stderr.log" or xbail $!; + open my $errfh, '+>', "$ENV{HOME}/stderr.log" or xbail $!; my $opt = { run_mode => 0, 2 => $errfh, 1 => $w }; my $cmd = [qw(lei q -q -t), @$out, 'z:1..']; my $tp = start_script($cmd, undef, $opt); @@ -37,6 +37,9 @@ EOM $tp->join; ok(WIFSIGNALED($?), "signaled @$out"); is(WTERMSIG($?), SIGPIPE, "got SIGPIPE @$out"); + seek($errfh, 0, 0) or xbail $!; + my $s = do { local $/; <$errfh> }; + is($s, '', "quiet after sigpipe @$out"); } });
->DESTROY ordering via "exit()" calls is tricky, and dedupe checks were causing problems. AFAIK, this only affects users who manually enable WAL on lei/store/ei*/over.sqlite3. Fortunately, there is no data corruption as a result even though "read-only" WAL requires write permissions. --- lib/PublicInbox/LeiToMail.pm | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm index 90db30e53089..3c5e7e59e8ee 100644 --- a/lib/PublicInbox/LeiToMail.pm +++ b/lib/PublicInbox/LeiToMail.pm @@ -756,6 +756,12 @@ sub ipc_atfork_child { $lei->_lei_atfork_child; $lei->{auth}->do_auth_atfork($self) if $lei->{auth}; $SIG{__WARN__} = PublicInbox::Eml::warn_ignore_cb(); + $self->{git} = $self->{lei}->{ale}->git; + $SIG{TERM} = sub { # avoid ->DESTROY ordering problems + my $git = delete $self->{git}; + $git->async_wait_all if $git; + exit(15 + 128); + }; $self->SUPER::ipc_atfork_child; } @@ -776,7 +782,7 @@ sub write_mail { # via ->wq_io_do my ($self, $smsg, $eml) = @_; return $self->{wcb}->(undef, $smsg, $eml) if $eml; $smsg->{-lms_rw} = $self->{-lms_rw}; - $self->{lei}->{ale}->git->cat_async($smsg->{blob}, \&git_to_mail, + $self->{git}->cat_async($smsg->{blob}, \&git_to_mail, [$self->{wcb}, $smsg]); }
Maybe we can avoid them if we stop having buggy code :P --- Documentation/lei-security.pod | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/lei-security.pod b/Documentation/lei-security.pod index 8cbd89934568..104bfb48a26c 100644 --- a/Documentation/lei-security.pod +++ b/Documentation/lei-security.pod @@ -64,6 +64,12 @@ public-facing L<public-inbox-daemon(8)> processes. They may reside on shared storage and may be made world-readable to other users on the local system. +=head1 CORE DUMPS + +In case any process crashes, a core dumps may contain passwords or +contents of sensitive messages. Please report these so they can be +fixed (see L</CONTACT>). + =head1 NETWORK ACCESS lei currently uses the L<curl(1)> and L<git(1)> executables in