Search code examples
c#.net-corestatic-methodsidisposablestatic-classes

Can creating and returning a disposable object from a static class method cause memory leaks?


I've noticed a windows service taking a lot of memory and am trying to get to the bottom of the issue. I noticed that we have a static class containing a static method which is in charge of creating and returning an SftpClient instance, which implements IDisposable. Code for this static class is below.

public static class FTPHelper
{
    public static SftpClient CreateFTPClient(string ftpURL, string username, string password, string keyFilePath)
    {
        var privateKey = !string.IsNullOrEmpty(keyFilePath) ? new PrivateKeyFile(keyFilePath, password) : null;
        var client = !string.IsNullOrEmpty(keyFilePath)? new SftpClient(ftpURL, username, new[] { privateKey }):
                        new SftpClient(ftpURL, 22, username, password);
        return client;
    }
}

We are then calling this method in other parts of the service as below

using var ftpClient = FTPHelper.CreateFTPClient(
                ftpUrl,
                ftpUsername,
                ftpPassword,
                ftpKeyFilePath);

My question is could the above setup be causing a memory leak? As per the last screenshot, the "using" keyword is being used, however I have read that static objects and everything they reference are not collected by the Garbage collector, so is this the case here?


Solution

  • My question is could the above setup be causing a memory leak?

    No, creating objects in a static method is perfectly fine. And disposing objects with using is also perfectly correct.

    however I have read that static objects and everything they reference are not collected by the Garbage collector, so is this the case here

    This applies to static objects, but you only have a static method. Static methods are perfectly fine, at least if they do not use any static fields/objects.

    So this would be bad, for many reasons:

    public static class FTPHelper
    {
        public static SftpClient client; // BAD!
        public static SftpClient CreateFTPClient(string ftpURL, string username, string password, string keyFilePath)
        {
            var privateKey = !string.IsNullOrEmpty(keyFilePath) ? new PrivateKeyFile(keyFilePath, password) : null;
            client = !string.IsNullOrEmpty(keyFilePath)? new SftpClient(ftpURL, username, new[] { privateKey }):
                            new SftpClient(ftpURL, 22, username, password);
            return client;
        }
    }
    

    If you have a memory leak you should use a memory profiler to try figure out what objects are leaking. A common culprit is event registrations that do not unregister, especially if you have any static events. Another common reason is not disposing things that are disposable.

    But your example looks just fine, whatever your problem is it is most likely in some other piece of code.