Skip to content

Commit 4eccf49

Browse files
committed
GH-855: single SftpClient in SftpFileSystem
Drop the SftpClient pool and use a single thread-safe SftpClient per SftpFileSystem. The SftpClient is closed when the file system is closed. If the server side should close the channel, operations using this client may fail, but a server should not close channels unless there is no traffic. If a channel is closed by the server because it is idle, the SftpFileSystem will re-create a new SftpClient, thus opening a new channel, on the next operation. It is possible that a server decides to close a previously idle channel just in the moment the client-side wants to use that channel for a new operation. The new operation would then fail. But this possibility has always existed in all previous versions of the SftpFileSystem code. The main advantage of using a single SftpClient is that all SFTP operations are done in the same channel. This can be important with servers that restrict the number of concurrently open channels per SSH session.
1 parent c69cd0c commit 4eccf49

4 files changed

Lines changed: 136 additions & 228 deletions

File tree

CHANGES.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
* [GH-809](https://github.com/apache/mina-sshd/pull/809) Fix server-side authentication for FIDO/U2F sk-* keys with flags in `authorized_keys`
3535
* [GH-827](https://github.com/apache/mina-sshd/issues/827) Don't fail on invalid `known_hosts` lines; log and skip them
3636
* [GH-830](https://github.com/apache/mina-sshd/issues/830) EC public keys: let Bouncy Castle generate X.509 encodings with the curve OID as algorithm parameter
37+
* [GH-855](https://github.com/apache/mina-sshd/issues/855) SFTP: use a single `SftpClient` per `SftpFileSystem`
3738
* [GH-856](https://github.com/apache/mina-sshd/issues/856) Fix using ed25519 with BC-FIPS
3839
* [GH-861](https://github.com/apache/mina-sshd/issues/861) SFTP client: prevent sending zero-length writes in `SftpOutputStreamAsync`
3940

@@ -46,5 +47,9 @@ safe despite that CVE in the dependency.
4647

4748
## Potential Compatibility Issues
4849

50+
[GH-855](https://github.com/apache/mina-sshd/issues/855) changes the way `SftpFileSystem` deals with multiple threads. It newly uses a single SSH channel via a single thread-safe `SftpClient`, serializing writes at the channel level. The properties relating to the previously used pool of `SftpClient`s have been deprecated and have no effect anymore. User applications using the library should not see any changes.
51+
52+
A beneficial side-effect of this change is that an `SftpFileSystem` creates the SSH session and SFTP channel only when the first SFTP operation is performed. Previously the session and channel were opened right away when an `SftpFileSystem` was instantiated.
53+
4954
## Major Code Re-factoring
5055

sshd-sftp/src/main/java/org/apache/sshd/sftp/SftpModuleProperties.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,10 @@ public final class SftpModuleProperties {
6363
* The maximum size of the channel pool used by an {@link org.apache.sshd.sftp.client.fs.SftpFileSystem}; by default
6464
* 8. The value must be > zero.
6565
*
66-
* @see org.apache.sshd.sftp.client.fs.SftpFileSystem
66+
* @see org.apache.sshd.sftp.client.fs.SftpFileSystem
67+
* @deprecated since 2.17.0 this has no effect since there is no pool anymore
6768
*/
69+
@Deprecated
6870
public static final Property<Integer> POOL_SIZE
6971
= Property.integer("sftp-fs-pool-size", 8);
7072

@@ -76,8 +78,10 @@ public final class SftpModuleProperties {
7678
* The duration should not be shorter than 1 millisecond. If it is, 1 millisecond will be assumed.
7779
* </p>
7880
*
79-
* @see org.apache.sshd.sftp.client.fs.SftpFileSystem
81+
* @see org.apache.sshd.sftp.client.fs.SftpFileSystem
82+
* @deprecated since 2.17.0 this has no effect since there is no pool anymore
8083
*/
84+
@Deprecated
8185
public static final Property<Duration> POOL_LIFE_TIME
8286
= Property.duration("sftp-fs-pool-life-time", Duration.ofSeconds(10));
8387

@@ -87,8 +91,10 @@ public final class SftpModuleProperties {
8791
* {@link #POOL_SIZE}, channels will not expire and will be closed only then the file system is closed, or if the
8892
* server closes them.
8993
*
90-
* @see org.apache.sshd.sftp.client.fs.SftpFileSystem
94+
* @see org.apache.sshd.sftp.client.fs.SftpFileSystem
95+
* @deprecated since 2.17.0 this has no effect since there is no pool anymore
9196
*/
97+
@Deprecated
9298
public static final Property<Integer> POOL_CORE_SIZE
9399
= Property.integer("sftp-fs-pool-core-size", 1);
94100

0 commit comments

Comments
 (0)