From f744e337c81887f95cc79e27f5cb15ff7e76c776 Mon Sep 17 00:00:00 2001
From: Eric Wong <e@80x24.org>
Date: Tue, 5 Apr 2016 23:55:12 +0000
Subject: [PATCH] repobrowse: reduce risk of callback reference cycles

It's unnecessary to rely so much on the req hashref
like this and having to manually drop references could
be considered code smell.
---
 lib/PublicInbox/RepobrowseGitCommit.pm | 17 ++++++++---------
 lib/PublicInbox/RepobrowseGitDiff.pm   | 17 ++++++++---------
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/lib/PublicInbox/RepobrowseGitCommit.pm b/lib/PublicInbox/RepobrowseGitCommit.pm
index 8fdbc47e9..227dad97b 100644
--- a/lib/PublicInbox/RepobrowseGitCommit.pm
+++ b/lib/PublicInbox/RepobrowseGitCommit.pm
@@ -111,13 +111,13 @@ sub git_diff_cc_line_i ($$) {
 	}
 }
 
-sub git_commit_stream ($$) {
-	my ($self, $req) = @_;
+sub git_commit_stream ($$$$) {
+	my ($self, $req, $fail, $end) = @_;
 	my $dbuf = \($req->{dbuf});
 	my $off = length($$dbuf);
 	my $n = $req->{rpipe}->sysread($$dbuf, 8192, $off);
-	return $req->{fail}->() unless defined $n;
-	return $req->{end}->() if $n == 0;
+	return $fail->() unless defined $n;
+	return $end->() if $n == 0;
 	my $res = $req->{res};
 	if ($res) {
 		return if index($$dbuf, "\0") < 0;
@@ -164,8 +164,7 @@ sub call_git_commit {
 	my $err = $env->{'psgi.errors'};
 	my $vin;
 	$req->{dbuf} = '';
-	$req->{end} = sub {
-		$req->{end} = undef;
+	my $end = sub {
 		if (my $fh = delete $req->{fh}) {
 			my $dbuf = delete $req->{dbuf};
 			if (!$req->{diff_state}) {
@@ -194,19 +193,19 @@ sub call_git_commit {
 		# $id to psgi.errors w/o sanitizing...
 		$git->err;
 	};
-	$req->{fail} = sub {
+	my $fail = sub {
 		if ($!{EAGAIN} || $!{EINTR}) {
 			select($vin, undef, undef, undef) if defined $vin;
 			# $vin is undef on async, so this is a noop on EAGAIN
 			return;
 		}
 		my $e = $!;
-		$req->{end}->();
+		$end->();
 		$err->print("git show ($git->{git_dir}): $e\n");
 	};
 	$req->{'q'} = $q;
 	my $cb = sub { # read git-show output and stream to client
-		git_commit_stream($self, $req);
+		git_commit_stream($self, $req, $fail, $end);
 	};
 	if (my $async = $env->{'pi-httpd.async'}) {
 		$req->{rpipe} = $async->($req->{rpipe}, $cb);
diff --git a/lib/PublicInbox/RepobrowseGitDiff.pm b/lib/PublicInbox/RepobrowseGitDiff.pm
index 8c349e9bf..fb44c7526 100644
--- a/lib/PublicInbox/RepobrowseGitDiff.pm
+++ b/lib/PublicInbox/RepobrowseGitDiff.pm
@@ -36,8 +36,7 @@ sub call_git_diff {
 	$req->{dbuf} = '';
 	$req->{p} = [ $id2 ];
 	$req->{h} = $id;
-	$req->{end} = sub {
-		$req->{fail} = $req->{cb} = $req->{end} = undef;
+	my $end = sub {
 		if (my $fh = delete $req->{fh}) {
 			# write out the last bit that was buffered
 			my @buf = split(/\n/, delete $req->{dbuf}, -1);
@@ -54,21 +53,21 @@ sub call_git_diff {
 			$rpipe->close; # _may_ be Danga::Socket::close
 		}
 	};
-	$req->{fail} = sub {
+	my $fail = sub {
 		if ($!{EAGAIN} || $!{EINTR}) {
 			select($vin, undef, undef, undef) if defined $vin;
 			# $vin is undef on async, so this is a noop on EAGAIN
 			return;
 		}
 		my $e = $!;
-		$req->{end}->();
+		$end->();
 		$err->print("git diff ($git->{git_dir}): $e\n");
 	};
-	$req->{cb} = sub {
+	my $cb = sub {
 		my $off = length($req->{dbuf});
 		my $n = $req->{rpipe}->sysread($req->{dbuf}, 8192, $off);
-		return $req->{fail}->() unless defined $n;
-		return $req->{end}->() if $n == 0;
+		return $fail->() unless defined $n;
+		return $end->() if $n == 0;
 		if (my $res = delete $req->{res}) {
 			my $h = ['Content-Type', 'text/html; charset=UTF-8'];
 			my $fh = $req->{fh} = $res->([200, $h]);
@@ -84,14 +83,14 @@ sub call_git_diff {
 		}
 	};
 	if (my $async = $env->{'pi-httpd.async'}) {
-		$req->{rpipe} = $async->($req->{rpipe}, $req->{cb});
+		$req->{rpipe} = $async->($req->{rpipe}, $cb);
 		sub { $req->{res} = $_[0] } # let Danga::Socket handle the rest.
 	} else { # synchronous loop for other PSGI servers
 		$vin = '';
 		vec($vin, fileno($req->{rpipe}), 1) = 1;
 		sub {
 			$req->{res} = $_[0];
-			while ($req->{rpipe}) { $req->{cb}->() }
+			while ($req->{rpipe}) { $cb->() }
 		}
 	}
 }
-- 
2.47.3