From 620b5e23dded54e0abf954752767bc9683a882e3 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Thu, 4 Jul 2019 10:02:06 +0000 Subject: [PATCH] qspawn: retry sysread when parsing headers, too We need to ensure the BIN_DETECT (8000 byte) check in ViewVCS can be handled properly when sending giant files. Otherwise, EPOLLET won't notify us, again, and responses can get stuck. While we're at it, bump up the read-size up to 4096 bytes so we make fewer trips to the kernel. --- lib/PublicInbox/Qspawn.pm | 16 +++++++++++++--- t/httpd-corner.psgi | 13 +++++++++++++ t/httpd-corner.t | 16 ++++++++++++++-- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm index 8f0b9fe24..fb48585c2 100644 --- a/lib/PublicInbox/Qspawn.pm +++ b/lib/PublicInbox/Qspawn.pm @@ -195,9 +195,19 @@ sub psgi_return { my $buf = ''; my $rd_hdr = sub { - my $r = sysread($rpipe, $buf, 1024, length($buf)); - return if !defined($r) && $! == EAGAIN || $! == EINTR; - $parse_hdr->($r, \$buf); + # 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; + 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); + } until (defined $ret); + $ret; }; my $wcb = delete $env->{'qspawn.wcb'}; diff --git a/t/httpd-corner.psgi b/t/httpd-corner.psgi index f8396907f..9728aa05a 100644 --- a/t/httpd-corner.psgi +++ b/t/httpd-corner.psgi @@ -72,6 +72,19 @@ my $app = sub { getline => sub { undef }, close => sub { die 'CLOSE FAIL' }, ); + } elsif ($path eq '/async-big') { + require PublicInbox::Qspawn; + open my $null, '>', '/dev/null' or die; + my $rdr = { 2 => fileno($null) }; + my $cmd = [qw(dd if=/dev/zero count=30 bs=1024k)]; + my $qsp = PublicInbox::Qspawn->new($cmd, undef, $rdr); + return $qsp->psgi_return($env, undef, sub { + my ($r, $bref) = @_; + # make $rd_hdr retry sysread + $parse_hdr in Qspawn: + return until length($$bref) > 8000; + close $null; + [ 200, [ qw(Content-Type application/octet-stream) ]]; + }); } [ $code, $h, $body ] diff --git a/t/httpd-corner.t b/t/httpd-corner.t index 5efb9d14e..35318b501 100644 --- a/t/httpd-corner.t +++ b/t/httpd-corner.t @@ -251,9 +251,10 @@ SKIP: { $have_curl = 1; last; } - my $ntest = 2; + my $ntest = 4; $have_curl or skip('curl(1) missing', $ntest); - my $url = 'http://' . $sock->sockhost . ':' . $sock->sockport . '/sha1'; + my $base = 'http://' . $sock->sockhost . ':' . $sock->sockport; + my $url = "$base/sha1"; my ($r, $w); pipe($r, $w) or die "pipe: $!"; my $cmd = [qw(curl --tcp-nodelay --no-buffer -T- -HExpect: -sS), $url]; @@ -270,6 +271,17 @@ SKIP: { is($?, 0, 'curl exited successfully'); is($err, '', 'no errors from curl'); is($out, sha1_hex($str), 'read expected body'); + + open my $fh, '-|', qw(curl -sS), "$base/async-big" or die $!; + my $n = 0; + my $non_zero = 0; + while (1) { + my $r = sysread($fh, my $buf, 4096) or last; + $n += $r; + $buf =~ /\A\0+\z/ or $non_zero++; + } + is($n, 30 * 1024 * 1024, 'got expected output from curl'); + is($non_zero, 0, 'read all zeros'); } { -- 2.47.3