[ https://issues.apache.org/jira/browse/SSHD-1295?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17622890#comment-17622890 ]
dgü edited comment on SSHD-1295 at 10/23/22 11:00 PM: ------------------------------------------------------ Hello! I am not sure if canceling an asynchronous operation unnecessarily is convenient to the asynchronous nature. Asynchronous operation may still be needed; it doesn't have to be canceled. Here is a code: {code:java} do { ConnectFuture connectFuture = client.connect(uriString); try { connectFuture.verify(CONNECT_TIMEOUT); } catch (IOException e) { //If timeout occurs, do something doSomething(); } } while (...); {code} A better way may be adding a synchronous connection feature. I am new to SSHD. This is just an opinion. was (Author: JIRAUSER284065): Hello! I am not sure if canceling an asynchronous operation unnecessarily is convenient to the asynchronous nature. Asynchronous operation may still be needed; it doesn't have to be canceled. Here is a code: {code:java} int i=0; do { ConnectFuture connectFuture = client.connect(uriString); try { connectFuture.verify(CONNECT_TIMEOUT); } catch (IOException e) { //If timeout occurs, do something doSomething(); i++; } } while (i<10); {code} A better way may be adding a synchronous connection feature. I am new to SSHD. This is just an opinion. > 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