From 947840027e9a110d13686d18240b64659988a326 Mon Sep 17 00:00:00 2001
From: Goldstein Lyor <lyor@c-b4.com>
Date: Thu, 7 Dec 2017 08:30:33 +0200
Subject: [PATCH 1/2] [SSHD-786] Mark Nio2Service correctly as disposing when
 dispose() called

---
 .../sshd/common/io/nio2/Nio2Acceptor.java     | 37 ++++++++++++++-----
 .../sshd/common/io/nio2/Nio2Service.java      |  8 +++-
 2 files changed, 33 insertions(+), 12 deletions(-)

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 66a3c414c..ce86aaf92 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
@@ -89,8 +89,12 @@ public void bind(SocketAddress address) throws IOException {
 
     @Override
     public void unbind() {
-        log.debug("Unbinding");
-        unbind(getBoundAddresses());
+        Collection<SocketAddress> addresses = getBoundAddresses();
+        if (log.isDebugEnabled()) {
+            log.debug("Unbinding {}", addresses);
+        }
+
+        unbind(addresses);
     }
 
     @Override
@@ -105,7 +109,7 @@ public void unbind(Collection<? extends SocketAddress> addresses) {
                     channel.close();
                 } catch (IOException e) {
                     log.warn("unbind({}) {} while unbinding channel: {}",
-                             address, e.getClass().getSimpleName(), e.getMessage());
+                         address, e.getClass().getSimpleName(), e.getMessage());
                     if (log.isDebugEnabled()) {
                         log.debug("unbind(" + address + ") failure details", e);
                     }
@@ -155,6 +159,11 @@ public void doCloseImmediately() {
         super.doCloseImmediately();
     }
 
+    @Override
+    public String toString() {
+        return getClass().getSimpleName() + "[" + getBoundAddresses() + "]";
+    }
+
     protected class AcceptCompletionHandler extends Nio2CompletionHandler<AsynchronousSocketChannel, SocketAddress> {
         protected final AsynchronousServerSocketChannel socket;
 
@@ -167,6 +176,9 @@ protected class AcceptCompletionHandler extends Nio2CompletionHandler<Asynchrono
         protected void onCompleted(AsynchronousSocketChannel result, SocketAddress address) {
             // Verify that the address has not been unbound
             if (!channels.containsKey(address)) {
+                if (log.isDebugEnabled()) {
+                    log.debug("onCompleted({}) unbound address", address);
+                }
                 return;
             }
 
@@ -187,7 +199,7 @@ protected void onCompleted(AsynchronousSocketChannel result, SocketAddress addre
                     try {
                         session.close();
                     } catch (Throwable t) {
-                        log.warn("Failed (" + t.getClass().getSimpleName() + ")"
+                        log.warn("onCompleted(" + address + ") Failed (" + t.getClass().getSimpleName() + ")"
                                + " to close accepted connection from " + address
                                + ": " + t.getMessage(),
                                  t);
@@ -223,13 +235,18 @@ protected void onFailed(Throwable exc, SocketAddress address) {
                 return;
             }
 
-            if (!disposing.get()) {
-                log.warn("Caught " + exc.getClass().getSimpleName()
-                       + " while accepting incoming connection from " + address
-                       + ": " + exc.getMessage(),
-                        exc);
-                // TODO (SSHD-786) consider closing the channel
+            if (disposing.get()) {
+                if (log.isDebugEnabled()) {
+                    log.debug("Caught {} for tracked channel of {} while disposing: {}",
+                        exc.getClass().getSimpleName(), address, exc.getMessage());
+                }
+                return;
             }
+
+            log.warn("Caught " + exc.getClass().getSimpleName()
+                   + " while accepting incoming connection from " + address
+                   + ": " + exc.getMessage(),
+                    exc);
         }
     }
 }
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Service.java b/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Service.java
index 763b54c90..15d25206a 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Service.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Service.java
@@ -93,6 +93,10 @@ public IoHandler getIoHandler() {
 
     public void dispose() {
         try {
+            if (disposing.getAndSet(true)) {
+                log.warn("dispose({}) already disposing", this);
+            }
+
             long maxWait = Closeable.getMaxCloseWaitTime(getFactoryManager());
             boolean successful = close(true).await(maxWait);
             if (!successful) {
@@ -100,11 +104,11 @@ public void dispose() {
             }
         } catch (IOException e) {
             if (log.isDebugEnabled()) {
-                log.debug(e.getClass().getSimpleName() + " while stopping service: " + e.getMessage());
+                log.debug("dispose({}) {} while stopping service: {}", this, e.getClass().getSimpleName(), e.getMessage());
             }
 
             if (log.isTraceEnabled()) {
-                log.trace("Stop exception details", e);
+                log.trace("dispose(" + this + ") Stop exception details", e);
             }
         }
     }

From 91074db17be498c0870affa4991f4a2a9e0f0fbb Mon Sep 17 00:00:00 2001
From: Goldstein Lyor <lyor@c-b4.com>
Date: Thu, 7 Dec 2017 09:02:35 +0200
Subject: [PATCH 2/2] [SSHD-786] Clients can't authenticate after unexpected
 exception in Nio2Acceptor

---
 .../apache/sshd/common/io/nio2/Nio2Acceptor.java    | 13 +++++++++++--
 .../apache/sshd/common/auth/AuthenticationTest.java | 12 ++++++++----
 2 files changed, 19 insertions(+), 6 deletions(-)

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 ce86aaf92..c047f2106 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
@@ -245,8 +245,17 @@ protected void onFailed(Throwable exc, SocketAddress address) {
 
             log.warn("Caught " + exc.getClass().getSimpleName()
                    + " while accepting incoming connection from " + address
-                   + ": " + exc.getMessage(),
-                    exc);
+                   + ": " + 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);
+            }
         }
     }
 }
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java b/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java
index 5e0333bcc..0f99de045 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/auth/AuthenticationTest.java
@@ -515,8 +515,8 @@ public String getUpdatedPassword(ClientSession session, String prompt, String la
     @Test   // see SSHD-600
     public void testAuthExceptionPropagation() throws Exception {
         try (SshClient client = setupTestClient()) {
-            final RuntimeException expected = new RuntimeException("Synthetic exception");
-            final AtomicInteger invocations = new AtomicInteger(0);
+            RuntimeException expected = new RuntimeException("Synthetic exception");
+            AtomicInteger invocations = new AtomicInteger(0);
             client.addSessionListener(new SessionListener() {
                 @Override
                 public void sessionEvent(Session session, Event event) {
@@ -533,11 +533,15 @@ public void sessionEvent(Session session, Event event) {
                 assertTrue("Failed to complete auth in allocated time", future.await(11L, TimeUnit.SECONDS));
                 assertFalse("Unexpected authentication success", future.isSuccess());
 
-                Throwable actual = future.getException();
+                Throwable signalled = future.getException();
+                Throwable actual = signalled;
                 if (actual instanceof IOException) {
                     actual = actual.getCause();
                 }
-                assertSame("Mismatched authentication failure reason", expected, actual);
+
+                if (expected != actual) {
+                    fail("Mismatched authentication failure reason: signalled=" + signalled + ", actual=" + actual);
+                }
             } finally {
                 client.stop();
             }