Arnaud F.
Arnaud F.

Reputation: 8452

IDisposable : Memory leaks

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?

Upvotes: 5

Views: 1269

Answers (2)

docmanhattan
docmanhattan

Reputation: 2416

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.

Upvotes: 2

JaredPar
JaredPar

Reputation: 754505

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

Upvotes: 6

Related Questions