Why do I need to create a property to check if a resource has been disposed when using disposable pastern in c#?

Question

I need to be write a class where I want to enable the consumer to dispose the code by wrapping the code with using(...) statement in C#.

To do that, I must implement Microsoft's IDisposable interface.

Based on Microsoft approach on implementing it, I should do something like this

a generic interface that so

public interface ISomeClass : IDisposable
{
     // ... some methods to include
}

public class SomeClass : ISomeClass
{
     private readonly TimeTrackerContext _context;

    private bool disposed = false;

    protected virtual void Dispose(bool disposing)
    {
        if (!this.disposed && disposing && _context != null)
        {
            _context.Dispose();

        }
        this.disposed = true;
    }

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

I am trying to learn C# the correct way so I have some question regarding this implementation.

Question

Why do I really need to have a property that tell me if the object has been disposed or not before I dispose it?

In other words, Can't I just check if the _context is null instead before disposing it? Something like this

public class SomeClass : ISomeClass
{
    private readonly TimeTrackerContext _context;

    private void SelfDisposing()
    {
        if (_context != null)
        {
            _context.Dispose();
        }

    }

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

    private void SelfDisposing(bool disposing)
    {
        if (_context != null && !this.disposed && disposing)
        {
            _context.Dispose();
        }

        this.disposed = true;
    }
}

You can, but you don't set context to null after disposing it, so when it will be null then? — Evk, Oct 10 '16 at 18:02
wouldn't dispose do that since the entire object is no longer in the memory? — Junior, Oct 10 '16 at 18:04
no, Dispose does what you tell it to do. You don't set context to null, so it won't be null. — Evk, Oct 10 '16 at 18:05
Please read [MSDN - Dispose Pattern](https://msdn.microsoft.com/en-us/library/b1yfkh5e(v=vs.110).aspx) and [MSDN - Implementing a Dispose Method](https://msdn.microsoft.com/en-us/library/fs2xkftw.aspx). Especially the first link can help you avoid common pitfalls. — Igor, Oct 10 '16 at 18:10
@MikeA The context *is* still in memory. Disposing of unmanaged resources has no effect on managed memory. — Servy, Oct 10 '16 at 18:10
Recommend reading for anyone writing disposable classes. "[IDisposable: What Your Mother Never Told You About Resource Deallocation](http://www.codeproject.com/Articles/29534/IDisposable-What-Your-Mother-Never-Told-You-About)". I had been programming for years and still learned some things. — Scott Chamberlain, Oct 10 '16 at 18:11

score 6 · Accepted Answer · answered Oct 10 '16 at 18:03

6

_context won't be null if the object has already been disposed. It'll still reference the already disposed object.

Now if you can determine whether or not your object is disposed without needing to store a boolean variable, then that's fine.

What you shouldn't actually have is GC.SuppressFinalize Your object has no finalizer, so there is nothing to suppress.

answered Oct 10 '16 at 18:03

Servy

202,030
26
332
449

That explains it! regarding the `GC.SuppressFinalize` Microsoft has the folling in the docs `✓ DO implement the Basic Dispose Pattern and provide a finalizer on types holding resources that need to be freed explicitly and that do not have finalizers.` – Junior Oct 10 '16 at 18:07
2

@MikeA `on types holding resources that need to be freed explicitly` You aren't holding onto any resources that need to be freed explicitly, so you have no need for a finalizer (and don't have one anyway). In all likelihood you'll never need to write a finalizer in your life, even though you'll need to create disposable objects from time to time, you should pretty much only ever need to do so because you're encapsulating other disposable objects. – Servy Oct 10 '16 at 18:12
@MikeA See [this answer](http://stackoverflow.com/a/2605502/2779530). The Microsoft Dispose pattern is provided and meant to be robust in situations where anyone can derive from your class and provide a finalizer themselves. In situations like that it's okay to have the call to `GC.SuppressFinalize` in the base class even though it doesn't have a finalizer itself. You almost surely don't need this (and should probably make your class `sealed` so you don't have to worry about inheritance at all). – Kyle Oct 10 '16 at 18:18
So if you encapsulate other disposable objects you never need to write finalizer, because those objects should have their own finalizer if necessary? – Evk Oct 10 '16 at 18:25

score 1 · Answer 2 · answered Oct 10 '16 at 18:13

1

The field (or property) indicating that the Dispose has been already called is not strongly required for implementing the Dispose(bool) method itself, as mentioned in the following rule:

✓ DO allow the Dispose(bool) method to be called more than once. The method might choose to do nothing after the first call.

It's needed if you have other methods / properties and you want to implement the following rule from the same document:

✓ DO throw an ObjectDisposedException from any member that cannot be used after the object has been disposed of.

answered Oct 10 '16 at 18:13

Ivan Stoev

195,425
15
312
343

+1 Thank you for this info. Why do they use `virtual` method instead of `override` here? Based on my understanding `virtual` will allow other to yet override this method, in my case should that be `override` instead? – Junior Oct 10 '16 at 18:17
1

They use `virtual` because the class in example is the one implementing the pattern. `override` would be used in classes that derive (inherit) from that class (if any). – Ivan Stoev Oct 10 '16 at 18:21

oldovets · Answer 3 · 2016-10-10T21:01:18.540

I would like to share my thoughts on dispose pattern. From my experience, you need to use the pattern in situation, when a class contains an unmanaged resource, or the class is designed to be inherited (even if it does not have an unmanaged resource just to provide well-known logic for the programmers who will implement derived classes). In most cases it seems like you write a bunch of useless code. So in most cases what you need to do is:

public sealed class SomeClass : ISomeClass
{
    private readonly TimeTrackerContext _context;

    public void Dispose()
    {
        _context.Dispose();
    }
}

The second thought is disposing the _context field itself. In case if you pass context via constructor you actually do not know if you really need to dispose it. .NET Stream-based classes (like StreamWriter) also have been suffering from this problem till .NET 4.0. And the solution was to write Stream wrappers (like NonDisposableStreamWrapper) which do not dispose internal stream when dispose is called. The really good solution to this problem is to add additional dispose field in the constructor, which specifies whether to dispose the internal class or not. For example:

public sealed class SomeClass : ISomeClass
{
    private readonly TimeTrackerContext _context;
    private bool _dispose;

    public SomeClass(TimeTrackerContext context, bool dispose = true)
    {
        _context = context;
        _dispose = dispose;
    }

    public void Dispose()
    {
        if (_dispose)
        {
            _context.Dispose();
        }
    }
}

Why do I need to create a property to check if a resource has been disposed when using disposable pastern in c#?

3 Answers3