[ https://issues.apache.org/jira/browse/SSHD-1295?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17603622#comment-17603622 ]
Thomas Wolf commented on SSHD-1295: ----------------------------------- I see a similar question regarding {{ConnectFuture.cancel()}} was already posted in SSHD-1280. For cancellation, I think the expectations for {{cancel()}} are quite obvious: * If a value is already set on the future (session or exception), {{cancel()}} does nothing. * Otherwise it fulfills the future with a "cancelled" marker. ** If there is some asynchronous operation going on for that future, that operation should be cancelled. ** If that operation cannot be cancelled: *** If it terminates successfully, behavior may be different depending on what kind of future it is: **** A {{ConnectFuture}} must close the connection/session, otherwise there is a stale connection the user cannot close using up one socket. **** An {{AuthFuture}} may result in the session being authenticated even if it was cancelled. *** If it terminates unsuccessfully, nothing needs to be done. For time-outs the question is more difficult to answer if several threads are allowed to call {{{}verify(){}}}. Here, SSHD-1278 is directly related. > Connection attempt not canceled when a connection timeout occurs > ---------------------------------------------------------------- > > Key: SSHD-1295 > URL: https://issues.apache.org/jira/browse/SSHD-1295 > Project: MINA SSHD > Issue Type: Bug > Affects Versions: 2.9.2 > Reporter: Thomas Wolf > Priority: Major > > This is not a new bug; it's also present in earlier versions. > When {{client.connect().verify(timeout)}} times out, the underlying > asynchronous connection attempt still continues _and may still succeed_. If > that happens, there is a connection and session that the user cannot use or > close. (Short of calling verify().getSession() again, but that is a pattern I > have never seen, and it also makes using a timeout rather useless.) > This means that timeouts are inherently unreliable. > Here's a simple test case illustrating this problem (for > {{org.apache.sshd.client.ClientTest}}): > {code:java} > @Test > public void testConnectTimeout() throws Exception { > client.start(); > try { > ConnectFuture future = client.connect(getCurrentTestName(), > TEST_LOCALHOST, port); > try { > future.verify(1); > fail("Timeout expected"); > } catch (InterruptedIOException | SshException e) { > ClientSession session = null; > try { > session = future.verify(CONNECT_TIMEOUT).getSession(); > } catch (SshException e2) { > assertTrue("Expected a timeout, got " + e2, > e2.getMessage().contains("timeout")); > } > if (session != null) { > assertTrue("Session should be closed", session.isClosed() || > session.isClosing()); > } > } > } finally { > client.stop(); > } > } > {code} > This test fails. > When the {{ConnectFuture}} times out (or is canceled, or waiting is > interrupted), the underlying {{IoConnectFuture}} must be canceled. If we do > get an {{IoSession}} all the same, but the {{ConnectFuture}} is already > canceled or timed out, the {{IoSession}} must be closed immediately. > Preferrably before a {{Session}} is created at all; otherwise, that SSH > session must be closed, too. Creating a {{ClientSession}} already starts the > application-level protocol (sending the SSH ident, or if a proxy is used, the > proxy connect request). > The test also highlights another problem: there is no clear indication that > the failure of {{verify()}} indicates a timeout. Probably there should be a > dedicated exception for this, or at least the {{SshException}} should have a > {{TimeoutException}} as cause. -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org