Connection pool with possible severed connections

Question

I have multiple threads accessing the same database (with same connection string). Each thread:

• creates it's own SqlConnection instance using the same connection string

• uses code below to open it's own connection instance whenever it needs one

      try
      {
          wasOpened = connection.State == ConnectionState.Open;
  
          if (connection.State == ConnectionState.Closed)
          {
              connection.Open();
          }
      }
      catch (Exception ex)
      {
          throw new Exception(string.Format("Connection to data source {0} can not be established! Reason: {1} - complete stack {2}",
                                            connection.Database, ex.Message, ex.StackTrace == null ? "NULL" : ex.StackTrace.ToString()));
      }
  

We have tested this code on 2 servers so far, and one server sometimes throws an exception in SqlConnection.Open method. Here is the exception message we get from catch block:

Connection to data source xyz can not be established! Reason: Invalid operation. The connection is closed. - complete stack

at System.Data.SqlClient.SqlConnection.GetOpenConnection()
at System.Data.SqlClient.SqlConnection.get_Parser()
at System.Data.SqlClient.SqlConnection.Open()

Inspecting SqlConnection.GetOpenConnection method shows that innerConnection is null:

internal SqlInternalConnection GetOpenConnection()
{
    SqlInternalConnection innerConnection = this.InnerConnection as SqlInternalConnection;
    if (innerConnection == null)
    {
        throw ADP.ClosedConnectionError();
    }
    return innerConnection;
}

It stays unclear to me: why does connection pool sometimes give me severed connection (innerConnection == null)?

Edit #1: there are no static properties in code - we ARE always closing connection when appropriate, wasOpened is used in our Close method and means: if connection was already opened when our Open is called, just leave it open on Close, otherwise close it. However, this is not related to problem described in this question (innerConnection == null).

Edit #2: Server: SQL Server 2008 R2, Windows Server 2003. Client: Windows Server 2003 (code runs within SSIS package custom component). Connection string: Data Source=server_name;Initial Catalog=db_name;Integrated Security=SSPI;Application Name=app_name

Doublepark

Why do you need to preserve the old connection state? Close it as soon as your ready with it, best via using-statement. Is something static here? – Jus

@TimSchmelter is correct..you should close connection after accessing it.. – Adila

@TimSchmelter: no, nothing static; we are aware of this bad practice but this is legacy code: this is base class for some old database access objects that support usage of the same connection from multiple database access objects - means: wasOpened is used in base class Close method to leave the connection open if it was not opened in corresponding Open method - that's all. – Straightjacket

@BizApps: please, take a look at my comment to TimSchmelter and Edit #1. – Straightjacket

@FilipPopović: I'm afraid that you need that breaking change. Have a look at my answer on a related question a few days ago, maybe it helps to understand why it's bad practise not to close the connection: #9706137 (i need to go to bed now) – Jus

@TimSchmelter: thanks for link I fully agree with it. Just, I don't see how it relates to my problem - breaking change is not an option at the moment. Connection is created (new SqlConnection(...)) within thread. The exception does not state that someone else is using connection and SqlConnection.GetOpenConnection code shows severed innerConnection object. And I don't even know what sets innerConnection to null, but Close does not, that's for sure because that is connection that connection pool does not really close when you ask it to close connection. – Straightjacket

@FilipPopović: In short: don't poach in the territory of the connection-pool ;) Edit: They are related because i assume that no connection is available in the pool since all are in use(not closed from you). – Jus

@TimSchmelter: good point, voted up! :) and have a nice sleep , you don't have connection pool problems after all :) – Straightjacket

@TimSchmelter: I am sure that I closed it because of finally blocks with close everywhere (reviewed it many times so far). when connection pool is full, error is "Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached". However, I will recheck connection pool state tomorrow and get back here with info. – Straightjacket

Answer 1

First, read this carefully: SQL Server Connection Pooling (ADO.NET)

I'll quote the most important part for you (i think):

A connection pool is created for each unique connection string. When a pool is created, multiple connection objects are created and added to the pool so that the minimum pool size requirement is satisfied. Connections are added to the pool as needed, up to the maximum pool size specified (100 is the default). Connections are released back into the pool when they are closed or disposed.

When a SqlConnection object is requested, it is obtained from the pool if a usable connection is available. To be usable, a connection must be unused, have a matching transaction context or be unassociated with any transaction context, and have a valid link to the server.

The connection pooler satisfies requests for connections by reallocating connections as they are released back into the pool. If the maximum pool size has been reached and no usable connection is available, the request is queued. The pooler then tries to reclaim any connections until the time-out is reached (the default is 15 seconds). If the pooler cannot satisfy the request before the connection times out, an exception is thrown.

We strongly recommend that you always close the connection when you are finished using it so that the connection will be returned to the pool. You can do this using either the Close or Dispose methods of the Connection object, or by opening all connections inside a using statement in C#, or a Using statement in Visual Basic. Connections that are not explicitly closed might not be added or returned to the pool. For more information, see using Statement (C# Reference)

In short: don't poach in the territory of the connection-pool and close connections as soon as you're finished with them(f.e. via using-statement).

Since you don't want to throw your DB-Class into the garbage can, i would suggest to either increase the maximum poolsize and/or the timeout or disable pooling and see what happens.

<add name="theConnectionString" connectionString="Data Source=(local);
     Database=AdventureWorks; Integrated Security=SSPI; 
     Max Pool Size=200; Pooling=True; Timout=60" />

You should also try to catch this specific error and clear all connection pools:

System.Data.SqlClient.SqlConnection.ClearAllPools();

Or have a look at these questions which look promising:

• SQL Server connection pool doesn't detect closed connections?
• http://forums.asp.net/t/1729521.aspx/1

Jus

+1 for very detailed answer and a lot of good references. You might be interested to read my answer, also. – Straightjacket

forgot to mention: I tested number of connections as you advised in comments to question, and the number of connections on server during multiple executions did not exceed 24. – Straightjacket

Answer 2

I have multiple threads accessing the same database (with same connection string). Each thread:

1. creates it's own SqlConnection instance using the same connection string
2. uses code below to open it's own connection instance whenever it needs one

If you have a problem that appears at random, in your case, given the code you have displayed, you might have:

1. A problem with the connection pooling on the server
2. A race condition somewhere in your code

All that being said...

You should wrap your SqlConnection in a using statement. This way the connection will be closed when your code, or thread, is done with it.

using (SqlConnection connection = new SqlConnection(connectionString))
{
   //... stuff
}

This way the connection is guaranteed to call the Dispose() method (which internally calls Close() anyways). That way the connection can be returned to the pool, if it is in use (which it probably is).

Merwyn

I agree that that your way is the way to go in general, but please provide additional explanation on how this relates to innerConnection == null problem described in question? – Straightjacket

@FilipPopović: Why are you checking if the connection is open in the first place? Doing so suggests that you aren't using using statement somewhere in your code... – Merwyn

Also, would you mind posting your connection string and any more specifics about the database? – Merwyn

Well... it is legacy code, and each method has try open (as seen in answer), finally close. And close leaves connection open if it was opened when Open was called. Simply to allow different instances (not static code) to use this same connection and transaction if applicable. By examining code I see no way to make SqlConnection.InnerConnection be set to null. And from SqlConnection.GetOpenConnection it is clear that it is set to null somehow. So, whether it is open or not, it doesn't matter. It fails on SqlConnection.Open with message Invalid operation. Connection is closed. – Straightjacket

And sure it is closed, that's why I want to open it! :) – Straightjacket

Didn't see your comment about database info at first. Take a look at Edit #2. – Straightjacket

+1 for reasoning and advices. You might be interested to read my answer, also. – Straightjacket

Answer 3

Answers provided by Tim Schmelter and Bryan Crosby are very valuable in terms of guidelines, references, best practice and so. Unfortunately it was not enough for me, because I can't make a breaking change in legacy code.

Solution to this particular problem was to encapsulate SqlConnection's Open and Close methods with same lock. Please note that it fits our scenario, it might not fit others.

I am truly sorry I can't dig into this deeper now and find out whether the source of problem was our code or connection pool was not fully thread-safe. I am aware that most likely the source is in our code. Having this in mind, this answer is more workaround than real solution.

Before anyone applies this workaround, please read other answers as well.

Doublepark