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/08/26 06:24:54 UTC

[5/5] mina-sshd git commit: [SSHD-841] Use Nio2ServiceFactoryFactory as the hardwired default if no other found or explicitly set

[SSHD-841] Use Nio2ServiceFactoryFactory as the hardwired default if no other found or explicitly set


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/ab8194d6
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/ab8194d6
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/ab8194d6

Branch: refs/heads/master
Commit: ab8194d6e956fda6384e5687225cbc4ed5a4a965
Parents: 12026ed
Author: Lyor Goldstein <ly...@gmail.com>
Authored: Sat Aug 25 11:54:10 2018 +0300
Committer: Goldstein Lyor <ly...@cb4.com>
Committed: Sun Aug 26 09:24:44 2018 +0300

----------------------------------------------------------------------
 README.md                                       |  29 +++++++--
 assembly/src/main/distribution/bin/scp.sh       |   1 -
 assembly/src/main/distribution/bin/sftp.sh      |   1 -
 .../src/main/distribution/bin/ssh-keyscan.sh    |   1 -
 assembly/src/main/distribution/bin/ssh.sh       |   1 -
 assembly/src/main/distribution/bin/sshd.sh      |   1 -
 sshd-cli/key.ser                                | Bin 0 -> 1936 bytes
 sshd-core/pom.xml                               |   3 -
 ...pache.sshd.common.io.IoServiceFactoryFactory |  20 ------
 .../io/DefaultIoServiceFactoryFactory.java      |  62 ++++++++++++++-----
 sshd-mina/pom.xml                               |   3 -
 sshd-netty/pom.xml                              |   3 -
 12 files changed, 73 insertions(+), 52 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ab8194d6/README.md
----------------------------------------------------------------------
diff --git a/README.md b/README.md
index 251370a..0458a8e 100644
--- a/README.md
+++ b/README.md
@@ -53,9 +53,8 @@ Required mainly for writing keys to PEM files or for special keys/ciphers/etc. t
 ## NIO2 default socket factory replacements
 
 Optional dependency to enable choosing between NIO asynchronous sockets (the default - for improved performance), and "legacy" sockets.
-See `IoServiceFactoryFactory` implementations and specifically the `DefaultIoServiceFactoryFactory` for the available options and how it
-can be configured to select among them. **Note:** the required Maven module(s) are defined as `optional` so must be added as an **explicit**
-dependency in order to be included in the classpath.
+**Note:** the required Maven module(s) are defined as `optional` so must be added as an **explicit** dependency in order to be included
+in the classpath.
 
 ### [MINA core](https://mina.apache.org/mina-project/)
 
@@ -102,6 +101,28 @@ implementation. This is also an **optional** dependency and must be add explicit
 
 ```
 
+### Selecting an `IoServiceFactoryFactory`
+
+As part of the their initialization, both client and server code require the specification of a `IoServiceFactoryFactory`
+that is used to initialize network connections. If not set explicitly during the client/server setup code, then a factory
+is automatically detected and selected when the client/server is started. The used `IoServiceFactoryFactory` is a **singleton**
+that is lazy created the 1st time `DefaultIoServiceFactoryFactory#create` is invoked. The selection process is as follows:
+
+* The `org.apache.sshd.common.io.IoServiceFactoryFactory` system property is examined for a factory specification. The
+specification can be either a **fully-qualified** class name or one of the `BuiltinIoServiceFactoryFactories` values.
+
+* If no specific factory is specified, then the [ServiceLoader#load](https://docs.oracle.com/javase/tutorial/ext/basics/spi.html#register-service-providers)
+mechanism is used to detect and instantiate any registered services in any `META-INF\services\org.apache.sshd.common.io.IoServiceFactoryFactory`
+location in the classpath. If **exactly one** implementation was instantiated, then it is used. If several such implementations are found then
+an exception is thrown.
+
+* Otherwise, the built-in `Nio2ServiceFactoryFactory` is used.
+
+**Note:** the default command line scripts for SSH client/server, SCP/SFTP clients are set up to use NIO2 as their default provider,
+unless overridden via the `-io` command line option. The `org.apache.sshd.common.io.IoServiceFactoryFactory` system property does
+not apply for the command line wrappers since they look for only the `-io` option and use it to initialize the client/server **explicitly**,
+before starting the client/server. Therefore, the default selection process described in this section does not take effect.
+
 ## [ed25519-java](https://github.com/str4d/ed25519-java)
 
 
@@ -950,7 +971,7 @@ On the client side, all the supported extensions are classes that implement `Sft
                 if (SftpConstants.EXT_ACL_SUPPORTED.equalsIgnoreCase(extName)) {
                     AclCapabilities capabilities = (AclCapabilities) extValue;
                     ...see what other information can be gleaned from it...
-                } else if ((SftpConstants.EXT_VERSIONS.equalsIgnoreCase(extName)) {
+                } else if (SftpConstants.EXT_VERSIONS.equalsIgnoreCase(extName)) {
                     Versions versions = (Versions) extValue;
                     ...see what other information can be gleaned from it...
                 } else if ("my-special-extension".equalsIgnoreCase(extName)) {

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ab8194d6/assembly/src/main/distribution/bin/scp.sh
----------------------------------------------------------------------
diff --git a/assembly/src/main/distribution/bin/scp.sh b/assembly/src/main/distribution/bin/scp.sh
index 0d28c89..152a575 100644
--- a/assembly/src/main/distribution/bin/scp.sh
+++ b/assembly/src/main/distribution/bin/scp.sh
@@ -246,7 +246,6 @@ init() {
 
     # Install debug options
     setupDebugOptions
-
 }
 
 run() {

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ab8194d6/assembly/src/main/distribution/bin/sftp.sh
----------------------------------------------------------------------
diff --git a/assembly/src/main/distribution/bin/sftp.sh b/assembly/src/main/distribution/bin/sftp.sh
index 477e7fe..d77a891 100644
--- a/assembly/src/main/distribution/bin/sftp.sh
+++ b/assembly/src/main/distribution/bin/sftp.sh
@@ -246,7 +246,6 @@ init() {
 
     # Install debug options
     setupDebugOptions
-
 }
 
 run() {

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ab8194d6/assembly/src/main/distribution/bin/ssh-keyscan.sh
----------------------------------------------------------------------
diff --git a/assembly/src/main/distribution/bin/ssh-keyscan.sh b/assembly/src/main/distribution/bin/ssh-keyscan.sh
index 083f7b8..169421d 100644
--- a/assembly/src/main/distribution/bin/ssh-keyscan.sh
+++ b/assembly/src/main/distribution/bin/ssh-keyscan.sh
@@ -246,7 +246,6 @@ init() {
 
     # Install debug options
     setupDebugOptions
-
 }
 
 run() {

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ab8194d6/assembly/src/main/distribution/bin/ssh.sh
----------------------------------------------------------------------
diff --git a/assembly/src/main/distribution/bin/ssh.sh b/assembly/src/main/distribution/bin/ssh.sh
index fe07901..90769f4 100644
--- a/assembly/src/main/distribution/bin/ssh.sh
+++ b/assembly/src/main/distribution/bin/ssh.sh
@@ -246,7 +246,6 @@ init() {
 
     # Install debug options
     setupDebugOptions
-
 }
 
 run() {

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ab8194d6/assembly/src/main/distribution/bin/sshd.sh
----------------------------------------------------------------------
diff --git a/assembly/src/main/distribution/bin/sshd.sh b/assembly/src/main/distribution/bin/sshd.sh
index a7b21ed..d5bd930 100644
--- a/assembly/src/main/distribution/bin/sshd.sh
+++ b/assembly/src/main/distribution/bin/sshd.sh
@@ -246,7 +246,6 @@ init() {
 
     # Install debug options
     setupDebugOptions
-
 }
 
 run() {

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ab8194d6/sshd-cli/key.ser
----------------------------------------------------------------------
diff --git a/sshd-cli/key.ser b/sshd-cli/key.ser
new file mode 100644
index 0000000..824484a
Binary files /dev/null and b/sshd-cli/key.ser differ

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ab8194d6/sshd-core/pom.xml
----------------------------------------------------------------------
diff --git a/sshd-core/pom.xml b/sshd-core/pom.xml
index 2046d0c..0bcab83 100644
--- a/sshd-core/pom.xml
+++ b/sshd-core/pom.xml
@@ -156,9 +156,6 @@
                 <configuration>
                     <redirectTestOutputToFile>true</redirectTestOutputToFile>
                     <reportsDirectory>${project.build.directory}/surefire-reports-nio2</reportsDirectory>
-                    <systemProperties>
-                        <org.apache.sshd.common.io.IoServiceFactoryFactory>org.apache.sshd.common.io.nio2.Nio2ServiceFactoryFactory</org.apache.sshd.common.io.IoServiceFactoryFactory>
-                    </systemProperties>
                     <excludes>
                         <exclude>**/*LoadTest.java</exclude>
                     </excludes>

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ab8194d6/sshd-core/src/main/filtered-resources/META-INF/services/org.apache.sshd.common.io.IoServiceFactoryFactory
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/filtered-resources/META-INF/services/org.apache.sshd.common.io.IoServiceFactoryFactory b/sshd-core/src/main/filtered-resources/META-INF/services/org.apache.sshd.common.io.IoServiceFactoryFactory
deleted file mode 100644
index 4190aeb..0000000
--- a/sshd-core/src/main/filtered-resources/META-INF/services/org.apache.sshd.common.io.IoServiceFactoryFactory
+++ /dev/null
@@ -1,20 +0,0 @@
-##
-## Licensed to the Apache Software Foundation (ASF) under one
-## or more contributor license agreements.  See the NOTICE file
-## distributed with this work for additional information
-## regarding copyright ownership.  The ASF licenses this file
-## to you under the Apache License, Version 2.0 (the
-## "License"); you may not use this file except in compliance
-## with the License.  You may obtain a copy of the License at
-##
-##  http://www.apache.org/licenses/LICENSE-2.0
-##
-## Unless required by applicable law or agreed to in writing,
-## software distributed under the License is distributed on an
-## "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-## KIND, either express or implied.  See the License for the
-## specific language governing permissions and limitations
-## under the License.
-##
-
-org.apache.sshd.common.io.nio2.Nio2ServiceFactoryFactory

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ab8194d6/sshd-core/src/main/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactory.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactory.java b/sshd-core/src/main/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactory.java
index 62ae203..1e95fd0 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactory.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/io/DefaultIoServiceFactoryFactory.java
@@ -18,7 +18,9 @@
  */
 package org.apache.sshd.common.io;
 
+import java.util.Deque;
 import java.util.Iterator;
+import java.util.LinkedList;
 import java.util.ServiceLoader;
 
 import org.apache.sshd.common.Factory;
@@ -55,19 +57,31 @@ public class DefaultIoServiceFactoryFactory extends AbstractIoServiceFactoryFact
      */
     public IoServiceFactoryFactory getIoServiceProvider() {
         synchronized (this) {
+            if (factory != null) {
+                return factory;
+            }
+
+            factory = newInstance(IoServiceFactoryFactory.class);
             if (factory == null) {
-                factory = newInstance(IoServiceFactoryFactory.class);
-                Factory<CloseableExecutorService> executorServiceFactory = getExecutorServiceFactory();
-                if (executorServiceFactory != null) {
-                    factory.setExecutorServiceFactory(executorServiceFactory);
-                }
+                factory = BuiltinIoServiceFactoryFactories.NIO2.create();
+                log.info("No detected/configured " + IoServiceFactoryFactory.class.getSimpleName()
+                    + " using " + factory.getClass().getSimpleName());
+            } else {
+                log.info("Using {}", factory.getClass().getSimpleName());
+            }
+
+            Factory<CloseableExecutorService> executorServiceFactory = getExecutorServiceFactory();
+            if (executorServiceFactory != null) {
+                factory.setExecutorServiceFactory(executorServiceFactory);
             }
         }
+
         return factory;
     }
 
     public static <T extends IoServiceFactoryFactory> T newInstance(Class<T> clazz) {
-        String factory = System.getProperty(clazz.getName());
+        String propName = clazz.getName();
+        String factory = System.getProperty(propName);
         if (!GenericUtils.isEmpty(factory)) {
             return newInstance(clazz, factory);
         }
@@ -75,7 +89,7 @@ public class DefaultIoServiceFactoryFactory extends AbstractIoServiceFactoryFact
         Thread currentThread = Thread.currentThread();
         ClassLoader cl = currentThread.getContextClassLoader();
         if (cl != null) {
-            T t = tryLoad(ServiceLoader.load(clazz, cl));
+            T t = tryLoad(propName, ServiceLoader.load(clazz, cl));
             if (t != null) {
                 return t;
             }
@@ -83,28 +97,48 @@ public class DefaultIoServiceFactoryFactory extends AbstractIoServiceFactoryFact
 
         ClassLoader clDefault = DefaultIoServiceFactoryFactory.class.getClassLoader();
         if (cl != clDefault) {
-            T t = tryLoad(ServiceLoader.load(clazz, clDefault));
+            T t = tryLoad(propName, ServiceLoader.load(clazz, clDefault));
             if (t != null) {
                 return t;
             }
         }
-        throw new IllegalStateException("Could not find a valid sshd io provider");
+
+        return null;
     }
 
-    public static <T extends IoServiceFactoryFactory> T tryLoad(ServiceLoader<T> loader) {
+    public static <T extends IoServiceFactoryFactory> T tryLoad(String propName, ServiceLoader<T> loader) {
         Iterator<T> it = loader.iterator();
+        Deque<T> services = new LinkedList<>();
         try {
             while (it.hasNext()) {
                 try {
-                    return it.next();
+                    T instance = it.next();
+                    services.add(instance);
                 } catch (Throwable t) {
-                    LOGGER.trace("Exception while loading factory from ServiceLoader", t);
+                    LOGGER.warn("Exception while instantiating factory from ServiceLoader", t);
                 }
             }
         } catch (Throwable t) {
-            LOGGER.trace("Exception while loading factory from ServiceLoader", t);
+            LOGGER.warn("Exception while loading factory from ServiceLoader", t);
         }
-        return null;
+
+        int numDetected = services.size();
+        if (numDetected <= 0) {
+            return null;
+        }
+
+        if (numDetected != 1) {
+            LOGGER.error("Multiple ({}) registered instances detected:", numDetected);
+            for (T s : services) {
+                LOGGER.error("===> {}", s.getClass().getName());
+            }
+            throw new IllegalStateException("Multiple (" + numDetected + ")"
+                + " registered " + IoServiceFactoryFactory.class.getSimpleName() + " instances detected."
+                + " Please use -D" + propName + "=...factory class.. to select one"
+                + " or remove the extra providers from the classpath");
+        }
+
+        return services.removeFirst();
     }
 
     public static <T extends IoServiceFactoryFactory> T newInstance(Class<T> clazz, String factory) {

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ab8194d6/sshd-mina/pom.xml
----------------------------------------------------------------------
diff --git a/sshd-mina/pom.xml b/sshd-mina/pom.xml
index ec900fe..14f37a5 100644
--- a/sshd-mina/pom.xml
+++ b/sshd-mina/pom.xml
@@ -155,9 +155,6 @@
                 <configuration>
                     <redirectTestOutputToFile>true</redirectTestOutputToFile>
                     <reportsDirectory>${project.build.directory}/surefire-reports-mina</reportsDirectory>
-                    <systemProperties>
-                        <org.apache.sshd.common.io.IoServiceFactoryFactory>org.apache.sshd.common.io.mina.MinaServiceFactoryFactory</org.apache.sshd.common.io.IoServiceFactoryFactory>
-                    </systemProperties>
                     <excludes>
                             <!-- These tests use NIO explicitly -->
                         <exclude>**/*LoadTest.java</exclude>

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ab8194d6/sshd-netty/pom.xml
----------------------------------------------------------------------
diff --git a/sshd-netty/pom.xml b/sshd-netty/pom.xml
index 4fa6b26..9f77cc1 100644
--- a/sshd-netty/pom.xml
+++ b/sshd-netty/pom.xml
@@ -173,9 +173,6 @@
                 <configuration>
                     <redirectTestOutputToFile>true</redirectTestOutputToFile>
                     <reportsDirectory>${project.build.directory}/surefire-reports-netty</reportsDirectory>
-                    <systemProperties>
-                        <org.apache.sshd.common.io.IoServiceFactoryFactory>org.apache.sshd.netty.NettyIoServiceFactoryFactory</org.apache.sshd.common.io.IoServiceFactoryFactory>
-                    </systemProperties>
                     <excludes>
                             <!-- These tests use NIO and/or MINA explicitly -->
                         <exclude>**/*LoadTest.java</exclude>