user/dev discussion of public-inbox itself
 help / color / Atom feed
* [PATCH] qspawn: clarify and improve error handling
@ 2019-09-15  1:00 Eric Wong
  0 siblings, 0 replies; only message in thread
From: Eric Wong @ 2019-09-15  1:00 UTC (permalink / raw)
  To: meta

EINTR should not happen when using non-blocking sockets like we
do in our daemons, but maybe some OSes allow it to happen and
edge-triggered notifications won't notify us again.

So always retry immediately on EINTR without relying on kqueue
or epoll to notify us, and log any other unrecoverable errors
which may happen while we're at it.
---
 lib/PublicInbox/Qspawn.pm | 38 +++++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm
index 73fd3225..4b9bca5a 100644
--- a/lib/PublicInbox/Qspawn.pm
+++ b/lib/PublicInbox/Qspawn.pm
@@ -73,6 +73,11 @@ sub child_err ($) {
 	$msg;
 }
 
+sub log_err ($$) {
+	my ($env, $msg) = @_;
+	$env->{'psgi.errors'}->print($msg, "\n");
+}
+
 # callback for dwaitpid
 sub waitpid_err ($$) {
 	my ($self, $pid) = @_;
@@ -99,8 +104,7 @@ sub waitpid_err ($$) {
 	$self->{err} = $err;
 	my $env = $self->{env} or return;
 	if (!$env->{'qspawn.quiet'}) {
-		$err = join(' ', @{$self->{args}->[0]}).": $err\n";
-		$env->{'psgi.errors'}->print($err);
+		log_err($env, join(' ', @{$self->{args}->[0]}) . ": $err");
 	}
 }
 
@@ -155,6 +159,8 @@ sub psgi_qx {
 	my $scalar = '';
 	open(my $qx, '+>', \$scalar) or die; # PerlIO::scalar
 	my $end = sub {
+		my $err = $_[0]; # $!
+		log_err($env, "psgi_qx: $err") if defined($err);
 		finish($self, $env);
 		eval { $qx_cb->(\$scalar) };
 		$qx = $scalar = undef;
@@ -162,14 +168,17 @@ sub psgi_qx {
 	my $rpipe; # comes from popen_rd
 	my $async = $env->{'pi-httpd.async'};
 	my $cb = sub {
-		my $r = sysread($rpipe, my $buf, 65536);
+		my ($r, $buf);
+reread:
+		$r = sysread($rpipe, $buf, 65536);
 		if ($async) {
 			$async->async_pass($env->{'psgix.io'}, $qx, \$buf);
 		} elsif (defined $r) {
 			$r ? $qx->write($buf) : $end->();
 		} else {
-			return if $! == EAGAIN || $! == EINTR; # loop again
-			$end->();
+			return if $! == EAGAIN; # try again when notified
+			goto reread if $! == EINTR;
+			$end->($!);
 		}
 	};
 	$limiter ||= $def_limiter ||= PublicInbox::Qspawn::Limiter->new(32);
@@ -223,6 +232,8 @@ sub psgi_return {
 	my ($self, $env, $limiter, $parse_hdr) = @_;
 	my ($fh, $rpipe);
 	my $end = sub {
+		my $err = $_[0]; # $!
+		log_err($env, "psgi_return: $err") if defined($err);
 		finish($self, $env);
 		$fh->close if $fh; # async-only
 	};
@@ -233,14 +244,19 @@ sub psgi_return {
 		# we must loop until EAGAIN for EPOLLET in HTTPD/Async.pm
 		# We also need to check EINTR for generic PSGI servers.
 		my $ret;
-		my $n = 0;
+		my $total_rd = 0;
 		do {
 			my $r = sysread($rpipe, $buf, 4096, length($buf));
-			return if !defined($r) && $! == EAGAIN || $! == EINTR;
-
-			# $r may be undef, here:
-			$n += $r if $r;
-			$ret = $parse_hdr->($r ? $n : $r, \$buf);
+			if (defined($r)) {
+				$total_rd += $r;
+				$ret = $parse_hdr->($r ? $total_rd : 0, \$buf);
+			} else {
+				# caller should notify us when it's ready:
+				return if $! == EAGAIN;
+				next if $! == EINTR; # immediate retry
+				log_err($env, "error reading header: $!");
+				$ret = [ 500, [], [ "Internal error\n" ] ];
+			}
 		} until (defined $ret);
 		$ret;
 	};
-- 
EW


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, back to index

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-15  1:00 [PATCH] qspawn: clarify and improve error handling Eric Wong

user/dev discussion of public-inbox itself

Archives are clonable:
	git clone --mirror https://public-inbox.org/meta
	git clone --mirror http://czquwvybam4bgbro.onion/meta
	git clone --mirror http://hjrcffqmbrq6wope.onion/meta
	git clone --mirror http://ou63pmih66umazou.onion/meta

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.mail.public-inbox.meta
	nntp://ou63pmih66umazou.onion/inbox.comp.mail.public-inbox.meta
	nntp://czquwvybam4bgbro.onion/inbox.comp.mail.public-inbox.meta
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.mail.public-inbox.meta
	nntp://news.gmane.org/gmane.mail.public-inbox.general

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox