[SSHD-340] Mark SSH client/server as having 'Opened' state if re-star…

…ted after being stopped
apache · Jul 25, 2018 · 738264f · 738264f
1 parent bd8fe7e
commit 738264f
Show file tree

Hide file tree

Showing 6 changed files with 122 additions and 8 deletions.
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java b/sshd-core/src/main/java/org/apache/sshd/client/SshClient.java
@@ -72,6 +72,7 @@
 import org.apache.sshd.common.helpers.AbstractFactoryManager;
 import org.apache.sshd.common.io.IoConnectFuture;
 import org.apache.sshd.common.io.IoConnector;
+import org.apache.sshd.common.io.IoServiceFactory;
 import org.apache.sshd.common.io.IoSession;
 import org.apache.sshd.common.keyprovider.KeyPairProvider;
 import org.apache.sshd.common.session.helpers.AbstractSession;
@@ -371,6 +372,9 @@ public boolean isStarted() {
      */
     public void start() {
         if (isStarted()) {
+            if (log.isDebugEnabled()) {
+                log.debug("start({}) already started", this);
+            }
             return;
         }
 
@@ -380,6 +384,7 @@ public void start() {
         }
 
         setupSessionTimeout(sessionFactory);
+        reopenIfNotOpen();
 
         connector = createConnector();
         started.set(true);
@@ -687,7 +692,8 @@ protected void setupDefaultSessionIdentities(ClientSession session) {
     }
 
     protected IoConnector createConnector() {
-        return getIoServiceFactory().createConnector(getSessionFactory());
+        IoServiceFactory factory = getIoServiceFactory();
+        return factory.createConnector(getSessionFactory());
     }
 
     protected SessionFactory createSessionFactory() {

diff --git a/sshd-core/src/main/java/org/apache/sshd/common/io/IoAcceptor.java b/sshd-core/src/main/java/org/apache/sshd/common/io/IoAcceptor.java
@@ -19,10 +19,15 @@
 package org.apache.sshd.common.io;
 
 import java.io.IOException;
+import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
 import java.util.Set;
 
+import org.apache.sshd.common.util.GenericUtils;
+
 /**
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
@@ -41,4 +46,31 @@ public interface IoAcceptor extends IoService {
 
     Set<SocketAddress> getBoundAddresses();
 
+    /**
+     * @param acceptor The {@link IoAcceptor} - ignored if {@code null}
+     * @return The port associated with the <u>first</u> bound address - {@code -1} if none available
+     * @see #resolveBoundAddress(IoAcceptor)
+     */
+    static int resolveBoundPort(IoAcceptor acceptor) {
+        SocketAddress boundEndpoint = resolveBoundAddress(acceptor);
+        if (boundEndpoint instanceof InetSocketAddress) {
+            return ((InetSocketAddress) boundEndpoint).getPort();
+        }
+
+        return -1;
+    }
+
+    /**
+     * @param acceptor The {@link IoAcceptor} - ignored if {@code null}
+     * @return The <u>first</u> bound address - {@code null} if none available
+     * @see #getBoundAddresses()
+     */
+    static SocketAddress resolveBoundAddress(IoAcceptor acceptor) {
+        Collection<SocketAddress> boundAddresses = (acceptor == null) ? Collections.emptySet() : acceptor.getBoundAddresses();
+        if (GenericUtils.isEmpty(boundAddresses)) {
+            return null;
+        }
+        Iterator<SocketAddress> boundIterator = boundAddresses.iterator();
+        return boundIterator.next();
+    }
 }
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/util/closeable/AbstractCloseable.java b/sshd-core/src/main/java/org/apache/sshd/common/util/closeable/AbstractCloseable.java
@@ -144,6 +144,23 @@ protected CloseFuture doCloseGracefully() {
         return null;
     }
 
+    /**
+     * Checks if the current state is {@link State#Opened} and sets it
+     * as such if not.
+     *
+     * @return {@code true} if had to set the state to {@link State#Opened}
+     */
+    protected boolean reopenIfNotOpen() {
+        State curState = state.get();
+        if (curState == State.Opened) {
+            return false;
+        }
+
+        log.info("reopenIfNotOpen({}) update state {} -> {}", this, curState, State.Opened);
+        state.set(State.Opened);
+        return true;
+    }
+
     /**
      * <P>doCloseImmediately is called once and only once
      * with state == Immediate</P>

diff --git a/sshd-core/src/main/java/org/apache/sshd/server/SshServer.java b/sshd-core/src/main/java/org/apache/sshd/server/SshServer.java
@@ -278,7 +278,11 @@ public boolean isStarted() {
      * @throws IOException If failed to start
      */
     public void start() throws IOException {
+        boolean debugEnabled = log.isDebugEnabled();
         if (isStarted()) {
+            if (debugEnabled) {
+                log.debug("start({}) already started", this);
+            }
             return;
         }
 
@@ -289,33 +293,37 @@ public void start() throws IOException {
         acceptor = createAcceptor();
 
         setupSessionTimeout(sessionFactory);
+        reopenIfNotOpen();
 
         String hostsList = getHost();
         if (!GenericUtils.isEmpty(hostsList)) {
             String[] hosts = GenericUtils.split(hostsList, ',');
+            boolean traceEnabled = log.isTraceEnabled();
             for (String host : hosts) {
-                if (log.isDebugEnabled()) {
+                if (debugEnabled) {
                     log.debug("start() - resolve bind host={}", host);
                 }
 
                 InetAddress[] inetAddresses = InetAddress.getAllByName(host);
                 for (InetAddress inetAddress : inetAddresses) {
-                    if (log.isTraceEnabled()) {
+                    if (traceEnabled) {
                         log.trace("start() - bind host={} / {}", host, inetAddress);
                     }
 
                     acceptor.bind(new InetSocketAddress(inetAddress, port));
                     if (port == 0) {
-                        port = ((InetSocketAddress) acceptor.getBoundAddresses().iterator().next()).getPort();
-                        log.info("start() listen on auto-allocated port=" + port);
+                        port = IoAcceptor.resolveBoundPort(acceptor);
+                        ValidateUtils.checkState(port > 0, "Cannot resolve bound port for host=%s", host);
+                        log.info("start() listen on auto-allocated port={} for host={}[{}]", port, host, inetAddress);
                     }
                 }
             }
         } else {
             acceptor.bind(new InetSocketAddress(port));
             if (port == 0) {
-                port = ((InetSocketAddress) acceptor.getBoundAddresses().iterator().next()).getPort();
-                log.info("start() listen on auto-allocated port=" + port);
+                port = IoAcceptor.resolveBoundPort(acceptor);
+                ValidateUtils.checkState(port > 0, "Cannot resolve generic bound port");
+                log.info("start({}) listen on auto-allocated port={}", this, port);
             }
         }
 

diff --git a/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java b/sshd-core/src/test/java/org/apache/sshd/client/ClientTest.java
@@ -244,6 +244,32 @@ public void testClientStartedIndicator() throws Exception {
         assertFalse("Client not marked as stopped", client.isStarted());
     }
 
+    @Test
+    public void testClientStopIsIdempotent() throws Exception {
+        client.start();
+        for (int index = 1; index <= 4; index++) {
+            client.stop();
+        }
+    }
+
+    @Test   // see SSHD-340
+    public void testClientIsRestartable() throws Exception {
+        for (int index = 1; index <= 4; index++) {
+            client.start();
+            assertTrue("Client not started at attempt #" + index, client.isStarted());
+            assertTrue("Client not open at attempt #" + index, client.isOpen());
+
+            try (ClientSession s = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+                s.addPasswordIdentity(getCurrentTestName());
+                s.auth().verify(11L, TimeUnit.SECONDS);
+            } catch (Exception e) {
+                fail("Failed (" + e.getClass().getSimpleName() + ") to authenticate at attempt #" + index + ": " + e.getMessage());
+            }
+
+            client.stop();
+        }
+    }
+
     @Test
     public void testPropertyResolutionHierarchy() throws Exception {
         String sessionPropName = getCurrentTestName() + "-session";

diff --git a/sshd-core/src/test/java/org/apache/sshd/server/SshServerTest.java b/sshd-core/src/test/java/org/apache/sshd/server/SshServerTest.java
@@ -20,7 +20,10 @@
 
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
 
+import org.apache.sshd.client.SshClient;
+import org.apache.sshd.client.session.ClientSession;
 import org.apache.sshd.util.test.BaseTestSupport;
 import org.junit.FixMethodOrder;
 import org.junit.Test;
@@ -81,8 +84,30 @@ public void testDynamicPort() throws Exception {
             sshd.start();
 
             assertNotEquals(0, sshd.getPort());
-
             sshd.stop();
         }
     }
+
+    @Test   // see SSHD-340
+    public void testServerRestartable() throws Exception {
+        try (SshClient client = setupTestClient();
+             SshServer sshd = setupTestServer()) {
+            sshd.setHost(TEST_LOCALHOST);
+            client.start();
+
+            for (int index = 1; index <= 4; index++) {
+                sshd.start();
+                assertTrue("SSHD not started at attempt #" + index, sshd.isStarted());
+                assertTrue("SSHD not open at attempt #" + index, sshd.isOpen());
+
+                int port = sshd.getPort();
+                try (ClientSession s = client.connect(getCurrentTestName(), TEST_LOCALHOST, port).verify(7L, TimeUnit.SECONDS).getSession()) {
+                    s.addPasswordIdentity(getCurrentTestName());
+                    s.auth().verify(11L, TimeUnit.SECONDS);
+                } catch (Exception e) {
+                    fail("Failed (" + e.getClass().getSimpleName() + ") to authenticate at attempt #" + index + ": " + e.getMessage());
+                }
+            }
+        }
+    }
 }