[mina-sshd] branch SSHD-931 created (now c2ba677)

[lg...@apache.org]

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

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


      at c2ba677  [SSHD-931] Added SFTP ExecutorProtectingSubsystemFactory

This branch includes the following new commits:

     new c2ba677  [SSHD-931] Added SFTP ExecutorProtectingSubsystemFactory

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

[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);