unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [PATCH 0/5] lei: fix various SIGPIPE problems
@ 2021-10-30  8:11 Eric Wong
  2021-10-30  8:11 ` [PATCH 1/5] lei: do not access {sock} after SIGPIPE Eric Wong
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Eric Wong @ 2021-10-30  8:11 UTC (permalink / raw)
  To: meta

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(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/5] lei: do not access {sock} after SIGPIPE
  2021-10-30  8:11 [PATCH 0/5] lei: fix various SIGPIPE problems Eric Wong
@ 2021-10-30  8:11 ` Eric Wong
  2021-10-30  8:11 ` [PATCH 2/5] lei_to_mail: limit workers for text, reply and v2 outputs Eric Wong
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-10-30  8:11 UTC (permalink / raw)
  To: meta

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;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/5] lei_to_mail: limit workers for text, reply and v2 outputs
  2021-10-30  8:11 [PATCH 0/5] lei: fix various SIGPIPE problems Eric Wong
  2021-10-30  8:11 ` [PATCH 1/5] lei: do not access {sock} after SIGPIPE Eric Wong
@ 2021-10-30  8:11 ` Eric Wong
  2021-10-30  8:11 ` [PATCH 3/5] lei_xsearch: quiet error message on SIG{PIPE,TERM} Eric Wong
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-10-30  8:11 UTC (permalink / raw)
  To: meta

"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);

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/5] lei_xsearch: quiet error message on SIG{PIPE,TERM}
  2021-10-30  8:11 [PATCH 0/5] lei: fix various SIGPIPE problems Eric Wong
  2021-10-30  8:11 ` [PATCH 1/5] lei: do not access {sock} after SIGPIPE Eric Wong
  2021-10-30  8:11 ` [PATCH 2/5] lei_to_mail: limit workers for text, reply and v2 outputs Eric Wong
@ 2021-10-30  8:11 ` Eric Wong
  2021-10-30  8:11 ` [PATCH 4/5] lei_to_mail: avoid SEGV on worker exit via SIGTERM Eric Wong
  2021-10-30  8:11 ` [PATCH 5/5] doc: lei-security: add a note about core dumps Eric Wong
  4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-10-30  8:11 UTC (permalink / raw)
  To: meta

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");
 	}
 });
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/5] lei_to_mail: avoid SEGV on worker exit via SIGTERM
  2021-10-30  8:11 [PATCH 0/5] lei: fix various SIGPIPE problems Eric Wong
                   ` (2 preceding siblings ...)
  2021-10-30  8:11 ` [PATCH 3/5] lei_xsearch: quiet error message on SIG{PIPE,TERM} Eric Wong
@ 2021-10-30  8:11 ` Eric Wong
  2021-10-30  8:11 ` [PATCH 5/5] doc: lei-security: add a note about core dumps Eric Wong
  4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-10-30  8:11 UTC (permalink / raw)
  To: meta

->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]);
 }
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 5/5] doc: lei-security: add a note about core dumps
  2021-10-30  8:11 [PATCH 0/5] lei: fix various SIGPIPE problems Eric Wong
                   ` (3 preceding siblings ...)
  2021-10-30  8:11 ` [PATCH 4/5] lei_to_mail: avoid SEGV on worker exit via SIGTERM Eric Wong
@ 2021-10-30  8:11 ` Eric Wong
  4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-10-30  8:11 UTC (permalink / raw)
  To: meta

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-10-30  8:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-30  8:11 [PATCH 0/5] lei: fix various SIGPIPE problems Eric Wong
2021-10-30  8:11 ` [PATCH 1/5] lei: do not access {sock} after SIGPIPE Eric Wong
2021-10-30  8:11 ` [PATCH 2/5] lei_to_mail: limit workers for text, reply and v2 outputs Eric Wong
2021-10-30  8:11 ` [PATCH 3/5] lei_xsearch: quiet error message on SIG{PIPE,TERM} Eric Wong
2021-10-30  8:11 ` [PATCH 4/5] lei_to_mail: avoid SEGV on worker exit via SIGTERM Eric Wong
2021-10-30  8:11 ` [PATCH 5/5] doc: lei-security: add a note about core dumps Eric Wong

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).