Memory Leak in SftpFileSystemProvider

[javadaydayup]

Version

master

Bug description

The newInputStream and newOutputStream methods of SftpFileSystemProvider call the SftpFileSystem#getClient() method without call the close() method:

Actual behavior

I have a thread that uploads a file to the server every 10 seconds, as time goes by, the heap memory will get bigger and bigger, and eventually the heap memory will occur out of memory:

When I exported the heap memory, I found that there are a lot of objects of org.apache.sshd.sftp.client.fs.SftpFileSystem$Wrapper

Expected behavior

When the program is running normally, the heap memory will not overflow

Relevant log output

No response

Other information

No response

[luchenguang]

Meet the same question when using SftpFileSystem, seems to be caused by SftpFileSystem using a ThreadLocal to cache a sftp client but unable to clean it

[tomaswolf]

Thanks for pointing out this bug. It should be fixed by the referenced pull request.

But personally I'm not convinced using a ThreadLocal to store these wrappers is a good idea. It appears the idea was to use different SftpClients and thus SSH channels for different threads. I cannot be sure though since there is no design documentation and this code was written long before I became involved here. I'm not entirely sure that using ThreadLocals for this works as intended in all cases.

[hitYunhongXu]

hello, how can i get the code without memory leak? it seems that the problem is not fixed in version 2.9.2

[zongbintu]

@tomaswolf
I use ThreadPool operate many files.
sftp version：2.9.3-SNAPSHOT
Using the 2.9.3-SNAPSHOT, still unable to release.

[zongbintu]

I switched to SftpClient and it's working normally now.

in ThreadPool use

Bad

    public String downloadFile(String path) {
        try (ClientSession session = client.connect(username, host, port).verify(timeout, TimeUnit.SECONDS).getSession()) {
            session.addPasswordIdentity(password);
            session.auth().verify();
            try (SftpFileSystem fs = SftpClientFactory.instance().createSftpFileSystem(session);
                 ByteArrayOutputStream out = new ByteArrayOutputStream()
            ) {
                Path targetPath = fs.getDefaultDir().resolve(path);
                Files.copy(targetPath, out);
                return out.toString("UTF-8");
            }
        } catch (SftpException e) {
            log.error("download file:{} error:{}.", path, e.getMessage());
        } catch (Exception e) {
            log.error("download file:{} error.", path, e);
        }
        return null;
    }

Good

Version：2.8.0

    public String downloadFile(String path) {
        try (ClientSession session = client.connect(username, host, port).verify(timeout, TimeUnit.SECONDS).getSession()) {
            session.addPasswordIdentity(password);
            session.auth().verify();

            SftpClientFactory factory = SftpClientFactory.instance();

            try (SftpClient sftp = factory.createSftpClient(session);
                 StringWriter writer = new StringWriter();) {
                InputStream inputStream = sftp.read(path);
                IOUtils.copy(inputStream, writer, StandardCharsets.UTF_8);

                String contentStr = writer.toString();

                return contentStr;
            }
        } catch (IOException e) {
            log.error("download file:{} error.", path, e);
        }
        return null;
    }

After downloading, moving, and parsing approximately 40142 files, Each file has an approximate size of 4KB.

[tomaswolf]

I cannot reproduce this. Can you show fully working test code using a thread pool (I presume from an ExecutorService) that downloads a number of files and where at the end, when all downloads are done, there are extra ThreadLocals in the pool threads?