From: Shawn O. Pearce <spearce@spearce.org>
Date: Fri, 18 Jul 2008 01:43:54 GMT
Subject: [JGIT PATCH 01/28] Fix deadlock caused by push over SSH
Message-ID: <1216345461-59382-2-git-send-email-spearce@spearce.org>
In-Reply-To: <1216345461-59382-1-git-send-email-spearce@spearce.org>

Push does not clean up the Transport correctly when it is done
so the JSch library still has user level threads running and we
do not close down the JVM.  We must close the transport.

This was broken by 2e05675201 ("Reuse the same SSH connection")
as the JSch session is held open inside of the Transport instance
in case there are additional calls made on the same Transport.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/pgm/Push.java             |    8 ++++----
 .../org/spearce/jgit/transport/FetchProcess.java   |    2 +-
 .../spearce/jgit/transport/OperationResult.java    |   16 +++++++++++++++-
 .../org/spearce/jgit/transport/PushProcess.java    |    2 +-
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/pgm/Push.java b/org.spearce.jgit/src/org/spearce/jgit/pgm/Push.java
index cbdf465..ef93f2f 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/pgm/Push.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/pgm/Push.java
@@ -52,8 +52,6 @@ class Push extends TextBuiltin {
 
 	private boolean verbose = false;
 
-	private Transport transport;
-
 	private boolean first = true;
 
 	@Override
@@ -97,7 +95,7 @@ class Push extends TextBuiltin {
 			repository = "origin";
 		else
 			repository = args[argi++];
-		transport = Transport.open(db, repository);
+		final Transport transport = Transport.open(db, repository);
 		if (thin != null)
 			transport.setPushThin(thin);
 		if (exec != null)
@@ -114,6 +112,8 @@ class Push extends TextBuiltin {
 
 		final PushResult result = transport.push(new TextProgressMonitor(),
 				toPush);
+		transport.close();
+
 		printPushResult(result);
 	}
 
@@ -149,7 +149,7 @@ class Push extends TextBuiltin {
 			final RemoteRefUpdate rru) {
 		if (first) {
 			first = false;
-			out.format("To %s\n", transport.getURI());
+			out.format("To %s\n", result.getURI());
 		}
 
 		final String remoteName = rru.getRemoteName();
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java
index f9c2266..e99869c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/FetchProcess.java
@@ -97,7 +97,7 @@ class FetchProcess {
 
 		conn = transport.openFetch();
 		try {
-			result.setAdvertisedRefs(conn.getRefsMap());
+			result.setAdvertisedRefs(transport.getURI(), conn.getRefsMap());
 			final Set<Ref> matched = new HashSet<Ref>();
 			for (final RefSpec spec : toFetch) {
 				if (spec.getSource() == null)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/OperationResult.java b/org.spearce.jgit/src/org/spearce/jgit/transport/OperationResult.java
index 6700e85..a2e502c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/OperationResult.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/OperationResult.java
@@ -55,9 +55,22 @@ public abstract class OperationResult {
 
 	protected Map<String, Ref> advertisedRefs = Collections.emptyMap();
 
+	protected URIish uri;
+
 	protected final SortedMap<String, TrackingRefUpdate> updates = new TreeMap<String, TrackingRefUpdate>();
 
 	/**
+	 * Get the URI this result came from.
+	 * <p>
+	 * Each transport instance connects to at most one URI at any point in time.
+	 * 
+	 * @return the URI describing the location of the remote repository.
+	 */
+	public URIish getURI() {
+		return uri;
+	}
+
+	/**
 	 * Get the complete list of refs advertised by the remote.
 	 * <p>
 	 * The returned refs may appear in any order. If the caller needs these to
@@ -109,7 +122,8 @@ public abstract class OperationResult {
 		return updates.get(localName);
 	}
 
-	protected void setAdvertisedRefs(final Map<String, Ref> ar) {
+	protected void setAdvertisedRefs(final URIish u, final Map<String, Ref> ar) {
+		uri = u;
 		advertisedRefs = ar;
 	}
 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/PushProcess.java b/org.spearce.jgit/src/org/spearce/jgit/transport/PushProcess.java
index 9e63f2f..6a2176f 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/PushProcess.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/PushProcess.java
@@ -211,7 +211,7 @@ class PushProcess {
 
 	private PushResult prepareOperationResult() {
 		final PushResult result = new PushResult();
-		result.setAdvertisedRefs(connection.getRefsMap());
+		result.setAdvertisedRefs(transport.getURI(), connection.getRefsMap());
 		result.setRemoteUpdates(toPush);
 
 		for (final RemoteRefUpdate rru : toPush.values()) {
-- 
1.5.6.3.569.ga9185