How does this implementation of IDisposable work?

Question

I'm about to build my first n-tier web application, and as such, I'm educating myself on the principles, and looking at example code. I came across a great example of this type of application here: https://github.com/MarlabsInc/SocialGoal However, I'm confused about how the EF Context on the data layer is disposed. Up to this point, I've disposed of my context, either by, for example, utilizing a using block, or overriding the Dispose method on my MVC Controllers. In this application, however, the Dispose method is never explicitly called, instead the factory class that instantiates the context is derived from a base class that implements IDisposable:

Factory class:

public class DatabaseFactory : Disposable, IDatabaseFactory
{
    private SocialGoalEntities dataContext;
    public SocialGoalEntities Get()
    {
        return dataContext ?? (dataContext = new SocialGoalEntities());
    }
    protected override void DisposeCore()
    {
        if (dataContext != null)
            dataContext.Dispose();
    }
}

Disposable class:

public class Disposable : IDisposable
{
    private bool isDisposed;

    ~Disposable()
    {
        Dispose(false);
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
    private void Dispose(bool disposing)
    {
        if (!isDisposed && disposing)
        {
            DisposeCore();
        }

        isDisposed = true;
    }

    protected virtual void DisposeCore()
    {
    }
}   

With a newbie's eyes, this looks like the Context will never be disposed. The only way Dispose(bool) is called, is from the finalizer. And since this passes false as parameter, the virtual DisposeCore() method is never called, and since this has to be called for the Context to dispose, it never will. Am I correct in thinking this, or am missing some key piece of .NET knowledge?

Answer 1

Garbage collector is anyway going to clean or release your managed objects/resources. Dispose for managed objects or resources is needed only when you have urgency to release those resources (for example images). If framework is not calling Dispose method on factory, it is only you who can decide when you want to call that Dispose method (may be on some event, for example, closing of one particular tab.). Then you are sure, you don't need those resources anymore and disposing them would free-up memory.

Answer 2

This parameterless Dispose() method a classic implementation of the IDisposable interface and it is done correctly on your Disposable base class:

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

Dispose() will be called at the end of any using-block, which will subsequently call Dispose(true) and DisposeCore() which, presumably, would be implemented in derived classes to clean up managed resources.

This is not a particularly good piece of code

First of all, this is how the dispose-pattern really should be implemented:

public class MyDisposableThing : IDisposable
{
    public MyDisposableThing()
    {
        // Constructor
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            // Dispose managed resources
        }

        // Dispose unmanaged resources
    }

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

    ~MyDisposableThing()
    {
        Dispose(false);
    }
}

An instance of this type can be destroyed in two ways:

1. Deterministically: Dispose() is called explicitly or by the end of a using-block
2. Garbage Collection: the instance is doomed by the GC and its finalizer is called.

In the first case, you want to release managed resources - that's why you called Dispose(). In the second, you don't because, by definition, they are managed and the GC should handle them just like it is handling your own object. You ALWAYS want to clean up unmanaged resources. This is why the disposing flag exists, along with the associated if-statement.

The call to GC.SuppressFinalize() is an optimization - it tells the garbage collector that the instance has already been cleaned up by Dispose() and its finalizer, ~MyDisposableThing() need not be called again.

Finally, note that Dispose(bool disposing) is virtual and this is vitally important. Derived classes that introduce new resources that need to be handled during disposal can now override it and, after releasing their own resources (managed OR unmanaged) they can call base.Dispose(disposing)

public class MyDerivedDisposableThing : MyDisposableThing
{
    public MyDerivedDisposableThing()
    {
        // Constructor
    }

    protected override void Dispose(bool disposing)
    {
        if (disposing)
        {
            // Dispose managed resources
        }

        // Dispose unmanaged resources

        // Call base class
        base.Dispose(disposing);
    }
}

Contrast this to your code snippets

Firstly, DisposeCore() has no parameters and will not be called from the finalizer (because disposing would be false) so derived classes are prevented from exploiting the whole pattern and cleaning up unmanaged resources.

Secondly, isDisposed is private which makes it entirely unnecessary and useless on the base class since the base class is pretty primitive.

Opinion

Do not create a base Disposable class at all. Just implement the Dispose pattern properly as shown in this answer and also on the MSDN

http://msdn.microsoft.com/en-us/library/fs2xkftw(v=vs.110).aspx

It is a bit of a faff but it is best done properly.