git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Robin Rosenberg <robin.rosenberg@dewire.com>,
	Marek Zawirski <marek.zawirski@gmail.com>
Cc: git@vger.kernel.org
Subject: [JGIT PATCH 01/28] Fix deadlock caused by push over SSH
Date: Thu, 17 Jul 2008 21:43:54 -0400	[thread overview]
Message-ID: <1216345461-59382-2-git-send-email-spearce@spearce.org> (raw)
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

  reply	other threads:[~2008-07-18  1:45 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-18  1:43 [JGIT PATCH 00/28] Convert command line parsing to args4j Shawn O. Pearce
2008-07-18  1:43 ` Shawn O. Pearce [this message]
2008-07-18  1:43   ` [JGIT PATCH 02/28] Use die utility method in glog Shawn O. Pearce
2008-07-18  1:43     ` [JGIT PATCH 03/28] Add args4j library for command line switch processing Shawn O. Pearce
2008-07-18  1:43       ` [JGIT PATCH 04/28] Move org.spearce.jgit.pgm to its own Java project Shawn O. Pearce
2008-07-18  1:43         ` [JGIT PATCH 05/28] Make TextBuiltin public so other packages can implement and use it Shawn O. Pearce
2008-07-18  1:43           ` [JGIT PATCH 06/28] Initialize TextBuiltins with the repository before execution Shawn O. Pearce
2008-07-18  1:44             ` [JGIT PATCH 07/28] Define our own extended CmdLineParser for extra parsing support Shawn O. Pearce
2008-07-18  1:44               ` [JGIT PATCH 08/28] Add parseTree method to RevWalk to obtain a RevTree from AnyObjectId Shawn O. Pearce
2008-07-18  1:44                 ` [JGIT PATCH 09/28] Add option handler for AbstractTreeIterator values Shawn O. Pearce
2008-07-18  1:44                   ` [JGIT PATCH 10/28] Add option handler for ObjectId values Shawn O. Pearce
2008-07-18  1:44                     ` [JGIT PATCH 11/28] Add option handler for PathTreeFilter values Shawn O. Pearce
2008-07-18  1:44                       ` [JGIT PATCH 12/28] Add option handler for RefSpec values Shawn O. Pearce
2008-07-18  1:44                         ` [JGIT PATCH 13/28] Add option handler for RevCommit values Shawn O. Pearce
2008-07-18  1:44                           ` [JGIT PATCH 14/28] Add option handler for RevTree values Shawn O. Pearce
2008-07-18  1:44                             ` [JGIT PATCH 15/28] Register most of our OptionHandler implementations for automatic use Shawn O. Pearce
2008-07-18  1:44                               ` [JGIT PATCH 16/28] Convert jgit's Main to use args4j for basic parsing services Shawn O. Pearce
2008-07-18  1:44                                 ` [JGIT PATCH 17/28] Support automatic command line parsing for TextBuiltin subclasses Shawn O. Pearce
2008-07-18  1:44                                   ` [JGIT PATCH 18/28] Convert diff-tree program to args4j Shawn O. Pearce
2008-07-18  1:44                                     ` [JGIT PATCH 19/28] Convert fetch " Shawn O. Pearce
2008-07-18  1:44                                       ` [JGIT PATCH 20/28] Convert index-pack " Shawn O. Pearce
2008-07-18  1:44                                         ` [JGIT PATCH 21/28] Convert ls-remote " Shawn O. Pearce
2008-07-18  1:44                                           ` [JGIT PATCH 22/28] Convert ls-tree " Shawn O. Pearce
2008-07-18  1:44                                             ` [JGIT PATCH 23/28] Convert merge-base " Shawn O. Pearce
2008-07-18  1:44                                               ` [JGIT PATCH 24/28] Convert push " Shawn O. Pearce
2008-07-18  1:44                                                 ` [JGIT PATCH 25/28] Convert show-ref " Shawn O. Pearce
2008-07-18  1:44                                                   ` [JGIT PATCH 26/28] Convert tag " Shawn O. Pearce
2008-07-18  1:44                                                     ` [JGIT PATCH 27/28] Convert rev-list, log, glog programs " Shawn O. Pearce
2008-07-18  1:44                                                       ` [JGIT PATCH 28/28] Remove support for legacy style TextBuiltins Shawn O. Pearce

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1216345461-59382-2-git-send-email-spearce@spearce.org \
    --to=spearce@spearce.org \
    --cc=git@vger.kernel.org \
    --cc=marek.zawirski@gmail.com \
    --cc=robin.rosenberg@dewire.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).