CODEWITHSUNDEEP
c#disposeidisposable
Robert Harvey
Robert Harvey

Reputation: 180787

Disposing objects in the Destructor

I have an object that has a disposable object as a member.

public class MyClass
{
    private MyDisposableMember member;

    public DoSomething
    {
         using (member = new MyDisposableMember())
         {
             // Blah...
         }
    }
}

There can be many methods in MyClass, all requiring a using statement. But what if I did this instead?

public class MyClass
{
    private MyDisposableMember member = new MyDisposableMember();

    public DoSomething
    {
         // Do things with member :)
    }

    ~MyClass()
    {
        member.Dispose();
    }
}

As you can see, member is being disposed in the destructor. Would this work? Are there any problems with this approach?

Upvotes: 5

Views: 3197

Answers (7)

Reed Copsey
Reed Copsey

Reputation: 564323

Ideally, Dispose() should have already been called prior to finalization. It would be better to follow the typical dispose pattern, and allow the user to Dispose() the object properly, and have the finalizer Dispose of it if dispose has not already been called.

In this case, since you're encapsulating an IDisposable, you really don't need to implement the finalizer at all, though. (At the the point of finalization, your encapsulated member will get finalized, so there's no need to finalize your object - it just adds overhead.) For details, read this blog article I wrote on encapsulating an IDisposable.

Upvotes: 11

supercat
supercat

Reputation: 81115

When a finalizer runs, one of the following will be true about almost any IDisposable object to which it holds a reference:

  1. The object will have already had its finalizer run, in which case calling Dispose on the object will be at best useless.

  2. The object will not have had its finalizer run, but its finalizer will be scheduled to run, so calling Dispose on the object will be useless.

  3. The object will still be in use by something other than the object being finalized, so calling Dispose on it would be bad.

There are a few situations where calling Dispose in a finalizer might be useful, but most situations fit the cases listed above, which all have a common feature: the finalizer shouldn't call Dispose.

Upvotes: 0

Rune FS
Rune FS

Reputation: 21742

In your first example the member is not really part of the object's state since you're instantiating it every time it's used and disposing it right after. Since it's not part of the state don't model it as such just use a local variable when needed.

In more general you should put all disposal logic in Dispose() and implement IDisposable then use you class together with using or try-finally

Upvotes: 1

ChaosPandion
ChaosPandion

Reputation: 78242

Following the Microsoft pattern is your best bet so the users of your class have full control over when it is disposed. 

public class MyClass : IDisposable
{
    private MyDisposableMember member = new MyDisposableMember();

    public DoSomething
    {
         // Do things with member :)
    }

    ~MyClass()
    {
        Dispose(false);
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)  // Release managed resources
        {           
            member.Dispose();
        }

        // Release unmanaged resources
    }
}

Upvotes: 0

Matthias
Matthias

Reputation: 12259

DO NOT DO THAT!

The GC will do that for you (indirectly as the object to dispose or another one will contain a destructor)

MyDisposableMember might even be disposed by the GC even before you dispose it - what happens then might not be what you intended to do.

Even worse: Adding a destructor (or finalizer) to a class costs additional time when disposing of the object (much more time as the object will stay in memory for at least one collection cyclus and maybe even promoted to the next generation).

Therfore, it would be completely useless and even backfire.

Upvotes: 3

JonH
JonH

Reputation: 33141

The only thing I see wrong (and it isn't an error) is the fact that in a using statement you explicitly dispose of the object at that point in time (when your function / method is called). The destructor cannot be called they are invoked automatically. So at this point it may take some time for member to be disposed of. Better to implement the IDisposeable interface for MyClass.

Upvotes: 0

recursive
recursive

Reputation: 86064

You should probably make MyClass implement IDisposable. Inside the Dispose() method, call member.Dispose();. That way the programmer can have control over when the member gets disposed.

Upvotes: 8

Related Questions