Why are my connections not closed even if I explicitly dispose of the DataContext?

Question

I encapsulate my linq to sql calls in a repository class which is instantiated in the constructor of my overloaded controller. The constructor of my repository class creates the data context so that for the life of the page load, only one data context is used.

In my destructor of the repository class I explicitly call the dispose of the DataContext though I do not believe this is necessary.

Using performance monitor, if I watch my User Connections count and repeatedly load a page, the number increases once per page load. Connections do not get closed or reused (for about 20 minutes).

I tried putting Pooling=false in my config to see if this had any effect but it did not. In any case with pooling I wouldn't expect a new connection for every load, I would expect it to reuse connections.

I've tried putting a break point in the destructor to make sure the dispose is being hit and sure enough it is. So what's happening?

Some code to illustrate what I said above:

The controller:

public class MyController : Controller
{
    protected MyRepository rep;

    public MyController ()
    {
        rep = new MyRepository();
    }
}

The repository:

public class MyRepository
{
    protected MyDataContext dc;

    public MyRepository()
    {
        dc = getDC();
    }

    ~MyRepository()
    {
        if (dc != null)
        {
            //if (dc.Connection.State != System.Data.ConnectionState.Closed)
            //{
            //    dc.Connection.Close();
            //}
            dc.Dispose();
        }
    }

    // etc
}

Note: I add a number of hints and context information to the DC for auditing purposes. This is essentially why I want one connection per page load

Update: After having implemented IDisposable on my repository and on my controller class I couldn't find a way to specifically call the Dispose method on my controller as the controller is created and destroyed behind the scenes by the MvcHandler. However I did find that my connections were being closed anyway. I wasn't comfortable knowing that this was working but not knowing why so I did some digging and found an MSDN quote that made me happy:

When execution is complete, the MvcHandler will check if the controller implements the IDisposable interface, and if so, will invoke Dispose on the controller to clean up unmanaged resources.

Final Update: After a month or so with working this I've now removed all this code and gone down the MS advised route of wrapping a "using" statement around the code in my public repository methods and passing this DC into the private methods. This seems a little wasteful and repetitive as well as resulting in a few more connections being opened and closed. But I was getting linq to sql caching that I could only resolve by resetting the DC.

Answer 1

Destructors are only called by the GC. Your MyRepository should implement the Dispose pattern, and dispose of the dc there.

See this question for more detail. http://stackoverflow.com/questions/1076965/in-c-what-is-the-difference-between-a-destructor-and-a-finalize-method-in-a-clas

MyRepository should implement IDisposable, any disposable objects should be disposed of there if you are holding them open for the life time of the object.

Most of the time when you are using a Disposable object you should wrap it in a using block

i.e.

using(var dc = getDC())
{
    //do stuff with the dc
}//the dc will be Disposed here

Edit: Link to Language guide for c# destructors http://msdn.microsoft.com/en-us/library/66x5fx1b(v=VS.100).aspx

Answer 2

The correct pattern (short but sufficient version) here is:

public class MyRepository : IDisposable
{
    ...  // everything except the dtor

    public void Dispose()
    {
        if (dc != null)
        {
            dc.Dispose();
        } 
    }
}

public class MyController : Controller, IDisposable
{
    protected MyRepository rep;

    public MyController ()
    {
        rep = new MyRepository();
    }

    public void Dispose()
    {
       if (rep!= null)
       {
          rep.Dispose();
       } 
    }
}

And now you can (should) use MyController with the using clause:

using (var ctl = new MyController ())
{
   // use ctl
}

Edit:
Just noticed it cascades to MyController, code added. This shows how indirect ownership of an unmanaged resource spreads out.

Edit 2:
This also correct (as would be a try/finally):

var ctl = GetController ();
using (ctl)
{
   // use ctl
}

If you can't keep it local to 1 method, just do your best to call ctl.Dispose() in a Closing event or such.

Answer 3

I agree with the fact that disposable pieces were incorrect and the suggestions above by Henk Holterman and Darryl Braaten are very good ones i don't think it answers your underlying question.

The answer to your question is that calling Dispose on MyRepository (assuming its an DataContext) does not close the connection. It just returns the connection to the pool for next use.

This SO Post, explains when you should worry about closing connections...