CODEWITHSUNDEEP
c#.netdisposeidisposable
Andrej Adamenko
Andrej Adamenko

Reputation: 1668

Where to call Dispose() of IDisposable created in constructor?

Where to call Dispose() for IDisposable objects owned by an object? 

public class MyClass
{
    public MyClass()
    {
        log = new EventLog { Source = "MyLogSource", Log = "MyLog" };
        FileStream stream = File.Open("MyFile.txt", FileMode.OpenOrCreate);
    }


    private readonly EventLog log;
    private readonly FileStream stream;

    // Other members, using the fields above
}

Should I implement Finalize() for this example? What if I do not implement anything at all? Will there be any problems?

My first thought was that MyClass should implement IDisposable. But the following statement in an MSDN article confused me:

Implement IDisposable only if you are using unmanaged resources directly. If your app simply uses an object that implements IDisposable, don't provide an IDisposable implementation.

Is this statement wrong?

Upvotes: 14

Views: 2927

Answers (3)

supercat
supercat

Reputation: 81169

Much of the advice surrounding Dispose and Finalize was written by people who expected Finalize to be workable as a primary resource cleanup mechanism. Experience has shown such expectation to have been overly optimistic. Public-facing objects which acquire any sort of resources and hold them between method calls should implement IDisposable and should not override Finalize. If an object holds any resources which would not otherwise get cleaned up if it was abandoned, it should encapsulate each such resource in a privately-held object which should then use Finalize to clean up that resource if required.

Note that a class should generally not use Finalize to clean up resources held by other objects. If Finalize runs on an object that holds a reference to the other object, one of several conditions will usually apply:

  • No other reference exists to the other object, and it has already run Finalize, so this object doesn't need to do anything to clean it up.
  • No other reference exists to the other object, and it hasn't yet run Finalize but is scheduled to do so, so this object doesn't need to do anything to clean it up.
  • Something else is still using the other object, so this object shouldn't try to clean it up.
  • The other object's clean-up method cannot be safely run from within the context of a finalizer thread, so this object shouldn't try to clean it up.
  • This object only became eligible to run Finalize because all necessary cleanup has already been accomplished, so this object doesn't need to do anything to clean things up.

Only define a Finalize method in those cases where one can understand why none of the above conditions apply. Although such cases exist, they are rare, and it's better not to have a Finalize method than to have an inappropriate one.

Upvotes: 1

Yuval Itzchakov
Yuval Itzchakov

Reputation: 149538

For objects containing other IDisposable objects, it's a good and recommended practice to implement IDisposable on your own object, so others consuming your type can wrap it in a using statement:

public class MyClass : IDisposable
{
    public MyClass()
    {
        log = new EventLog { Source = "MyLogSource", Log="MyLog" };
        FileStream stream = File.Open("MyFile.txt", FileMode.OpenOrCreate);
    }


    private readonly EventLog log;
    private readonly FileStream stream;

    public void Dispose()
    {
        Dispose(true);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            // Free managed objects here
            stream.Dispose();
        }
    }

    // Other members, using the fields above
}

In your case, you aren't freeing up any managed resources so no finalizer is needed. If you were, then you would implement a finalizer and call Dispose(false), indicating to your dispose method that it is running from the finalizer thread.

If you don't implement IDisposable, you're leaving it up to the GC to clean up the resources (e.g, close the Handle on the FileStream you've opened) once it kicks in for collection. Lets say your MyClass object is eligible for collection and is currently in generation 1. You would be leaving your FileStream handle open until the GC cleans up the resource once it runs. Also, many implementations of Dispose call GC.SuppressFinalize to avoid having your object live another GC cycle, passing from the Initialization Queue to the F-Reachable queue.

Upvotes: 1

Marc Gravell
Marc Gravell

Reputation: 1062875

If MyClass owns an IDisposable resource, then MyClass should itself be IDisposable, and it should dispose the encapsulated resource when Dispose() is called on MyClass:

public class MyClass : IDisposable {
    // ...
    public virtual void Dispose() {
        if(stream != null) {
            stream.Dispose();
            stream = null;
        }
        if(log != null) {
            log.Dispose();
            log = null;
        }
    }
}

No, you should not implement a finalizer here.

Note: an alternative implemention might be something like:

private static void Dispose<T>(ref T obj) where T : class, IDisposable {
    if(obj != null) {
        try { obj.Dispose(); } catch {}
        obj = null;
    }
}

public virtual void Dispose() {
    Dispose(ref stream);
    Dispose(ref log);
}

Upvotes: 27

Related Questions