You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2018/02/28 17:44:20 UTC
[4/5] mina-sshd git commit: [SSHD-786] Avoid AcceptPendingException
on failed session attempt to re-accept incoming connections
[SSHD-786] Avoid AcceptPendingException on failed session attempt to re-accept incoming connections
Note: Also removed the ClientDeadlockTest since it was dependent on a specific timing race condition
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/deb2445e
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/deb2445e
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/deb2445e
Branch: refs/heads/master
Commit: deb2445ef9ec5bbfb8a3ed4e0693288b1a86f7f1
Parents: acc40c2
Author: Goldstein Lyor <ly...@c-b4.com>
Authored: Wed Feb 28 08:29:48 2018 +0200
Committer: Lyor Goldstein <ly...@gmail.com>
Committed: Wed Feb 28 19:47:04 2018 +0200
----------------------------------------------------------------------
.../sshd/common/io/nio2/Nio2Acceptor.java | 52 ++++++++++++--------
.../apache/sshd/client/ClientDeadlockTest.java | 37 ++++++++------
2 files changed, 54 insertions(+), 35 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/deb2445e/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Acceptor.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Acceptor.java b/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Acceptor.java
index 0427c80..92196ea 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Acceptor.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Acceptor.java
@@ -164,6 +164,7 @@ public class Nio2Acceptor extends Nio2Service implements IoAcceptor {
return getClass().getSimpleName() + "[" + getBoundAddresses() + "]";
}
+ @SuppressWarnings("synthetic-access")
protected class AcceptCompletionHandler extends Nio2CompletionHandler<AsynchronousSocketChannel, SocketAddress> {
protected final AsynchronousServerSocketChannel socket;
@@ -172,7 +173,6 @@ public class Nio2Acceptor extends Nio2Service implements IoAcceptor {
}
@Override
- @SuppressWarnings("synthetic-access")
protected void onCompleted(AsynchronousSocketChannel result, SocketAddress address) {
// Verify that the address has not been unbound
if (!channels.containsKey(address)) {
@@ -184,6 +184,7 @@ public class Nio2Acceptor extends Nio2Service implements IoAcceptor {
Nio2Session session = null;
Long sessionId = null;
+ boolean keepAccepting;
try {
// Create a session
IoHandler handler = getIoHandler();
@@ -201,8 +202,10 @@ public class Nio2Acceptor extends Nio2Service implements IoAcceptor {
} else {
session.startReading();
}
+
+ keepAccepting = true;
} catch (Throwable exc) {
- failed(exc, address);
+ keepAccepting = okToReaccept(exc, address);
// fail fast the accepted connection
if (session != null) {
@@ -219,15 +222,18 @@ public class Nio2Acceptor extends Nio2Service implements IoAcceptor {
unmapSession(sessionId);
}
- try {
- // Accept new connections
- socket.accept(address, this);
- } catch (Throwable exc) {
- failed(exc, address);
+ if (keepAccepting) {
+ try {
+ // Accept new connections
+ socket.accept(address, this);
+ } catch (Throwable exc) {
+ failed(exc, address);
+ }
+ } else {
+ log.error("=====> onCompleted({}) no longer accepting incoming connections <====", address);
}
}
- @SuppressWarnings("synthetic-access")
protected Nio2Session createSession(Nio2Acceptor acceptor, SocketAddress address, AsynchronousSocketChannel channel, IoHandler handler) throws Throwable {
if (log.isTraceEnabled()) {
log.trace("createNio2Session({}) address={}", acceptor, address);
@@ -236,15 +242,28 @@ public class Nio2Acceptor extends Nio2Service implements IoAcceptor {
}
@Override
- @SuppressWarnings("synthetic-access")
protected void onFailed(Throwable exc, SocketAddress address) {
+ if (okToReaccept(exc, address)) {
+ try {
+ // Accept new connections
+ socket.accept(address, this);
+ } catch (Throwable t) {
+ // Do not call failed(t, address) to avoid infinite recursion
+ log.error("Failed (" + t.getClass().getSimpleName()
+ + " to re-accept new connections on " + address
+ + ": " + t.getMessage(), t);
+ }
+ }
+ }
+
+ protected boolean okToReaccept(Throwable exc, SocketAddress address) {
AsynchronousServerSocketChannel channel = channels.get(address);
if (channel == null) {
if (log.isDebugEnabled()) {
log.debug("Caught {} for untracked channel of {}: {}",
exc.getClass().getSimpleName(), address, exc.getMessage());
}
- return;
+ return false;
}
if (disposing.get()) {
@@ -252,22 +271,13 @@ public class Nio2Acceptor extends Nio2Service implements IoAcceptor {
log.debug("Caught {} for tracked channel of {} while disposing: {}",
exc.getClass().getSimpleName(), address, exc.getMessage());
}
- return;
+ return false;
}
log.warn("Caught " + exc.getClass().getSimpleName()
+ " while accepting incoming connection from " + address
+ ": " + exc.getMessage(), exc);
-
- try {
- // Accept new connections
- socket.accept(address, this);
- } catch (Throwable t) {
- // Do not call failed(t, address) to avoid infinite recursion
- log.error("Failed (" + t.getClass().getSimpleName()
- + " to re-accept new connections on " + address
- + ": " + t.getMessage(), t);
- }
+ return true;
}
}
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/deb2445e/sshd-core/src/test/java/org/apache/sshd/client/ClientDeadlockTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/client/ClientDeadlockTest.java b/sshd-core/src/test/java/org/apache/sshd/client/ClientDeadlockTest.java
index 9c37ba4..23c60f7 100644
--- a/sshd-core/src/test/java/org/apache/sshd/client/ClientDeadlockTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/client/ClientDeadlockTest.java
@@ -19,27 +19,28 @@
package org.apache.sshd.client;
import java.io.IOException;
-import java.util.EnumSet;
import java.util.concurrent.TimeUnit;
import org.apache.sshd.client.future.ConnectFuture;
import org.apache.sshd.client.session.ClientSession;
+import org.apache.sshd.client.session.ClientSessionImpl;
+import org.apache.sshd.client.session.SessionFactory;
import org.apache.sshd.common.io.IoSession;
import org.apache.sshd.server.SshServer;
-import org.apache.sshd.server.session.ServerSessionImpl;
-import org.apache.sshd.server.session.SessionFactory;
import org.apache.sshd.util.test.BaseTestSupport;
import org.junit.After;
import org.junit.Before;
+import org.junit.FixMethodOrder;
import org.junit.Test;
+import org.junit.runners.MethodSorters;
/**
* TODO Add javadoc
*
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
public class ClientDeadlockTest extends BaseTestSupport {
-
private SshServer sshd;
private SshClient client;
private int port;
@@ -51,16 +52,16 @@ public class ClientDeadlockTest extends BaseTestSupport {
@Before
public void setUp() throws Exception {
sshd = setupTestServer();
- sshd.setSessionFactory(new SessionFactory(sshd) {
- @Override
- protected ServerSessionImpl doCreateSession(IoSession ioSession) throws Exception {
- throw new IOException("Closing");
- }
- });
sshd.start();
port = sshd.getPort();
client = setupTestClient();
+ client.setSessionFactory(new SessionFactory(client) {
+ @Override
+ protected ClientSessionImpl doCreateSession(IoSession ioSession) throws Exception {
+ throw new SimulatedException(getCurrentTestName());
+ }
+ });
}
@After
@@ -73,13 +74,21 @@ public class ClientDeadlockTest extends BaseTestSupport {
}
}
- @Test
+ @Test(expected = SimulatedException.class)
public void testSimpleClient() throws Exception {
client.start();
ConnectFuture future = client.connect(getCurrentTestName(), TEST_LOCALHOST, port);
- ClientSession session = future.verify(5L, TimeUnit.SECONDS).getSession();
- session.waitFor(EnumSet.of(ClientSession.ClientSessionEvent.CLOSED), TimeUnit.SECONDS.toMillis(7L));
- assertFalse(session.isOpen());
+ try (ClientSession session = future.verify(5L, TimeUnit.SECONDS).getSession()) {
+ fail("Unexpected session established: " + session);
+ }
+ }
+
+ static class SimulatedException extends IOException {
+ private static final long serialVersionUID = 2460966941758520525L;
+
+ SimulatedException(String message) {
+ super(message);
+ }
}
}