qspawn: shorten lifetime of environ and opts args

[public-inbox.git] / lib / PublicInbox / Qspawn.pm

diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm
index 294bf0a4dde2a8446a3c1e14cce849e0260bda51..54976b008d839b302a0d4c262157691f32818a07 100644 (file)
--- a/lib/PublicInbox/Qspawn.pm
+++ b/lib/PublicInbox/Qspawn.pm
@@ -27,7 +27,6 @@ package PublicInbox::Qspawn;
 use strict;
 use warnings;
 use PublicInbox::Spawn qw(popen_rd);
-use POSIX qw(WNOHANG);
 require Plack::Util;
 
 # n.b.: we get EAGAIN with public-inbox-httpd, and EINTR on other PSGI servers
@@ -57,6 +56,11 @@ sub _do_spawn {
        }
 
        ($self->{rpipe}, $self->{pid}) = popen_rd($cmd, $env, \%opts);
+
+       # drop any IO handles opt was holding open via $opt->{hold}
+       # No need to hold onto the descriptor once the child process has it.
+       $self->{args} = $cmd; # keep this around for logging
+
        if (defined $self->{pid}) {
                $limiter->{running}++;
        } else {
@@ -74,6 +78,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) = @_;
@@ -100,8 +109,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}}) . ": $err");
        }
 }
 
@@ -156,6 +164,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;
@@ -163,14 +173,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);
@@ -224,6 +237,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
        };
@@ -234,14 +249,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;
        };