tomaswolf commented on PR #242:
URL: https://github.com/apache/mina-sshd/pull/242#issuecomment-1240757939
As far as I see what happens is that the asynchronous connection attempt
succeeds after the timeout.
I'd move the timeout flag and the `isTimeout()` method to
`DefaultSshFuture`. I also think the flag must be set and read under lock,
otherwise there can still be a race condition. And instead of
```
if (connectFuture.isTimeout()) {
session.close();
} else {
connectFuture.setSession(session);
}
```
there needs to be a mechanism to do this atomically. Something like
```
/** Like setValue; but sets the value only if not timed out yet. Throws
TimeoutException otherwise. */
public void setValueIfNotTimedOut(Object value) throws TimeoutException {
synchronized(lock) {
if (result != null ) {
return;
}
if (isTimeout()) {
throw new TimeoutException();
}
result = (newValue != null) ? newValue : GenericUtils.NULL;
lock.notifyAll();
}
}
```
and then
```
private void setSessionSafely(ConnectFuture connectFuture, ClientSession
session) throws IOException {
try {
connectFuture.setValueIfNotTimedOut(session);
} catch (TimeoutException e) {
session.close();
}
}
```
Finally, it would be really cool if this could be done before the
`ClientSession` gets created at all (by closing the IoSession immediately).
Creating the client session will already start the SSH protocol. But the
`ClientSession` is created in
`AbstractSessionIoHandler.sessionCreated(IoSession)`, where we don't have
access to the ConnectFuture, so I don't quite see how this could be done.
--
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]
