SQL Connection Leaks

Lets look at a couple of patterns that can leak SQL Connections:

Try - Catch

What’s wrong with the following code? How does it leak the connection? There’s a call to Close() right there!

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
public void UpdateHitCount(string url)
{
    var conn = GetOpenConnection();
    try
    {
        // Linq to SQL
        using (var context = new MyDataContext(conn))
        {
            var row = context.Hits.FirstOrDefault(h => h.Url == url);
            row.Hits++;
            context.SaveChanges();
        }

        conn.Close()
    }
    catch(Exception e)
    {
        //log it
    }
}

Can you see where?

If the code inside the try block throws, then the call to conn.Close() will be skipped as execution jumps down to the catch block.

This method could easily fail if the row was missing: FirstOrDefault would return null and we’d get a null reference exception trying to update the Hits count.

As an aside, it can also throw a ChangeConflictException, especially in a high load web server.

Imagine two requests enter this method at the same time. Both pull back the current row for same url, and get a hit count of 5. They both increment the hit count to 6, and try to save changes. One will get in first and pass, but the other will detect that its understanding of the world is wrong (it thought the database had 5, but now it also has 6). Then it throws the exception.

Passing Open Connections to Linq2Sql Context

I’ve also come across code that looks like this:

1
2
3
4
5
6
7
8
9
10
public void GetMostViewedPages()
{
    var conn = GetOpenConnection();

    using (var context = new MyDataContext(conn))
    {
        var rows = context.Hits.OrderByDesc(h => h.Hits).Take(10).ToList();
        // do something with them
    }
}

There’s no call to Close() here, but the original developer assumed the Linq2Sql context would take responsibility for the SqlConnection. I mean, normally if you pass the constructor a connection string, it will open and close the connection as needed, right?

But see this warning from MSDN

A DataContext opens and closes a database connection as needed if you provide a closed connection or a connection string. In general, you should never have to call Dispose on a DataContext. If you provide an open connection, the DataContext will not close it. Therefore, do not instantiate a DataContext with an open connection unless you have a good reason to do this. In a System.Transactions transaction, a DataContext will not open or close a connection to avoid promotion.

So, if you pass it a closed connection, it will go ahead and open/close it as needed.

But if you pass it an already open connection, it assumes you know what you’re doing and absolves itself of responsibility for closing it.

This is a bit error prone as its not immediate from just the using statement if the connection will be closed. What kind of connection do we have? If GetOpenConnection() had been named just GetConnection() you’d have to go inspect it to see if what state the returned connections were in.

In Entity Framework, they fixed this by having the corresponding constructor overload also require a boolean flag to indicate if the Db Context should own the connection. This is better because it forces the caller to be explicit about who’s responsibility it is to close the connection when finished.

Why would you do this anyway?

There’s some scenarios where you might want to open a transaction, use Linq2Sql or Entity Framework to do some work, and then run your own custom SQL command as part of that same transaction. In that case, you would need to manage the connection and transaction state yourself. That’s why the libraries provide such an escape hatch.

Is it really that bad?

Won’t the garbage collection get the missed connections? It doesn’t seem like that big of a deal.

Yes, garbage collection will eventually pick up the leftover connections and Dispose() them, but until that happens you are chewing up resources on both the application and the SQL server.

In a high load environment, this can be detrimental to throughput. Requests start queuing as they wait an available connection from the pool. SQL Server starts slowing down because of isolation levels on each open connection. Eventually one or both of them falls over.

Monitoring Connections

The number of free connections in the app server’s connection pool

In red, we have the leaked connections under a simulated high load. See how the number of connections in the pool is really high? The pool had to increase its size to handle requests. The wild swinging is due to leaked connections eventually reclaimed by the garbage collector.

In blue, we have a much more stable environment, and the number of connections in the pool stays below 20.

Though both of these tests were for the same number of requests, the period the test ran is shorter in blue than red, as the throughput of the server was much improved.

There’s a number of useful ADO.NET Performance Counters you can watch with perfmon.

https://msdn.microsoft.com/en-us/library/ms254503.aspx

Note that some of the more fun ones like number of Free or Active connections take a web.config or app.config change to enable:

1
2
3
4
5
6
<system.diagnostics>
  <switches>
    <add name="ConnectionPoolPerformanceCounterDetail"
         value="4"/>
  </switches>
</system.diagnostics>

Evidently they’re costly to record or publish, and probably shouldn’t be enabled in production.

The counter called NumberOfReclaimedConnections would be helpful to watch as well, as it counts the number of connections cleaned up by garbage collection. If this number is not staying relatively constant, you have a leak somewhere.

Stackify Prefix can also show you if a connection might have leaked during a particular request.

How do we fix them?

It is almost always an error to open a SqlConnection outside of a using block.

A common pattern is to get the SqlConnection from a factory method like GetOpenConnection(), but you should always call that factory method from inside the using statement:

1
2
3
4
using(var conn = GetOpenConnection())
{
    ...
}

Comments