IDisposable : Memory leaks

Question

I'm currently do a review of a C# app in which I see that :

public static bool ServiceExists(string servicename)
{
    ServiceController[] services = ServiceController.GetServices();
    foreach (ServiceController s in services)
    {
        if (s.ServiceName == servicename)
        {
            return true;
        }
    }
    return false;
}

In this answer, Henk said that the non-use of Dispose() (or using) will not generate memory leaks, true or false?

Can I keep the previous code has it is or should I write someting like :

public static bool ServiceExists(string servicename)
{
    ServiceController[] services = ServiceController.GetServices();
    bool exists = false;

    foreach (ServiceController s in services)
    {
        using(s)
        {
            if (s.ServiceName == servicename)
            {
                exists = true;
            }
        }
    }

    return exists;
}

What are the risks to no using Dispose() (or using) in this case?

Wrong example. This method does not _own_ th services. It shouldn't Dispose() them. And the 2nd version only disposes _some_ of them. — H H, Nov 07 '11 at 16:50
@HenkHolterman: I edited the 2nd version including your remark. On what can I base me to know if yes or no I should call `Dispose()`? — Arnaud F., Nov 07 '11 at 16:59
Only when the GetServices() method clearly documents that the responsibility shifts to the caller. Your version could very well result in runtime errors. — H H, Nov 07 '11 at 17:05
The object that OWNS the ServiceControllers should be the one to initiate Dispose on them. If GetServices() is creating new instances of ServiceControllers, then you should initiate Dispose in this method. Otherwise, the Dispose method should be called by the object that 'owns' these ServiceControllers. — docmanhattan, Nov 07 '11 at 17:06
Looking at the output of ILDASM on System.ServiceProcess.dll, it looks like it is necessary to Dispose() ServiceControllers returned by GetServices(). In GetServices, they call the ServiceController's constructor, passing it native resources. It might be worth noting that the MSDN sample code does not call Dispose(); https://msdn.microsoft.com/en-us/library/hde9d63a%28v=vs.100%29.aspx?cs-save-lang=1&cs-lang=csharp#code-snippet-2 , this seems like a mistake on Microsoft's part and I hope they will clarify or update their docs. — jrh, Aug 23 '16 at 17:58

JaredPar · Accepted Answer · 2011-11-07T16:40:08.157

A correctly written object shouldn't cause a memory leak if it's Dispose method isn't called. These days a .Net object which controls unmanaged resources should be doing so via a SafeHandle instance. These will ensure the native memory is freed even if Dispose is not called.

However it's very possible for objects which aren't written correctly to produce memory leaks if Dispose isn't called. I've seen many examples of such objects.

In general though if you are using an IDisposable instance which you own you should always call Dispose. Even if the object is correctly written it's to your benefit that unmanaged resources get cleaned up earlier instead of later.

EDIT

As James pointed out in the comments there is one case where not calling Dispose could cause a memory leak. Some objects use the Dispose callback to unhook from long lived events which if they stayed attached to would cause the object to reside in memory and constitute a leak. Yet another reason to always call Dispose

What about the case of the event handler which is unhooked in Dispose? — James L, Nov 07 '11 at 16:36

score 2 · Answer 2 · answered Nov 07 '11 at 16:44

It all depends on what ServiceController.GetServices() is doing. If it is creating new instances of ServiceControllers when it is called, then it could cause a memory leak depending on what it needs to do (the ServiceController) in its Dispose method.

That said, adding 'using' in this case wouldn't fix it anyway as if you DID need to call dispose (implicitly via 'using' in this case) on each instance, it WOULDN'T as it returns whenever it finds a ServiceController with a matching name.

Therefore, if your first iteration found a matching ServiceController, all the other ServiceControllers wouldn't get disposed of anyway.

IDisposable : Memory leaks

2 Answers2