tomaswolf commented on code in PR #242:
URL: https://github.com/apache/mina-sshd/pull/242#discussion_r966772266
##########
sshd-core/src/main/java/org/apache/sshd/client/future/ConnectFuture.java:
##########
@@ -78,4 +80,13 @@ default ClientSession getClientSession() {
* Cancels the connection attempt and notifies all threads waiting for
this future.
*/
void cancel();
+
+
+ /**
+ * Sets the newly connected session only if not timeout and notifies all
threads waiting for this future. This method is invoked by SSHD
+ * internally. Please do not call this method directly.
+ *
+ * @param session The {@link ClientSession}
Review Comment:
Missing `@throws` javadoc
##########
sshd-common/src/main/java/org/apache/sshd/common/future/DefaultSshFuture.java:
##########
@@ -79,6 +82,9 @@ protected Object await0(long timeoutMillis, boolean
interruptable) throws Interr
}
curTime = System.currentTimeMillis();
+ if (curTime >= endTime) {
+ setTimeout();
+ }
Review Comment:
I think we also need to call setTimeout() above, in the exception handler,
inside `if (interruptable) {`.
##########
sshd-common/src/main/java/org/apache/sshd/common/future/DefaultSshFuture.java:
##########
@@ -227,6 +251,13 @@ public void cancel() {
setValue(CANCELED);
}
+ private boolean isTimeout() {
+ return timeout;
+ }
Review Comment:
Missing empty line between methods. Also make `isTimeout()` public; it may
be useful in other contexts. And it must access the flag under lock.
##########
sshd-common/src/main/java/org/apache/sshd/common/future/DefaultSshFuture.java:
##########
@@ -50,6 +52,7 @@ public DefaultSshFuture(Object id, Object lock) {
super(id);
this.lock = (lock != null) ? lock : this;
+ this.timeout = false;
Review Comment:
Unnecessary; it's the default value anyway.
##########
sshd-common/src/main/java/org/apache/sshd/common/future/DefaultSshFuture.java:
##########
@@ -112,6 +118,24 @@ public void setValue(Object newValue) {
notifyListeners();
}
+ /**
+ * Like setValue; but sets the value only if not timed out yet. Throws
TimeoutException otherwise.
+ *
+ * @param newValue The operation result
Review Comment:
Missing `@throws` javadoc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
