From 8a3fc4a2f027b36f27225ceee5908c571c8f4f47 Mon Sep 17 00:00:00 2001
From: Eric Wong <e@80x24.org>
Date: Sat, 4 Mar 2017 03:52:29 +0000
Subject: repoobrowse: explicit EOF handling for git async callback

We need to ensure we've fully-drained the pipe before
signalling EOF to the callback, since pipelining may
not be the best choice with detachable processes
in the future.
---
 lib/PublicInbox/Git.pm        |  5 +++--
 lib/PublicInbox/GitAsync.pm   |  3 ++-
 lib/PublicInbox/RepoGitRaw.pm | 20 +++++++++++---------
 lib/PublicInbox/RepoGitSrc.pm | 21 ++++++++++-----------
 t/git_async.t                 |  4 ++++
 5 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 6a7b109f..893df71e 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -253,10 +253,10 @@ sub cat_async_compat ($$$) {
 	$self->{out}->print($obj."\n") or fail($self, "write error: $!");
 	my $in = $self->{in};
 	my $info = async_info_compat($in);
+	my (undef, $type, $left) = @$info;
 	$cb->($info);
-	return if scalar(@$info) != 3; # missing
+	return if $info->[1] eq 'missing';
 	my $max = 8192;
-	my $left = $info->[2];
 	my ($buf, $r);
 	while ($left > 0) {
 		$r = read($in, $buf, $left > $max ? $max : $left);
@@ -267,6 +267,7 @@ sub cat_async_compat ($$$) {
 	$r = read($in, $buf, 1);
 	defined($r) or fail($self, "read failed: $!");
 	fail($self, 'newline missing after blob') if ($r != 1 || $buf ne "\n");
+	$cb->(0);
 }
 
 sub check_async {
diff --git a/lib/PublicInbox/GitAsync.pm b/lib/PublicInbox/GitAsync.pm
index 8369978c..24e0bf3b 100644
--- a/lib/PublicInbox/GitAsync.pm
+++ b/lib/PublicInbox/GitAsync.pm
@@ -65,7 +65,7 @@ take_job:
 		}
 		$cb->($info); # $info may 0 (EOF, or undef, $cb will see $!)
 		return $self->close unless $info;
-		if ($check || (scalar(@$info) != 3)) {
+		if ($check || $info->[1] eq 'missing') {
 			# do not monopolize the event loop if we're drained:
 			return if ${$self->{rbuf}} eq '';
 			goto take_job;
@@ -89,6 +89,7 @@ final_hunk:
 		my $lf = chop $$rbuf;
 		$lf eq "\n" or die "BUG: missing LF (got $lf)";
 		$cb->($rbuf);
+		$cb->(0);
 
 		return if $buf eq '';
 		goto take_job;
diff --git a/lib/PublicInbox/RepoGitRaw.pm b/lib/PublicInbox/RepoGitRaw.pm
index 858034a3..717ca71f 100644
--- a/lib/PublicInbox/RepoGitRaw.pm
+++ b/lib/PublicInbox/RepoGitRaw.pm
@@ -36,15 +36,17 @@ sub git_raw_check_res ($$$) {
 		my $buf = '';
 		$req->{-repo}->{git}->cat_async($req->{env}, $hex, sub {
 			my ($r) = @_;
-			return if ref($r) ne 'SCALAR';
-			$buf .= $$r;
-			return if bytes::length($buf) < $size;
-			$ct ||= index($buf, "\0") >= 0 ?
-					'application/octet-stream' :
-					'text/plain; charset=UTF-8';
-			$res->([200, ['Content-Type', $ct,
-					'Content-Length', $size ],
-				[ $buf ]]);
+			if (ref($r) eq 'SCALAR') {
+				$buf .= $$r;
+			} elsif ($r == 0) {
+				return if bytes::length($buf) < $size;
+				$ct ||= index($buf, "\0") >= 0 ?
+						'application/octet-stream' :
+						'text/plain; charset=UTF-8';
+				$res->([200, ['Content-Type', $ct,
+						'Content-Length', $size ],
+					[ $buf ]]);
+			}
 		});
 	}
 }
diff --git a/lib/PublicInbox/RepoGitSrc.pm b/lib/PublicInbox/RepoGitSrc.pm
index 67de86ee..51048773 100644
--- a/lib/PublicInbox/RepoGitSrc.pm
+++ b/lib/PublicInbox/RepoGitSrc.pm
@@ -154,18 +154,17 @@ sub git_blob_show {
 		return if $ref eq 'ARRAY'; # redundant info
 		if ($ref eq 'SCALAR') {
 			$buf .= $$r;
-			if (bytes::length($buf) == $size) {
-				my $fh = $res->($self->rt(200, 'html'));
-				my $sed = git_blob_sed($req, $hex, $size);
-				$fh->write($sed->($buf));
-				$fh->write($sed->(undef));
-				$fh->close;
-			}
-			return;
+		} elsif (!defined $r) {
+			my $cb = $res or return;
+			$res = undef;
+			$cb->($self->rt(500, 'plain', "Error\n"));
+		} elsif ($r == 0) {
+			my $fh = $res->($self->rt(200, 'html'));
+			my $sed = git_blob_sed($req, $hex, $size);
+			$fh->write($sed->($buf));
+			$fh->write($sed->(undef));
+			$fh->close;
 		}
-		my $cb = $res or return;
-		$res = undef;
-		$cb->($self->rt(500, 'plain', "Error\n"));
 	});
 }
 
diff --git a/t/git_async.t b/t/git_async.t
index 4f7e4ebe..ffe2b1a2 100644
--- a/t/git_async.t
+++ b/t/git_async.t
@@ -117,6 +117,7 @@ my $dir = "$tmpdir/git.git";
 	}
 	my @info;
 	my $str = '';
+	my $eof_seen = 0;
 	$git->cat_async_compat('HEAD:foo.txt', sub {
 		my $ref = $_[0];
 		my $t = ref $ref;
@@ -124,10 +125,13 @@ my $dir = "$tmpdir/git.git";
 			push @info, $ref;
 		} elsif ($t eq 'SCALAR') {
 			$str .= $$ref;
+		} elsif ($ref == 0) {
+			$eof_seen++;
 		} else {
 			fail "fail type: $t";
 		}
 	});
+	is($eof_seen, 1, 'EOF seen once');
 	is_deeply(\@info, [ [ 'bf4f17855632367a160bef055fc8ba4675d10e6b',
 			       'blob', 18 ]], 'info matches compat');
 	is($str, "-----\nhello\nworld\n", 'data matches compat');
-- 
cgit v1.2.3-24-ge0c7