Inconsistent Connection Pooling Behavior
-
I'm not sure if anyone will be able to help me with this problem without actually debugging the code, but I thought I'd give it a shot. Here's the situation. I have a fairly mature and stable C#/ASP.NET application that I inherited a few years back. I'm familiar with nearly all of the code, but there are few classes here and there that I've never had to deal with; I know they work and that's it. Late last night we came across a bug in which connections were getting leaked until the pool was saturated with the default 100 connections. It turned out that the connections were getting orphaned in a class that hasn't been touched in about five years. As strange as that is, the part that I really can't explain is that the method is called an several places in the application and it works correctly. However, when it is called in a page that was recently added to the application it leaks connections like a siv. If it can help, here's the offending method (underscores added for legibility).
public int GetUserID(String UserName) { ________// Create Instance of Connection and Command Object ________SqlConnection myConnection = new SqlConnection(ConfigurationSettings.AppSettings["connectionStringUSR"]); ________SqlCommand myCommand = new SqlCommand("GetSingleUserID", myConnection); ________// Mark the Command as a SPROC ________myCommand.CommandType = CommandType.StoredProcedure; ________// Add Parameters to SPROC ________SqlParameter parameterEmail = new SqlParameter("@Email", SqlDbType.NVarChar, 100); ________parameterEmail.Value = UserName; ________myCommand.Parameters.Add(parameterEmail); ________if ((parameterEmail.Value != null) && (parameterEmail.Value != System.DBNull.Value)) ________{ ________________// Open the database connection and execute the command ________________myConnection.Open(); ________________SqlDataReader dr = myCommand.ExecuteReader(CommandBehavior.CloseConnection); ________________if(dr.Read()==true) ________________{ ________________________if(dr.IsDBNull(0)) ________________________________return -1; ________________________else ________________________________return dr.GetInt32(0); ________________} ________________else ________________{ ________________________return -1; ________________} ________} ________else ________{ ________________return -1; ________} }ThemyConnection.Close()is missing, but theCommandBehavioris set toCloseConnectionso it should be getting closed c -
I don't know about you but I don't really trust something to close my connections for me. I'd have the open in a try block and the close in a finally block.
I couldn't agree more, in fact that's precisely what I did to correct the problem. But I also need to explain why the previous code behaved as it did, both for formal justifications as well as my own curiosity.
-
I couldn't agree more, in fact that's precisely what I did to correct the problem. But I also need to explain why the previous code behaved as it did, both for formal justifications as well as my own curiosity.
-
I'm not sure if anyone will be able to help me with this problem without actually debugging the code, but I thought I'd give it a shot. Here's the situation. I have a fairly mature and stable C#/ASP.NET application that I inherited a few years back. I'm familiar with nearly all of the code, but there are few classes here and there that I've never had to deal with; I know they work and that's it. Late last night we came across a bug in which connections were getting leaked until the pool was saturated with the default 100 connections. It turned out that the connections were getting orphaned in a class that hasn't been touched in about five years. As strange as that is, the part that I really can't explain is that the method is called an several places in the application and it works correctly. However, when it is called in a page that was recently added to the application it leaks connections like a siv. If it can help, here's the offending method (underscores added for legibility).
public int GetUserID(String UserName) { ________// Create Instance of Connection and Command Object ________SqlConnection myConnection = new SqlConnection(ConfigurationSettings.AppSettings["connectionStringUSR"]); ________SqlCommand myCommand = new SqlCommand("GetSingleUserID", myConnection); ________// Mark the Command as a SPROC ________myCommand.CommandType = CommandType.StoredProcedure; ________// Add Parameters to SPROC ________SqlParameter parameterEmail = new SqlParameter("@Email", SqlDbType.NVarChar, 100); ________parameterEmail.Value = UserName; ________myCommand.Parameters.Add(parameterEmail); ________if ((parameterEmail.Value != null) && (parameterEmail.Value != System.DBNull.Value)) ________{ ________________// Open the database connection and execute the command ________________myConnection.Open(); ________________SqlDataReader dr = myCommand.ExecuteReader(CommandBehavior.CloseConnection); ________________if(dr.Read()==true) ________________{ ________________________if(dr.IsDBNull(0)) ________________________________return -1; ________________________else ________________________________return dr.GetInt32(0); ________________} ________________else ________________{ ________________________return -1; ________________} ________} ________else ________{ ________________return -1; ________} }ThemyConnection.Close()is missing, but theCommandBehavioris set toCloseConnectionso it should be getting closed cOf course the behaviour depends on what else is happening in the code. You are never closing the data reader, so it will be depending on garbage collections to finalize it. If there doesn't happen to be a garbage collection soon, it will be leaking the connection.
Jason Pease wrote:
it leaks connections like a siv
Perhaps even like a sieve? ;)
Jason Pease wrote:
nderscores added for legibility
So much work for something that didn't end up very good anyway... Use the pre tag instead.
--- It's amazing to see how much work some people will go through just to avoid a little bit of work.
-
Of course the behaviour depends on what else is happening in the code. You are never closing the data reader, so it will be depending on garbage collections to finalize it. If there doesn't happen to be a garbage collection soon, it will be leaking the connection.
Jason Pease wrote:
it leaks connections like a siv
Perhaps even like a sieve? ;)
Jason Pease wrote:
nderscores added for legibility
So much work for something that didn't end up very good anyway... Use the pre tag instead.
--- It's amazing to see how much work some people will go through just to avoid a little bit of work.
Guffa wrote:
Of course the behaviour depends on what else is happening in the code. You are never closing the data reader, so it will be depending on garbage collections to finalize it. If there doesn't happen to be a garbage collection soon, it will be leaking the connection.
The garbage collection doesn't seems to be the issue based on the counters in perfmon, but I'm not sure how reliable they are.
Guffa wrote:
Perhaps even like a sieve?
No, more like a siv. Very very leaky.
Guffa wrote:
So much work for something that didn't end up very good anyway... Use the pre tag instead.
Nice tip. Can you tell I don't post that often? Thanks, Jason
-
Guffa wrote:
Of course the behaviour depends on what else is happening in the code. You are never closing the data reader, so it will be depending on garbage collections to finalize it. If there doesn't happen to be a garbage collection soon, it will be leaking the connection.
The garbage collection doesn't seems to be the issue based on the counters in perfmon, but I'm not sure how reliable they are.
Guffa wrote:
Perhaps even like a sieve?
No, more like a siv. Very very leaky.
Guffa wrote:
So much work for something that didn't end up very good anyway... Use the pre tag instead.
Nice tip. Can you tell I don't post that often? Thanks, Jason
Jason Pease wrote:
The garbage collection doesn't seems to be the issue based on the counters in perfmon, but I'm not sure how reliable they are.
You are missing the point. Read my reply again, and see if you can spot where I say that you are never closing the data reader.
--- It's amazing to see how much work some people will go through just to avoid a little bit of work.
-
Jason Pease wrote:
The garbage collection doesn't seems to be the issue based on the counters in perfmon, but I'm not sure how reliable they are.
You are missing the point. Read my reply again, and see if you can spot where I say that you are never closing the data reader.
--- It's amazing to see how much work some people will go through just to avoid a little bit of work.
Perhaps, you are missing the point. Try to read my initial post again and see you if you can pick out the phrase where I say the code works, and has worked for years, correctly in many parts of the application. If it were simply a question of never closing the data reader it would have leaked connections from day one. Or am I missing something here? --- It's amazing how rude some people are to people that they don't know
-
Perhaps, you are missing the point. Try to read my initial post again and see you if you can pick out the phrase where I say the code works, and has worked for years, correctly in many parts of the application. If it were simply a question of never closing the data reader it would have leaked connections from day one. Or am I missing something here? --- It's amazing how rude some people are to people that they don't know
It could depend on a lot of things as to why connections haven't been leaking up until recently. But it still stands that the connection and the data reader both need closing. I wouldn't bother looking into this too far as you never wrote the code, just fix it and move on.
-
Perhaps, you are missing the point. Try to read my initial post again and see you if you can pick out the phrase where I say the code works, and has worked for years, correctly in many parts of the application. If it were simply a question of never closing the data reader it would have leaked connections from day one. Or am I missing something here? --- It's amazing how rude some people are to people that they don't know
Jason Pease wrote:
Perhaps, you are missing the point.
Nope. :)
Jason Pease wrote:
Try to read my initial post again and see you if you can pick out the phrase where I say the code works, and has worked for years, correctly in many parts of the application.
I saw that when I read it the first time. The fact the code does not produce errors is in no way any guarantee that it is correct.
Jason Pease wrote:
If it were simply a question of never closing the data reader it would have leaked connections from day one.
It's not simply a question of not closing the data reader. As I explained, it depends on whether there are any garbage collections occuring that will finalize the data reader or not. If the code would have closed the data reader, it would not have to depend on the garbage collector. The code has been incorrect from day one. You have just been lucky that the garbage collector has been picking up the slack.
--- It's amazing to see how much work some people will go through just to avoid a little bit of work.
-
I'm not sure if anyone will be able to help me with this problem without actually debugging the code, but I thought I'd give it a shot. Here's the situation. I have a fairly mature and stable C#/ASP.NET application that I inherited a few years back. I'm familiar with nearly all of the code, but there are few classes here and there that I've never had to deal with; I know they work and that's it. Late last night we came across a bug in which connections were getting leaked until the pool was saturated with the default 100 connections. It turned out that the connections were getting orphaned in a class that hasn't been touched in about five years. As strange as that is, the part that I really can't explain is that the method is called an several places in the application and it works correctly. However, when it is called in a page that was recently added to the application it leaks connections like a siv. If it can help, here's the offending method (underscores added for legibility).
public int GetUserID(String UserName) { ________// Create Instance of Connection and Command Object ________SqlConnection myConnection = new SqlConnection(ConfigurationSettings.AppSettings["connectionStringUSR"]); ________SqlCommand myCommand = new SqlCommand("GetSingleUserID", myConnection); ________// Mark the Command as a SPROC ________myCommand.CommandType = CommandType.StoredProcedure; ________// Add Parameters to SPROC ________SqlParameter parameterEmail = new SqlParameter("@Email", SqlDbType.NVarChar, 100); ________parameterEmail.Value = UserName; ________myCommand.Parameters.Add(parameterEmail); ________if ((parameterEmail.Value != null) && (parameterEmail.Value != System.DBNull.Value)) ________{ ________________// Open the database connection and execute the command ________________myConnection.Open(); ________________SqlDataReader dr = myCommand.ExecuteReader(CommandBehavior.CloseConnection); ________________if(dr.Read()==true) ________________{ ________________________if(dr.IsDBNull(0)) ________________________________return -1; ________________________else ________________________________return dr.GetInt32(0); ________________} ________________else ________________{ ________________________return -1; ________________} ________} ________else ________{ ________________return -1; ________} }ThemyConnection.Close()is missing, but theCommandBehavioris set toCloseConnectionso it should be getting closed cJason This is actually quite a subtle one, but the problem is because the connection is closed with CommandBehavior.CloseConnection only when the datareader is closed (which isn't happening in this case). So, you've either got to call dr.Close, or remove the call to CommandBehavior and close the connection explicitly. Hint, try this:
using (SqlConnection myConnection = new SqlConnection(ConfigurationSettings.AppSettings["connectionStringUSR"])) { ... Do the other database work here... }or
SqlDataReader dr = myCommand.ExecuteReader(CommandBehavior.CloseConnection); try { if(dr.Read()==true) { if(dr.IsDBNull(0)) return -1; else return dr.GetInt32(0); } else { return -1; } } finally { dr.Close(); }the last thing I want to see is some pasty-faced geek with skin so pale that it's almost translucent trying to bump parts with a partner - John Simmons / outlaw programmer
Deja View - the feeling that you've seen this post before.