Update javadoc for RemoteSession and SshSessionFactory

The timeout on RemoteSession.exec() cannot be a timeout for the
whole command. It can only be a timeout for setting up the process;
after that it's the application's responsibility to implement some
timeout for the execution of the command, for instance by calling
Process.waitFor(int, TimeUnit) or through other means.

Sessions returned by an SshSessionFactory are already connected and
authenticated -- they must be, because RemoteSession offers no
operations for connecting or authenticating a session.

Change the implementation of SshdExecProcess.waitFor() to wait
indefinitely. The original implementation used the timeout from
RemoteSession.exec() because of that erroneous javadoc.

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

1 files changed, 16 insertions, 26 deletions

diff --git a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSession.java b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSession.java
index 0c1533c614..7ea7bd1c9a 100644
--- a/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSession.java
+++ b/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSession.java
@@ -13,7 +13,6 @@ import static java.text.MessageFormat.format;
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.InterruptedIOException;
 import java.io.OutputStream;
 import java.time.Duration;
 import java.util.ArrayList;
@@ -134,28 +133,23 @@ public class SshdSession implements RemoteSession {
 	public Process exec(String commandName, int timeout) throws IOException {
 		@SuppressWarnings("resource")
 		ChannelExec exec = session.createExecChannel(commandName);
-		long timeoutMillis = TimeUnit.SECONDS.toMillis(timeout);
-		try {
-			if (timeout <= 0) {
+		if (timeout <= 0) {
+			try {
 				exec.open().verify();
-			} else {
-				long start = System.nanoTime();
-				exec.open().verify(timeoutMillis);
-				timeoutMillis -= TimeUnit.NANOSECONDS
-						.toMillis(System.nanoTime() - start);
+			} catch (IOException | RuntimeException e) {
+				exec.close(true);
+				throw e;
+			}
+		} else {
+			try {
+				exec.open().verify(TimeUnit.SECONDS.toMillis(timeout));
+			} catch (IOException | RuntimeException e) {
+				exec.close(true);
+				throw new IOException(format(SshdText.get().sshCommandTimeout,
+						commandName, Integer.valueOf(timeout)), e);
 			}
-		} catch (IOException | RuntimeException e) {
-			exec.close(true);
-			throw e;
-		}
-		if (timeout > 0 && timeoutMillis <= 0) {
-			// We have used up the whole timeout for opening the channel
-			exec.close(true);
-			throw new InterruptedIOException(
-					format(SshdText.get().sshCommandTimeout, commandName,
-							Integer.valueOf(timeout)));
 		}
-		return new SshdExecProcess(exec, commandName, timeoutMillis);
+		return new SshdExecProcess(exec, commandName);
 	}
 
 	/**
@@ -195,14 +189,10 @@ public class SshdSession implements RemoteSession {
 
 		private final ChannelExec channel;
 
-		private final long timeoutMillis;
-
 		private final String commandName;
 
-		public SshdExecProcess(ChannelExec channel, String commandName,
-				long timeoutMillis) {
+		public SshdExecProcess(ChannelExec channel, String commandName) {
 			this.channel = channel;
-			this.timeoutMillis = timeoutMillis > 0 ? timeoutMillis : -1L;
 			this.commandName = commandName;
 		}
 
@@ -223,7 +213,7 @@ public class SshdSession implements RemoteSession {
 
 		@Override
 		public int waitFor() throws InterruptedException {
-			if (waitFor(timeoutMillis, TimeUnit.MILLISECONDS)) {
+			if (waitFor(-1L, TimeUnit.MILLISECONDS)) {
 				return exitValue();
 			}
 			return -1;