[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 <[email protected]> Authored: Sat Aug 25 11:54:10 2018 +0300 Committer: Goldstein Lyor <[email protected]> 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>
