[mina-sshd] 01/01: [SSHD-931] Added SFTP ExecutorProtectingSubsystemFactory

[lg...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch SSHD-931
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit c2ba677bf094e1e5764d4c4e08ad82544012d586
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Wed Jul 31 08:24:42 2019 +0300

    [SSHD-931] Added SFTP ExecutorProtectingSubsystemFactory
---
 docs/sftp.md                                       | 11 +++-
 .../sshd/common/util/threads/NoCloseExecutor.java  |  1 -
 .../sshd/common/util/threads/ThreadUtils.java      |  7 ++-
 .../sftp/ExecutorProtectingSubsystemFactory.java   | 71 ++++++++++++++++++++++
 .../subsystem/sftp/SftpSubsystemFactory.java       |  6 +-
 5 files changed, 89 insertions(+), 7 deletions(-)

diff --git a/docs/sftp.md b/docs/sftp.md
index 49dee63..5faff5f 100644
--- a/docs/sftp.md
+++ b/docs/sftp.md
@@ -25,7 +25,7 @@ On the server side, the following code needs to be added:
 
 ```
 
-### `SftpEventListener`
+#### `SftpEventListener`
 
 Provides information about major SFTP protocol events. The provided `File/DirectoryHandle` to the various callbacks can also be used to
 store user-defined attributes via its `AttributeStore` implementation. The listener is registered at the `SftpSubsystemFactory`:
@@ -61,7 +61,7 @@ store user-defined attributes via its `AttributeStore` implementation. The liste
 whether the close attempt was successful or not. In other words, after `SftpEventListener#closed` has been
 called, all attributes associated with the handle are cleared.
 
-### `SftpFileSystemAccessor`
+#### `SftpFileSystemAccessor`
 
 This is the abstraction providing the SFTP server subsystem access to files and directories. The SFTP subsystem
 uses this abstraction to obtain file channels and/or directory streams. One can override the default implementation
@@ -77,6 +77,13 @@ The accessor is registered/overwritten in via the `SftpSubSystemFactory`:
 
 ```
 
+#### Externally provided `CloseableExecutorService` re-use
+
+By default, the `SftpSubsystem` will close the executor service it uses when it is destroyed when the SFTP session
+is terminated. This is OK for the default internal executor being used. However, if users wish to provide their own
+service which should not be shut down on exit, then a special wrapper is required. Such a wrapper is available via
+the `ExecutorProtectingSubsystemFactory` class (available via the *sshd-contrib* module).
+
 **Note:**
 
 * Closing of file channel/directory streams created by the accessor are also closed
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/threads/NoCloseExecutor.java b/sshd-common/src/main/java/org/apache/sshd/common/util/threads/NoCloseExecutor.java
index cb42805..6bb096b 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/util/threads/NoCloseExecutor.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/util/threads/NoCloseExecutor.java
@@ -156,5 +156,4 @@ public class NoCloseExecutor implements CloseableExecutorService {
     public boolean isClosing() {
         return isClosed();
     }
-
 }
\ No newline at end of file
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ThreadUtils.java b/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ThreadUtils.java
index bef05f0..dd46612 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ThreadUtils.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ThreadUtils.java
@@ -48,12 +48,13 @@ public final class ThreadUtils {
      *
      * @param executorService The original service - ignored if {@code null}
      * @param shutdownOnExit  If {@code true} then it is OK to shutdown the executor
-     *                        so no wrapping takes place.
+     * so no wrapping takes place.
      * @return Either the original service or a wrapped one - depending on the
      * value of the <tt>shutdownOnExit</tt> parameter
      */
-    public static CloseableExecutorService protectExecutorServiceShutdown(CloseableExecutorService executorService, boolean shutdownOnExit) {
-        if (executorService == null || shutdownOnExit || executorService instanceof NoCloseExecutor) {
+    public static CloseableExecutorService protectExecutorServiceShutdown(
+            CloseableExecutorService executorService, boolean shutdownOnExit) {
+        if ((executorService == null) || shutdownOnExit || (executorService instanceof NoCloseExecutor)) {
             return executorService;
         } else {
             return new NoCloseExecutor(executorService);
diff --git a/sshd-contrib/src/main/java/org/apache/sshd/server/subsystem/sftp/ExecutorProtectingSubsystemFactory.java b/sshd-contrib/src/main/java/org/apache/sshd/server/subsystem/sftp/ExecutorProtectingSubsystemFactory.java
new file mode 100644
index 0000000..b3e6a52
--- /dev/null
+++ b/sshd-contrib/src/main/java/org/apache/sshd/server/subsystem/sftp/ExecutorProtectingSubsystemFactory.java
@@ -0,0 +1,71 @@
+/*
+ * 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.
+ */
+
+package org.apache.sshd.server.subsystem.sftp;
+
+import org.apache.sshd.common.util.threads.CloseableExecutorService;
+import org.apache.sshd.common.util.threads.ThreadUtils;
+
+/**
+ * A {@link SftpSubsystemFactory} that protects its {@link CloseableExecutorService}
+ * so that when the subsystem is destroyed it will not close/shutdown the service.
+ *
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class ExecutorProtectingSubsystemFactory extends SftpSubsystemFactory {
+    public static class Builder extends SftpSubsystemFactory.Builder {
+        public Builder() {
+            super();
+        }
+
+        @Override
+        public Builder withExecutorService(CloseableExecutorService service) {
+            return (Builder) super.withExecutorService(service);
+        }
+
+        @Override
+        public Builder withFileSystemAccessor(SftpFileSystemAccessor accessor) {
+            return (Builder) super.withFileSystemAccessor(accessor);
+        }
+
+        @Override
+        public Builder withSftpErrorStatusDataHandler(SftpErrorStatusDataHandler handler) {
+            return (Builder) super.withSftpErrorStatusDataHandler(handler);
+        }
+
+        @Override
+        public Builder withUnsupportedAttributePolicy(UnsupportedAttributePolicy p) {
+            return (Builder) super.withUnsupportedAttributePolicy(p);
+        }
+
+        @Override
+        protected SftpSubsystemFactory createSftpSubsystemFactory() {
+            return new ExecutorProtectingSubsystemFactory();
+        }
+    }
+
+    public ExecutorProtectingSubsystemFactory() {
+        super();
+    }
+
+    @Override   // see SSHD-931
+    public CloseableExecutorService getExecutorService() {
+        return ThreadUtils.noClose(super.getExecutorService());
+    }
+}
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystemFactory.java b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystemFactory.java
index 056b5fa..015f47f 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystemFactory.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystemFactory.java
@@ -68,9 +68,13 @@ public class SftpSubsystemFactory
             return this;
         }
 
+        protected SftpSubsystemFactory createSftpSubsystemFactory() {
+            return new SftpSubsystemFactory();
+        }
+
         @Override
         public SftpSubsystemFactory build() {
-            SftpSubsystemFactory factory = new SftpSubsystemFactory();
+            SftpSubsystemFactory factory = createSftpSubsystemFactory();
             factory.setExecutorService(executors);
             factory.setUnsupportedAttributePolicy(policy);
             factory.setFileSystemAccessor(fileSystemAccessor);