Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Personally, I break out in hives if I don't put ADO objects that implement IDisposable in using statements. But at my current contract, I've found that their homegrown enterprise framework "data access provider" code does not 1) implement IDisposable and 2) call Dispose() on anything it uses, at any point, ever. Users have been complaining a great deal about performance issues in the Winforms applications that heavily use this framework for data access, and though there are a LOT of other problems in the code that could be hitting performance, this one just screams at me and is more low-hanging-fruit than the others.

So, beyond saying something like "Dispose is there for a reason, use it," what can I tell these people to convince them that this is really, really bad?

share|improve this question
2  
Heh..my guess is that, at some point, convincing will not be necessary :) –  dr Hannibal Lecter Nov 9 '10 at 19:58

5 Answers 5

up vote 5 down vote accepted

I'd say the best thing you can do is point them to Microsoft's Patterns and Practices regarding Dispose() here. Let them see the full consequences of not utilizing this tool that's right in front of them.

share|improve this answer
    
Excellent link. Thanks for that. –  AJ Johnson Nov 9 '10 at 20:12
1  
Just wish I could find a version that didn't scream "this is retired content" at the top. –  AJ Johnson Nov 9 '10 at 20:19
    
You know, my eyes just glossed over that. But now reading it carefully, I wonder what technologies that people might "still be using" that are retired from that page. Maybe CAS? –  Jesse C. Slicer Nov 9 '10 at 20:24
    
Scanning it more closely, most of this still seems relevant to me. Not sure why it's marked retired either. –  AJ Johnson Nov 9 '10 at 21:36

If you do not call the Dispose method on a SQL Connection, when you done using it, that connection does NOT get returned back to the connection pool.

If you are having performance issues, my guess is that the maximum connections are opened on your database. A DBA could easily confirm this.

Microsoft's Best Practice calls for you to place your connection code inside of a Using statement, ensuring the connection is disposed of and that the connection is returned back to the pool.

share|improve this answer

The database connection pool is limited in size, and if it fills up, new connections will wait for old connections to be released. If you don't dispose them as soon as you're finished with them, they'll eventually get released when the finaliser runs, but that's an indeterminate amount of time in the future... So at best, you're going to see long delays when opening new connections.

At worst, they might've disabled the connection pool. Perhaps they discovered that after a while their app was returning "timeout waiting for connection" errors and disabling the connection pool "solved" that problem. However, this is much worse because it means you're creating a whole new connection every time - which is surprisingly resource intensive. If you've got hundreds of connections to the database open, it would be no wonder you're seeing performance problems.

The correct way to solve all of these problems is to dispose of your connection as soon as you're finished with it. The best idea is to hold the connection open for exactly as long as needed, and no longer.

share|improve this answer

In any serious application I would say that it is pretty bad. Not only will leaving these objects float around kill performance, it is also simply unprofessional. The good news is that Microsoft implements Finalize in the object hierarchy.

~Object()
{
    this.Dispose(false);    
}

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

protected virtual void Dispose(bool disposing)
{
    // ...
}

Take System.Data.SqlClient.SqlConnection for example:

System.ComponentModel.Component <-- Implements the Finalize dispose pattern.
    |
System.Data.Common.DbConnection
    |
System.Data.SqlClient.SqlConnection

The objects will eventually be disposed but the non-deterministic nature reaks havoc on performance.

share|improve this answer

Well, for one, you aren't terminating the connection. So it either has to a) Be dropped automatically or b) Get cycled through and updated even though the client has no use for it.

I'd assume b) due to the performance hit you're describing. However, that is likely not the only reason.

You MUST close your connections, preferably on the client side, but you must also implement a fail safe on the server side. Otherwise you've just got extra crap piled on that your database server has to process until it gets released at god-knows-when.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.