Is it really needed to implement dispose pattern for managed resources only

Question

I've read carefully through this article and it seems to clearly state that the dispose pattern should be implemented in all cases of IDisposable implementation. I'm trying to understand why I need to implement the dispose pattern in cases when my class holds only managed resources (i.e. other IDisposable members or safe handles). Why cannot I just write

class Foo : IDisposable
{
    IDisposable boo;

    void Dispose()
    {
        boo?.Dispose();
    }
}

If it's definitely known that there are no unmanaged resources and there is no point to call Dispose method from finalizer as managed resources aren't being freed from finalizer?

Update: In order to add some clarity. The discussion seems to come down to the question whether it's needed to implement the dispose pattern for every base public non-sealed class which implements IDisposable or not. I however cannot find potential problems with hierarchy when a base class without unmanaged resources doesn't use dispose pattern while child classes which have unmanaged resources do use this pattern:

class Foo : IDisposable
{
    IDisposable boo;

    public virtual void Dispose()
    {
        boo?.Dispose();
    }
}

// child class which holds umanaged resources and implements dispose pattern
class Bar : Foo
{
    bool disposed;
    IntPtr unmanagedResource = IntPtr.Zero;

    ~Bar()
    {
        Dispose(false);
    }

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

    protected virtual void Dispose(bool disposing)
    {
        if (disposed)
            return;

        if (disposing)
        {
            // Free any other managed objects here.
            //
        }
        // close handle

        disposed = true;
    }
}

// another child class which doesn't hold unmanaged resources and merely uses Dispose 
class Far : Foo
{
    private IDisposable anotherDisposable;

    public override void Dispose()
    {
        base.Dispose();
        anotherDisposable?.Dispose();
    }
}

Even more, for me it looks as better separation of concerns when implementations are responsible only for those things which they are aware of.

Answer 1

This

private class Foo : IDisposable
{
    IDisposable boo;

    public void Dispose()
    {
        boo?.Dispose();
    }
}

Is perfectly fine. As is

public sealed class Foo : IDisposable
{
    IDisposable boo;

    public void Dispose()
    {
        boo?.Dispose();
    }
}

What could go wrong if I have public not sealed base class implemented as above with virtual Dispose method ?

From the docs:

Because the order in which the garbage collector destroys managed objects during finalization is not defined, calling this Dispose overload with a value of false prevents the finalizer from trying to release managed resources that may have already been reclaimed.

Accessing a managed object that has already been reclaimed, or accessing its properties after it's been Disposed (perhaps by another finalizer) will cause an Exception to be raised in the Finalizer, which is bad:

If Finalize or an override of Finalize throws an exception, and the runtime is not hosted by an application that overrides the default policy, the runtime terminates the process and no active try/finally blocks or finalizers are executed. This behavior ensures process integrity if the finalizer cannot free or destroy resources.

So if you had:

   public  class Foo : IDisposable
    {
        IDisposable boo;

        public virtual void Dispose()
        {
            boo?.Dispose();
        }
    }
    public class Bar : Foo
    {
        IntPtr unmanagedResource = IntPtr.Zero;
        ~Bar()
        {
            this.Dispose();
        }

        public override void Dispose()
        {
            CloseHandle(unmanagedResource);
            base.Dispose();
        }

        void CloseHandle(IntPtr ptr)
        {
            //whatever
        }
    }

~Bar -> Bar.Dispose() -> base.Dispose() -> boo.Dispose() But boo may have been reclaimed by the GC.

Answer 2

I haven't seen this particular usage of Dispose mentioned yet, so I thought I'd point out a common source of memory leaks when not using the dispose pattern.

Visual Studio 2017 actually complains about this via static code analysis that I should "implement the dispose pattern". Do note I'm using SonarQube and SolarLint, and I don't believe Visual Studio will catch this alone. FxCop (another static code analysis tool) likely will, although I didn't test that.

I note the below code to showcase the dispose pattern is also there to protect against something like this, which has no unmanaged resources:

public class Foo : IDisposable
{
    IDisposable boo;

    public void Dispose()
    {
        boo?.Dispose();
    }
}

public class Bar : Foo
{
    //Memory leak possible here
    public event EventHandler SomeEvent;

    //Also bad code, but will compile
    public void Dispose()
    {
        someEvent = null;
        //Still bad code even with this line
        base.Dispose();
    }
}

The above illustrates very bad code. Don't do this. Why is this horrific code? That's because of this:

Foo foo = new Bar();
//Does NOT call Bar.Dispose()
foo.Dispose();

Let's assume this horrific code was exposed in our public API. Consider the above classes used by consumers of it:

public sealed class UsesFoo : IDisposable
{
    public Foo MyFoo { get; }

    public UsesFoo(Foo foo)
    {
        MyFoo = foo;
    }

    public void Dispose()
    {
        MyFoo?.Dispose();
    }
}

public static class UsesFooFactory
{
    public static UsesFoo Create()
    {
        var bar = new Bar();
        bar.SomeEvent += Bar_SomeEvent;
        return new UsesFoo(bar);
    }

    private static void Bar_SomeEvent(object sender, EventArgs e)
    {
        //Do stuff
    }
}

Are the consumers perfect? No.... UsesFooFactory should probably also unsubscribe from the event. But it does highlight a common scenario where an event subscriber outlives the publisher.

I've seen events cause countless memory leaks. Especially in very large or extreme high performance codebases.

I can also hardly count how many times I've seen objects live long past their time of disposal. This is a very common way many profilers find memory leaks (disposed objects still held onto by a GC Root of some kind).

Again, overly simplified example and horrible code. But it's really never good practice to call Dispose on an object and not expect it to dispose the entire object, whether it has been derived from a million times or not.

Edit

Please note this answer is intentionally only addressing managed resources, showcasing that the dispose pattern is useful in this scenario as well. This purposefully does not address the use case for unmanaged resources as I felt there was a lack of focus on managed only uses. And there are many other good answers on here that talk about that.

I will, however, note a few quick things that are important when it comes to unmanaged resources. The above code might not address unmanaged resources, but I want to make clear it does not contradict how they should be handled.

It's extremely important to use finalizers when your class is responsible for unmanaged resources. Briefly, finalizers are automatically called by the Garbage Collector. So it gives you a reasonable guarantee that it always gets called at some point in time. It's not bulletproof, but a far cry from hoping user code calls Dispose.

This guarantee is not true of Dispose. An object can be reclaimed by the GC without Dispose ever being called. That's the key reason finalizers are used for unmanaged resources. The GC itself only handles managed resources.

But I will also note it's equally important finalizers should not be used to clean up managed resources. There are countless reasons why (it's the GC's job to do this after all) but one of the largest drawbacks to using finalizers is delaying garbage collection on an object.

The GC, seeing an object is free to reclaim but has a finalizer, will delay the collection by placing the object in the finalizer queue. This adds significant unnecessary lifetime to an object, plus more pressure on the GC.

Finally I'll note that finalizers are non-deterministic for this reason, despite having similar syntax to something like a destructor in C++. They are very different beasts. You should never rely upon a finalizer to clean up unmanaged resources at a specific point in time.

Answer 3

You probably got it wrong. You don't need to implement finilizer if you don't have unmanaged resources. You can check it by using automatic pattern implementation in Visual Studio (it will even generate the comments saying that you should uncomment the finalizer ONLY if you use unmanaged resources).

The dispose pattern is used only for objects that access unmanaged resources.

If you design a base class and some of the inheriting classes access unmanaged resources, the inheriting classes will deal with it themselves by overriding Dispose(bool) and defining the finalizer.

It's explained in this article, all finalizers will be called anyway if not suppressed. And if suppressed everything would have been released by the chain of Diapose(true) calls in the first place.

Answer 4

If Dispose() is implemented using a public virtual method, derived classes that expect to override that method can do so and all will be well. If, however, anything on the inheritance chain implements IDisposable.Dispose() via means other than overriding the public virtual Dispose() method, that can make it impossible for a sub-derived class to implement its own IDisposable.Dispose() while still being able to access the parent implementation.

The Dispose(bool) pattern can be used regardless of whether or not there is a public Dispose() method, and thus avoids the need to have separate patterns for the cases where a class does or does not expose a public Dispose() method. The GC.SuppressFinalize(this) could generally be replaced by a GC.KeepAlive(this), but for classes without finalizers the cost is about the same. In the absence of that call, the finalizers for any objects to which a class holds a reference could get triggered while the class' own Dispose method is running. Not a likely scenario, nor one that would usually cause problems even if it occurs, but passing this to GC.KeepAlive(Object) or GC.SuppressFinalize(Object) makes such quirky situations impossible.