What’s wrong with the following code? How does it leak the connection? There’s a
Close() right there!
Can you see where?
If the code inside the
try block throws, then the call to
be skipped as execution jumps down to the
This method could easily fail if the row was missing:
null and we’d get a null reference exception trying to update the
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.
I’ve also come across code that looks like this:
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.
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.
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.
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:
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.
It is almost always an error to open a SqlConnection outside of a
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: