[JGIT PATCH 1/4] Avoid deadlock while fetching from local repository

From: Shawn O. Pearce <spearce@spearce.org>
Date: Jul 9, 2008, 04:15 UTC
Message-ID: <1215576931-4174-2-git-send-email-spearce@spearce.org>
In-Reply-To: <1215576931-4174-1-git-send-email-spearce@spearce.org>

We cannot send a packet line end command to git-upload-pack after we have sent our want list and obtained a pack back from it. Once the want list has ended git-upload-pack wants no further data sent to it, and attempting to write more may cause us to deadlock as the pipe won't accept the data.

Not sending the packet line end if we don't send a want list is a (minor) protocol error. To avoid these protocol errors (which may display on stderr from git-upload-pack) we still send the end during close if we have not yet sent a want list.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../spearce/jgit/transport/BasePackConnection.java |    7 ++++++-
 .../jgit/transport/BasePackFetchConnection.java    |    1 +
 .../jgit/transport/BasePackPushConnection.java     |    1 +
 3 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java
index a878f01..7dc4620 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java
@@ -83,6 +83,9 @@ abstract class BasePackConnection extends BaseConnection {
 	/** Packet line encoder around {@link #out}. */
 	protected PacketLineOut pckOut;
 
+	/** Send {@link PacketLineOut#end()} before closing {@link #out}? */
+	protected boolean outNeedsEnd;
+
 	/** Capability tokens advertised by the remote side. */
 	private final Set<String> remoteCapablities = new HashSet<String>();
 
@@ -99,6 +102,7 @@ abstract class BasePackConnection extends BaseConnection {
 
 		pckIn = new PacketLineIn(in);
 		pckOut = new PacketLineOut(out);
+		outNeedsEnd = true;
 	}
 
 	protected void readAdvertisedRefs() throws TransportException {
@@ -195,7 +199,8 @@ abstract class BasePackConnection extends BaseConnection {
 	public void close() {
 		if (out != null) {
 			try {
-				pckOut.end();
+				if (outNeedsEnd)
+					pckOut.end();
 				out.close();
 			} catch (IOException err) {
 				// Ignore any close errors.
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
index 04a91bf..12b36f2 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
@@ -253,6 +253,7 @@ abstract class BasePackFetchConnection extends BasePackConnection implements
 			pckOut.writeString(line.toString());
 		}
 		pckOut.end();
+		outNeedsEnd = false;
 		return !first;
 	}
 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java
index 784a578..6d95eaf 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java
@@ -148,6 +148,7 @@ class BasePackPushConnection extends BasePackConnection implements
 		if (monitor.isCancelled())
 			throw new TransportException(uri, "push cancelled");
 		pckOut.end();
+		outNeedsEnd = false;
 	}
 
 	private String enableCapabilties() {

-- 
1.5.6.74.g8a5e