robinrodricks / FluentFTP

An FTP and FTPS client for .NET & .NET Standard, optimized for speed. Provides extensive FTP commands, File uploads/downloads, SSL/TLS connections, Automatic directory listing parsing, File hashing/checksums, File permissions/CHMOD, FTP proxies, FXP support, UTF-8 support, Async/await support, Powershell support and more. Written entirely in C#.
MIT License
3.14k stars 656 forks source link

FluentFTP.GnuTLS possible memory leak #1579

Closed validide closed 1 month ago

validide commented 5 months ago

FTP Server OS: Unknown

FTP Server Type: Unknown

Client Computer OS: Debian

FluentFTP Version: 50.0.1 FluentFTP.GnuTLS Version: 1.0.30/1.0.31

Framework: .NET 7

Hi, I am working on an app that periodically connects to customer FTPS server and downloads some files (using the FluentFTP.GnuTLS package). Everything works well apart for increased memory usage in over time (days). After taking some memory dumps it seems that the unmanaged memory is the one that keeps increasing.

I can not provide any logs at this time but I will try to generate a sample project to reproduce the issue.

I have looked in the FluentFTP.GnuTLS and repository and I saw that the FunctionLoader.Free() call has been removed and that function is not called anywhere else. Could this be the cause of the issue?

I know there is not that much information in the issue.

FanDjango commented 5 months ago

Thanks for reporting this.

Everything works well apart for increased memory usage in over time (days). After taking some memory dumps it seems that the unmanaged memory is the one that keeps increasing.

This is important information

After taking some memory dumps it seems that the unmanaged memory is the one that keeps increasing.

Not surprising

I will try to generate a sample project to reproduce the issue

See next...

FunctionLoader.Free() call has been removed and that function is not called anywhere else. Could this be the cause of the issue?

Well, the idea was to keep the functions loaded (forever) but to initialize and deinitalize the GnuTLS binarys when they are no longer in use.

Certainly there may be a logic error in the code somewhere - probably Dispose(). If you have a reproducible mini-console-app that eats memory, let's isolate the problem step by step.

validide commented 5 months ago

Hi,

I tried to create a Docker environment to test this and I think I managed to reproduce the issue: https://github.com/validide/FluentFTP.GnuTLS

At the moment the application tries to connect to the server but fails with an exception. After running for 1000 iterations (just creating the client and trying to connect) it gets up to 3GB RAM consumed on my local setup.

I have 0 experience working with FTPS so I am not sure if this test is correct.

FanDjango commented 5 months ago

Also Hi!

I had a look at your source:

static async Task SimulateWork(int target) {
    var count = 0;
    while (count < target) {
        using (var conn = new FtpClient("ftp-server", "ftptest", "ftptest")) {
            ConfigureConnection(conn);

            await Task.Delay(100);

            try {
                conn.Connect();
            }
            catch (Exception ex) {
                Console.WriteLine(ex.ToString());
            }
        }
        count++;
        Console.WriteLine($"Created {count} clients.");
    }
}

Can you try this with "await using"? and report back? Using "just using" takes things down a different path on dispose (although I thought I was successful in catching this).

FanDjango commented 5 months ago

I have 0 experience working with FTPS so I am not sure if this test is correct.

Sheesh. All is good, you are on the track of something perhaps.

FanDjango commented 5 months ago

Also, it would be worth dockering a test with SYNC client - because actually FluentFTP.GnuTLS does not really support async operations. I was hoping it would gracefully block if used in an async environment - but it would be nice if you would see if there is a difference.

validide commented 5 months ago

Hi, I just pushed an updated version on my repository and there seems to be no difference between the sync and async version.

FanDjango commented 5 months ago

Good - we'll concentrate on the SYNC version first in order to determine more.

FanDjango commented 5 months ago

I have finally found time to run your code.

My initial run was 4000 connects (using FluentFTP.GnuTLS) and I took a snapshot at 100 and at 4000. The run took 30 mins. up to then.

Here is the result:

image

I don't see a leak. I'll let it run for longer, like overnight, see if that show anything up.

FanDjango commented 5 months ago

After many more hours: image 28kB.

FanDjango commented 5 months ago

Looking from the perspective of the OS, in this case Win10: image and later on, 4600 clients opened/closed: image yes, it looks like there is a leak somewhere. Gonna be difficult to find, IMO

validide commented 5 months ago

When running the project in WSL it seemed like it was leaking 3MB each time it would try to connect. I will try to come-up with better "evidence".

FanDjango commented 5 months ago

Yes, that would be nice.

I am running some ultra long interations also whenever I have the opportunity.

Note please, that it is also entirely possible that the called external non-managed library might also have a memory leak itself - since we keep it loaded instead unloading and reloading (big overhead).