Dispose pattern without unmanaged objects

Question

I've used the wellknown Dispose Pattern in .NET several times as described in the official docs.

Now I have the following, rather simplified code:

public class FirstForeignDependency : IDisposable
{
    // all the things from another library that I want to use in my code
}

public class SecondForeignDependency : IDisposable
{
    // all the things from another library that I want to use in my code
}

public class MyParent
{
    private readonly FirstForeignDependency _firstForeignDependency;

    public MyParent()
    {
        _firstForeignDependency = new FirstForeignDependency();
    }
}

public class MyChild : MyParent
{
    private readonly SecondForeignDependency _secondForeignDependency;

    public MyChild() : base()
    {
        _secondForeignDependency = new SecondForeignDependency();
    }
}

As you can see, there are two dependencies FirstForeignDependency and SecondForeignDependency, representing any third-party dependency.
And there are the two types MyParent and MyChild within my codebase consuming the third-party dependencies.

Since I'm instantiating the dependencies, I'm also responsible for disposing them. Applying Microsoft's Dispose pattern would give me something like this:

public class FirstForeignDependency : IDisposable
{
    // all the things from another library that I want to use in my code
}

public class SecondForeignDependency : IDisposable
{
    // all the things from another library that I want to use in my code
}

public class MyParent : IDisposable
{
    private readonly FirstForeignDependency _firstForeignDependency;
    private bool _disposed;

    public MyParent()
    {
        _firstForeignDependency = new FirstForeignDependency();
    }

    ~MyParent()
    {
        Dispose(false);
    }

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

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

        if (disposing)
        {
            _firstForeignDependency?.Dispose();
        }

        _disposed = true;
    }
}

public class MyChild : MyParent
{
    private readonly SecondForeignDependency _secondForeignDependency;
    private bool _disposed;

    public MyChild() : base()
    {
        _secondForeignDependency = new SecondForeignDependency();
    }

    ~MyChild()
    {
        this.Dispose(false);
    }

    protected override void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            if (disposing)
            {
                _secondForeignDependency?.Dispose();
            }

            _disposed = true;
        }

        base.Dispose(disposing);
    }
}

For me this code has some imperfections: the Dispose(bool) overload is intended to free unmanaged resources. Since I do only have managed resources, I think that I can safely skip this overload and the finalizers. So I ended up with this code:

public class FirstForeignDependency : IDisposable
{
    // all the things from another library that I want to use in my code
}

public class SecondForeignDependency : IDisposable
{
    // all the things from another library that I want to use in my code
}

public class MyParent : IDisposable
{
    private readonly FirstForeignDependency _firstForeignDependency;
    private bool _disposed;

    public MyParent()
    {
        _firstForeignDependency = new FirstForeignDependency();
    }

    public virtual void Dispose()
    {
        if (_disposed)
        {
            return;
        }

        _firstForeignDependency?.Dispose();

        _disposed = true;
    }
}

public class MyChild : MyParent
{
    private readonly SecondForeignDependency _secondForeignDependency;
    private bool _disposed;

    public MyChild() : base()
    {
        _secondForeignDependency = new SecondForeignDependency();
    }

    public override void Dispose()
    {
        if (!_disposed)
        {
            _secondForeignDependency?.Dispose();

            _disposed = true;
        }

        base.Dispose(disposing);
    }
}

Despite the fact that It does not adhere 100% to the original Dispose pattern, is there anything wrong with that simplification? Am I missing something with that approach?

Thanks!

Martin Maat · Accepted Answer · 2021-12-07 23:00:58Z

3

Your first approach is already wrong. The parent class implements the dispose pattern. Anything you descend from that must not mess with a finalizer, a Dispose() method or another _disposed flag, that is already in place. In your descending classes you only override Dispose(bool), dispose what you need to from the child class itself there and call base.Dispose(disposing). You must publish _disposed in the base class as a property so you can use it in each child class's Dispose(bool) to make sure you only dispose once.

If you create a class Disposable that does nothing but implement the dispose pattern you can use it as a base class for any class that has to dispose something. Just override Dispose(bool) and be done with it:

protected override void Dispose(bool disposing)
{
    if (!this.IsDisposed)
    {
        if (disposing)
        {
            // do your disposing here
        }

        base.Dispose(disposing);
    }
}

Make sure to call Dispose on all your disposables. Use FxCop to help you with CA2213 warnings.

answered Dec 7, 2021 at 23:00

Martin Maat

18.6k3 gold badges33 silver badges59 bronze badges

1

Why is the first approach wrong? It is documented here, just the second flavor

mu88
– mu88

2021-12-08 07:04:07 +00:00
Commented Dec 8, 2021 at 7:04
@mu88 I am not sure what they are trying to accomplish or clarify with the second example. I think this is for a scenario in which you must descend from an existing class that is not disposable and you have something to dispose. Then you add disposable behavior somewhere in the inheritance hierarchy rather than at the root. But then again I do not see a Dispose() method yet it is called from the finalizer. It is just confusing to me. The main takeaway is the first paragraph that states you should only implement the IDispose plumbing once and just override Dispose(bool) in any descendants.

Martin Maat
– Martin Maat

2021-12-08 12:15:56 +00:00
Commented Dec 8, 2021 at 12:15

Add a comment |

JonasH · Accepted Answer · 2021-12-07 15:30:28Z

Despite the fact that It does not adhere 100% to the original Dispose pattern, is there anything wrong with that simplification? Am I missing something with that approach?

Yes, if at some later time you introduce another derived class that do own a native resource you would need to introduce a finalizer etc to be safe. But now you cannot call Dispose() from the finalizer since that would be unsafe, so you would need to introduce another virtual method to dispose native resources, and that would need to be called from any further derived classes.

Or worse, if you at a later time change the base-class to own a native resource you would need to modify all the derived classes.

When designing classes for inheritance it is a good idea to think ahead as much as possible. Including the full dispose patter should help make the class more future proof and simplify extension in the future. Following a standardized pattern is a fairly simple way to ensure maximum flexibility without having to think thru every use case, and it it is possible to write a macro that simply includes a region with the typical pattern to reduce typing.

An alternative is to make classes sealed to prevent derived classes, this generally makes classes easier to modify since you do not have to worry about derived classes. This is probably a good thing to apply by default, since most classes are not designed to be inherited from, and inheriting from classes not designed for it can lead to various issues.

greenoldman · Accepted Answer · 2021-12-08 06:18:48Z

0

If you don't have unmanaged resources do not use finalizers, and stick only to Dispose. I don't know why Dispose pattern is poorly documented, but if you read more about GC, you will find out, that finalizers slows down GC.

In short you would use finalizers in managed cases only if you are lazy and you would like to GC to handle disposing of your objects (through finalizer).

In short, use only Dispose method (without finalizer) and you will be fine.

answered Dec 8, 2021 at 6:18

greenoldman

1,5331 gold badge14 silver badges29 bronze badges

1

Could you maybe add some links where you've read about the GC thing and the performance implications when using Finalizers? Thanks!

mu88
– mu88

2021-12-08 07:11:22 +00:00
Commented Dec 8, 2021 at 7:11
1

@mu88, no links, I am sorry. I found this info in Pro .NET Memory Management by Konrad Kokosa and FRAMEWORK DESIGN GUIDELINES by Krzysztof Cwalina, et al. Basically those are remarks in the books (but anyway, I highly recommend both books of course).

greenoldman
– greenoldman

2021-12-08 09:53:08 +00:00
Commented Dec 8, 2021 at 9:53
If the slowdown was ever an issue, with the dispose pattern it won't be because it is suppressed (and thus not called) every time. It will only be called if you fail to call Dispose() yourself, which you want to avoid.

Martin Maat
– Martin Maat

2021-12-09 07:43:19 +00:00
Commented Dec 9, 2021 at 7:43
@MartinMaat, it is not that the body of your finalizer slows down anything, it is the fact the finalizer exists. GC has to handle it in special way, and this is a problem (no matter if you call Dispose yourself or not).

greenoldman
– greenoldman

2021-12-09 08:41:28 +00:00
Commented Dec 9, 2021 at 8:41

Add a comment |

Stack Exchange Network

Dispose pattern without unmanaged objects

3 Answers 3

Hot Network Questions

Dispose pattern without unmanaged objects

3 Answers 3

Related

Hot Network Questions