Fix JSchProcess.waitFor() with time-out

SshSupport.runSshCommand() had a comment that wait with time-out
could not be used because JSchProcess.exitValue() threw the wrong
unchecked exception when the process was still running.

Fix this and make JSchProcess.exitValue() throw the right exception,
then wait with a time-out in SshSupport.

The Apache sshd client's SshdExecProcess has always used the correct
IllegalThreadStateException.

Add tests for SshSupport.runCommand().

Change-Id: Id30893174ae8be3b9a16119674049337b0cf4381
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

3 files changed, 52 insertions, 9 deletions

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 8cbcb0f67a..899a799989 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -621,6 +621,7 @@ sourceRefDoesntResolveToAnyObject=Source ref {0} doesn''t resolve to any object.
 sourceRefNotSpecifiedForRefspec=Source ref not specified for refspec: {0}
 squashCommitNotUpdatingHEAD=Squash commit -- not updating HEAD
 sshCommandFailed=Execution of ssh command ''{0}'' failed with error ''{1}''
+sshCommandTimeout=Execution of ssh command ''{0}'' timed out after {1} seconds
 sslFailureExceptionMessage=Secure connection to {0} could not be established because of SSL problems
 sslFailureInfo=A secure connection to {0} could not be established because the server''s certificate could not be validated.
 sslFailureCause=SSL reported: {0}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 782a3f872d..2976ab65fc 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -649,6 +649,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String sourceRefNotSpecifiedForRefspec;
 	/***/ public String squashCommitNotUpdatingHEAD;
 	/***/ public String sshCommandFailed;
+	/***/ public String sshCommandTimeout;
 	/***/ public String sslFailureExceptionMessage;
 	/***/ public String sslFailureInfo;
 	/***/ public String sslFailureCause;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java
index a151cd336f..e29704158d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java
@@ -11,6 +11,7 @@ package org.eclipse.jgit.util;
 
 import java.io.IOException;
 import java.text.MessageFormat;
+import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jgit.annotations.Nullable;
 import org.eclipse.jgit.errors.CommandFailedException;
@@ -61,11 +62,21 @@ public class SshSupport {
 		CommandFailedException failure = null;
 		@SuppressWarnings("resource")
 		MessageWriter stderr = new MessageWriter();
+		@SuppressWarnings("resource")
+		MessageWriter stdout = new MessageWriter();
 		String out;
-		try (MessageWriter stdout = new MessageWriter()) {
+		try {
+			long start = System.nanoTime();
 			session = SshSessionFactory.getInstance().getSession(sshUri,
 					provider, fs, 1000 * timeout);
-			process = session.exec(command, 0);
+			int commandTimeout = timeout;
+			if (timeout > 0) {
+				commandTimeout = checkTimeout(command, timeout, start);
+			}
+			process = session.exec(command, commandTimeout);
+			if (timeout > 0) {
+				commandTimeout = checkTimeout(command, timeout, start);
+			}
 			errorThread = new StreamCopyThread(process.getErrorStream(),
 					stderr.getRawStream());
 			errorThread.start();
@@ -73,9 +84,15 @@ public class SshSupport {
 					stdout.getRawStream());
 			outThread.start();
 			try {
-				// waitFor with timeout has a bug - JSch' exitValue() throws the
-				// wrong exception type :(
-				if (process.waitFor() == 0) {
+				boolean finished = false;
+				if (timeout <= 0) {
+					process.waitFor();
+					finished = true;
+				} else {
+					finished = process.waitFor(commandTimeout,
+							TimeUnit.SECONDS);
+				}
+				if (finished) {
 					out = stdout.toString();
 				} else {
 					out = null; // still running after timeout
@@ -103,15 +120,26 @@ public class SshSupport {
 				}
 			}
 			if (process != null) {
-				if (process.exitValue() != 0) {
-					failure = new CommandFailedException(process.exitValue(),
+				try {
+					if (process.exitValue() != 0) {
+						failure = new CommandFailedException(
+								process.exitValue(),
+								MessageFormat.format(
+										JGitText.get().sshCommandFailed,
+										command, stderr.toString()));
+					}
+					// It was successful after all
+					out = stdout.toString();
+				} catch (IllegalThreadStateException e) {
+					failure = new CommandFailedException(0,
 							MessageFormat.format(
-							JGitText.get().sshCommandFailed, command,
-							stderr.toString()));
+									JGitText.get().sshCommandTimeout, command,
+									Integer.valueOf(timeout)));
 				}
 				process.destroy();
 			}
 			stderr.close();
+			stdout.close();
 			if (session != null) {
 				SshSessionFactory.getInstance().releaseSession(session);
 			}
@@ -122,4 +150,17 @@ public class SshSupport {
 		return out;
 	}
 
+	private static int checkTimeout(String command, int timeout, long since)
+			throws CommandFailedException {
+		long elapsed = System.nanoTime() - since;
+		int newTimeout = timeout
+				- (int) TimeUnit.NANOSECONDS.toSeconds(elapsed);
+		if (newTimeout <= 0) {
+			// All time used up for connecting the session
+			throw new CommandFailedException(0,
+					MessageFormat.format(JGitText.get().sshCommandTimeout,
+							command, Integer.valueOf(timeout)));
+		}
+		return newTimeout;
+	}
 }