git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[JGIT PATCH 01/28] Fix deadlock caused by push over SSH

From
Shawn O. Pearce <spearce@spearce.org>
Date
Jul 18, 2008, 01:43 UTC
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
Previous: Shawn O. PearceNext: Shawn O. Pearce
Message 2 of 29 in “Convert command line parsing to args4j”
  1. 00/28 Convert command line parsing to args4jShawn O. Pearce, Jul 18, 2008
  2. 01/28 Fix deadlock caused by push over SSHShawn O. Pearce, Jul 18, 2008
  3. 02/28 Use die utility method in glogShawn O. Pearce, Jul 18, 2008
  4. 03/28 Add args4j library for command line switch processingShawn O. Pearce, Jul 18, 2008
  5. 04/28 Move org.spearce.jgit.pgm to its own Java projectShawn O. Pearce, Jul 18, 2008
  6. 05/28 Make TextBuiltin public so other packages can implement and use itShawn O. Pearce, Jul 18, 2008
  7. 06/28 Initialize TextBuiltins with the repository before executionShawn O. Pearce, Jul 18, 2008
  8. 07/28 Define our own extended CmdLineParser for extra parsing supportShawn O. Pearce, Jul 18, 2008
  9. 08/28 Add parseTree method to RevWalk to obtain a RevTree from AnyObjectIdShawn O. Pearce, Jul 18, 2008
  10. 09/28 Add option handler for AbstractTreeIterator valuesShawn O. Pearce, Jul 18, 2008
  11. 10/28 Add option handler for ObjectId valuesShawn O. Pearce, Jul 18, 2008
  12. 11/28 Add option handler for PathTreeFilter valuesShawn O. Pearce, Jul 18, 2008
  13. 12/28 Add option handler for RefSpec valuesShawn O. Pearce, Jul 18, 2008
  14. 13/28 Add option handler for RevCommit valuesShawn O. Pearce, Jul 18, 2008
  15. 14/28 Add option handler for RevTree valuesShawn O. Pearce, Jul 18, 2008
  16. 15/28 Register most of our OptionHandler implementations for automatic useShawn O. Pearce, Jul 18, 2008
  17. 16/28 Convert jgit's Main to use args4j for basic parsing servicesShawn O. Pearce, Jul 18, 2008
  18. 17/28 Support automatic command line parsing for TextBuiltin subclassesShawn O. Pearce, Jul 18, 2008
  19. 18/28 Convert diff-tree program to args4jShawn O. Pearce, Jul 18, 2008
  20. 19/28 Convert fetch program to args4jShawn O. Pearce, Jul 18, 2008
  21. 20/28 Convert index-pack program to args4jShawn O. Pearce, Jul 18, 2008
  22. 21/28 Convert ls-remote program to args4jShawn O. Pearce, Jul 18, 2008
  23. 22/28 Convert ls-tree program to args4jShawn O. Pearce, Jul 18, 2008
  24. 23/28 Convert merge-base program to args4jShawn O. Pearce, Jul 18, 2008
  25. 24/28 Convert push program to args4jShawn O. Pearce, Jul 18, 2008
  26. 25/28 Convert show-ref program to args4jShawn O. Pearce, Jul 18, 2008
  27. 26/28 Convert tag program to args4jShawn O. Pearce, Jul 18, 2008
  28. 27/28 Convert rev-list, log, glog programs to args4jShawn O. Pearce, Jul 18, 2008
  29. 28/28 Remove support for legacy style TextBuiltinsShawn O. Pearce, Jul 18, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.