locked
Microsoft Data Access Application Block not closing connections? RRS feed

  • Question

  • User-2044413392 posted
    I have run into a scenario that I have not seen posted here. If I missed something, please let me know. It appears that the code for the ExecuteNonQuery (and ExecuteDataSet for that matter) does not close the connection if you call it using a connection string instead of a Connection object. If I understand the code correctly, the following method (I apologize for the word wrap): ExecuteNonQuery( string connectionString, CommandType commandType, string commandText, params SqlParameter[] commandParameters) creates a connection using the connectionString parameter, then calls: ExecuteNonQuery(connection, commandType, commandText, commandParameters); However, it never closes the connection. Is that correct? Did I miss something? If not, is this a bug or is it working as designed? Do you have to depend on garbage collection if you use SqlHelper with connection strings instead of connections? I have seen the threads regarding the connection leaking using DataReaders (fixed in the sample code for DAAB 2.0, but I have not seen a reference to this issue. Thanks in advance for your assistance. -Weston
    Tuesday, September 16, 2003 4:50 PM

All replies

  • User76536425 posted
    Hi Weston, Do you have it backwards? The DAAB is supposed to close the connection when you pass a string, but not when you pass a connection object. This is because when you pass a string, the DAAB takes control of the entire connection. If you do mean that it isn't getting closed when you use a string, that is a problem. How do you know that it isn't closed? Are you monitoring connections? Or looking at the source code? Don
    Tuesday, September 16, 2003 5:47 PM
  • User-2044413392 posted
    That is what I was trying to communicate. I understood the DAAB to handle opening and closing the connection if you passed it a connection string. The connection is not getting closed. However, I thought it might be a connection pooling issue, so I looked at the code. In the source code for SqlHelper, the method call for ExecuteNonQuery that takes a connection string appears to work as follows: Open connection. Call the ExecuteNonQuery method that takes a connection passing the connection just created. The original method (ExecuteNonQuery with a connectionString) never closes the connection. -Weston
    Tuesday, September 16, 2003 6:01 PM
  • User76536425 posted
    Well, I was hoping that wasn't what you meant! But I just looked at the code for ExecuteNonQuery. That method is heavily overloaded, and all of the versions that take the connection string as the first parameter, as opposed to a connection object, close the connection. It may not do it itself, but calls another version that does it. Working in the source from top to bottom: ExecuteNonQuery(ByVal connectionString As String, _ ByVal commandType As CommandType, _ ByVal commandText As String) As Integer Neither opens nor closes the connection, but calls a version that does. ExecuteNonQuery(ByVal connectionString As String, _ ByVal commandType As CommandType, _ ByVal commandText As String, _ ByVal ParamArray commandParameters() As SqlParameter) As Integer Disposes the connection object in the Finally block. ExecuteNonQuery(ByVal connectionString As String, _ ByVal spName As String, _ ByVal ParamArray parameterValues() As Object) As Integer Calls a version of the method that handles closing the connection. The rest of the overloaded methods either use transactions or take a connection object as a parameter. So, it seems to take care of closing the connections. What am I missing? Don
    Tuesday, September 16, 2003 6:42 PM
  • User-2044413392 posted
    You are exactly right... for the VB version. It appears that the C# version does not close the connections. If you have access to the C# version, can you confirm that I am not crazy (or at least that I am crazy so that I can seek the appropriate professional help ;-) ). -Weston
    Tuesday, September 16, 2003 6:52 PM
  • User76536425 posted
    Ah. Why dindya say so??? :-D Same result, different process. In the C# version, the connection management is handled by the PrepareCommand method. If the method gets a connection that's open, it leaves it open. If not, it closes it when it's done with it. This eliminates the need for managing it in the various Execute methods. Why is this different from the VB.NET version, you ask? Sorry I don't do why questions when they're about Microsoft. :-D I suspect that you could dig into the VB.NET code and figure out why it is that way, but I'll leave that as an exercise for another day. Don
    Tuesday, September 16, 2003 7:26 PM
  • User-2044413392 posted
    I see what you are saying, but that only affects connections that are passed to ExecuteNonQuery, but have not been opened (a scenario that I have not seen implemented). Call the overloaded ExecuteNonQuery method that takes a connection string, a Command.Text, a sql statment string, and a param array. It opens a connection and calls the overloaded ExecuteNonQuery method with the same signature except a SqlConnection substitutes for the connection string. This method calls PrepareCommand. Had a closed connection been passed, then the mustCloseConnection boolean variable would be set to true in PrepareCommand. However, since the connection was already open, mustCloseConnection is set to false and the connection is never closed. The VB version of DAAB also has the mustCloseConnection boolean logic in PrepareCommand and I believe it works in the same manner. -Weston
    Tuesday, September 16, 2003 11:43 PM
  • User76536425 posted
    Correct. I'm not saying it's a good or bad way to do it, just that's how it is. I would hope that further delving into the source would reveal the reason for this design choice, but that will have to wait for another day. Don
    Wednesday, September 17, 2003 1:24 PM
  • User-2044413392 posted
    So, I still have my problem. If I call ExecuteNonQuery with a connection string and a dynamic sql statement, then the connection never gets closed. Right? -Weston
    Wednesday, September 17, 2003 1:59 PM
  • User76536425 posted
    No. Because as near as I can tell all ExecuteNonQuery overloads use the PrepareCommand method, they all create an unopened connection object. And PrepareCommand takes care of closing it. It keeps track of whether it needs to close it through this code: if (connection.State != ConnectionState.Open) { mustCloseConnection = true; connection.Open(); } else { mustCloseConnection = false; }mustCloseConnection is an output parameter, passed back to the ExecuteNonQuery that called PrepareCommand. Then after the query is run ExecuteNonQuery checks whether to close it this way: if( mustCloseConnection ) connection.Close();Assuming that Microsoft carefully implemented this code to take care of all the paths through the various overloaded methods, the connection is closed. If you really want to make sure, use Performance Monitor to watch the connections to the database and verify that you're not left with any open connections. Don
    Wednesday, September 17, 2003 2:41 PM
  • User-2044413392 posted
    In my post on 09-16-2003 at 10:43 PM, I gave a scenario where the overloaded method called PrepareCommand, but did so with an open connection. So, mustCloseConnection is set to false AND the method that opens the connection does not close it. Test function that orphans a connection: private void SampleExecuteNonQuery() { // Pull connection string from web.config for Northwind database string strConn = ConfigurationSettings.AppSettings["NorthwindConnect"]; string strSql = " insert into Northwind" + " (ProductName)" + " values ('New Product')"; SqlHelper.ExecuteNonQuery(strConnectString, CommandType.Text, strSqlStmt); } The test code calls an ExecuteNonQuery with a connection string and a dynamic SQL statement. A null parameter list is added and then the following method is called: public static int ExecuteNonQuery(string connectionString, CommandType commandType, string commandText, params SqlParameter[] commandParameters) { if( connectionString == null || connectionString.Length == 0 ) throw new ArgumentNullException( "connectionString" ); // Create & open a SqlConnection, and dispose of it after we are done using (SqlConnection connection = new SqlConnection(connectionString)) // This is the connection open that never gets closed. - Comment added by Weston Binford connection.Open(); // Call the overload that takes a connection in place of the connection string return ExecuteNonQuery(connection, commandType, commandText, commandParameters); } } Here, the connection is opened, then another ExecuteNonQuery method is called with the newly created connection object. When I traced through this execution path, I eventually called PrepareCommand, BUT mustCloseConnection was set to false because the method that called it already had an open connection. Thus, I still think it is a bug. In the VB version, the ExecuteNonQuery method shown above opens the connection in a try block and closes it in the finally block. The VB version has the same mustCloseConnection logic in the PrepareCommand, but also handles the scenario if you pass it a connection string. Please forgive me if the code above is not exactly correct. I ran the test at home last night and recreated the test stub from memory. I don't have the code available in my current evironment to make sure that I remembered correctly. -Weston
    Wednesday, September 17, 2003 3:45 PM
  • User76536425 posted
    Wow. I finally get it. Yep, you found a path through it that keeps the connection open. I'd call that a bug too. Interesting! And a bummer.... Don
    Wednesday, September 17, 2003 7:09 PM
  • User-2044413392 posted
    Okay. It is good to get confirmation. I was worried that I was missing something simple. Actually, this has really helped me understand how the DAAB is built. This is a problem that affects almost every set of methods that use a ConnectionString. However, I believe it only affects the C# version of the application. The problem is very broad in scope, but appears to be relatively easy to fix in one of two ways. Let me restate the problem: While using the C# version of the DAAB, if you call any of the following sets of methods that accept a ConnectionString, then the connection will not be closed by the program: ExecuteNonQuery ExecuteDataSet ExecuteScalar FillDataSet The ExecuteDataReader methods handle connection management completely differently and the ExecuteXmlDataReader does not have any methods that accept a ConnectionString as a parameter because it does not a method equivalent to cmd.ExecuteDataReader(CommandBehavior.CloseConnection). All of the methods except the FillDataSet methods funnel the calls to a single method that in turn calls an overloaded method passing a connection object in place of the ConnectionString. For some reason, each of the FillDataSet methods that accept a ConnectionString open a new connection and pass it to the associated FillDataSet method that accepts a connection object. In any event, the following methods open a connection that never gets closed: public static int ExecuteNonQuery(string connectionString, CommandType commandType, string commandText, params SqlParameter[] commandParameters) public static DataSet ExecuteDataset(string connectionString, CommandType commandType, string commandText, params SqlParameter[] commandParameters) public static object ExecuteScalar(string connectionString, CommandType commandType, string commandText, params SqlParameter[] commandParameters) public static void FillDataset(string connectionString, CommandType commandType, string commandText, DataSet dataSet, string[] tableNames) public static void FillDataset(string connectionString, CommandType commandType, string commandText, DataSet dataSet, string[] tableNames, params SqlParameter[] commandParameters) public static void FillDataset(string connectionString, string spName, DataSet dataSet, string[] tableNames, params object[] parameterValues) Using the first method as an example, the connection that is created and opened will never be closed. See here: public static int ExecuteNonQuery(string connectionString, CommandType commandType, string commandText, params SqlParameter[] commandParameters) { if( connectionString == null || connectionString.Length == 0 ) throw new ArgumentNullException( "connectionString" ); // Create & open a SqlConnection, and dispose of it after we are done using (SqlConnection connection = new SqlConnection(connectionString)) { <color="green">// Comment added by Weston Binford // There is not a close associated with this open</color> connection.Open(); // Call the overload that takes a connection in place of the connection string return ExecuteNonQuery(connection, commandType, commandText, commandParameters); } } I see two solutions to the problem: The first solution is to wrap the body of the method in a try finally block. This is how it is done in the VB version. So, with added code in red, the new method call would be: public static int ExecuteNonQuery(string connectionString, CommandType commandType, string commandText, params SqlParameter[] commandParameters) { if( connectionString == null || connectionString.Length == 0 ) throw new ArgumentNullException( "connectionString" ); <color="red">SqlConnection connection = null; try {</color> // Create & open a SqlConnection, and dispose of it after we are done using (connection = new SqlConnection(connectionString)) { connection.Open(); // Call the overload that takes a connection in place of the connection string return ExecuteNonQuery(connection, commandType, commandText, commandParameters); } <color="red">} finally { if (connection != null) connection.Dispose(); }</color> } The second and somewhat intriguing solution comes from the implementation of GetSpParameterSet. It creates a connection, but does not open it. The connection is eventually opened and closed by DiscoverSpParameterSet. Using the same technique, the connection.Open() line would be removed from all of the methods identified above. Then, I believe the PrepareCommand method would set the mustCloseConnection boolean variable correctly and we could depend on the ExecuteNonQuery method that accepts a connection object to close the connection. Assuming it works, this would be the simplest fix, but would cause a divergence between the VB and C# versions of the code. I have not debugged either solution. The first solution compiles, but I have not traced through the code. I thought I would get some feedback on this issue before I fixed it in my version of the DAAB. What do you think? Do you have any comments on either solution or have another solution? Anybody still with me? -Weston
    Wednesday, September 17, 2003 11:48 PM
  • User-14942096 posted
    if i'm not mistaking; the using statement automatically closes the connection
    Thursday, September 18, 2003 7:37 AM
  • User-2044413392 posted
    You are exactly right. I feel really stupid. For some reason, I thought that using only saved on typing. It also acts as a scope operator and calls the Dispose() method when it reaches the close brace. Nevermind. I agree it is working. Sorry for wasting your time. -Weston
    Thursday, September 18, 2003 10:31 AM
  • User76536425 posted
    D'oh! I can't believe I missed that too! I've been doing too much VB.NET lately! Thanks, klenne! Don
    Thursday, September 18, 2003 12:25 PM
  • User1522919459 posted
    I use the SqlHelper class a fair amount. I'm using along the lines of

    ds = SqlHelper.ExecuteDataset(ConfigurationManager.ConnectionStrings["ConnectionString"].ToString(), CommandType.Text, mystring, param);

     

    now the strange thing is, when I run a netstat in the command prompt, I get quite a few connections to sql popping up in "TIME_WAIT"

     

    any reason why this should be happening? or am I just thinking of functionality that isn't supposed to be there?

     

    EDIT:

    sorry, forgot to mention that the amount of connections increases as I carry on moving through pages.

    Tuesday, January 8, 2008 3:17 AM
  • User1434570679 posted

    hello,

    I have the same problem about closing connections but I'm using

    public static SqlDataReader ExecuteReader(string connectionString, CommandType commandType, string commandText)

    I see that it not close the connection and when I call it many times the SQL pool connections increase a lot. If I tray to fix it with using (SqlConnection cn = new SqlConnection(connectionString)) or with cn.Close(), the application that use SqlHelper generates error because the SqlDataReader object has the connection closed.

    I had to workaround with the use of DataTableReader object .

    Original Code:

    public static SqlDataReader ExecuteReader(string connectionString, CommandType commandType, string commandText, params SqlParameter[] commandParameters)

    {

    //create & open a SqlConnection

    SqlConnection cn = new SqlConnection(connectionString);

    cn.Open();

    try

    {

    //call the private overload that takes an internally owned connection in place of the connection string

    return ExecuteReader(cn, null, commandType, commandText, commandParameters,SqlConnectionOwnership.Internal);

    }

    catch

    {

    //if we fail to return the SqlDatReader, we need to close the connection ourselves

    cn.Close();

    throw;

    }

    }

    Code that does not work

    public static SqlDataReader ExecuteReader(string connectionString, CommandType commandType, string commandText, params SqlParameter[] commandParameters)

    {

    using (SqlConnection cn = new SqlConnection(connectionString))

    {

    cn.Open();

    //call the private overload that takes an internally owned connection in place of the connection string

    return ExecuteReader(cn, null, commandType, commandText, commandParameters, SqlConnectionOwnership.Internal);

    }

    }

    I have VS2005, SQL 2000 and SQL 2005 and framework 2.0.

    Regards,

    Roberto

    Friday, May 30, 2008 9:54 AM
  • User1434570679 posted

    Hello,

    I find the problem and it is because we needed to explicitly close the database connections when we were done using SqlDataReader. In teory, DAL has to close the connections, but SQLHelper does not do it. I do not want to worry about if I using BLL and DAL.

    You can read http://www.aspdotnetcodes.com/Ado.Net_DataTableReader.aspx.

    Best regards,

    Roberto

    Tuesday, June 3, 2008 6:56 PM