views:

917

answers:

2

We have some client code which is using the SqlConnection class in .NET to talk to a SQLServer database. It is intermittently failing with this error:

"ExecuteReader requires an open and available Connection. The connection's current state is Closed"

The "temporary" solution is to reboot the process, after which everything works - however, that's obviously unsatisfactory.

The code is keeping a cache of SqlConnection instances, one for each database.

We'd like to re-write the code, but before I do, I need to know a few things:

My first question is: Is it inefficient to repeatedly connect and disconnect SqlConnection objects, or does the underlying library perform connection pooling on our behalf?

// Is this bad/inefficient?
for(many-times)
{
    using(SQLConnection conn = new SQLConnection(connectionString))
    {
        // do stuff with conn
    }
}

Because our code does not do the above, what seems the likely cause of the problem is that something happens to the underlying SQLServer database during the "lifetime" of the connection that causes the connection to be closed...

If it turns out that it is worthwile to "cache" SqlConnection objects, what is the recommended way to handle all errors that could be resolved simply by "reconnecting" to the database. I'm talking about scenarios such as:

  • The database is taken offline and brought back online, but the client process had no open transactions while this was happening
  • The database was "disconnected", then "reconnected"

I notice that there is a "State" property on SqlConnection... is there an appropriate way to query that?

Finally, I have a test SQLServer instance set up with full access rights: how can I go about reproducing the exact error "ExecuteReader requires an open and available Connection. The connection's current state is Closed"

+5  A: 

No, it's not inefficient to create lots of SqlConnection objects and close each of them when you're done. That's exactly the right thing to do. Let the .NET framework connection pooling do its job - don't try to do it yourself. You don't need to do anything specific to enable connection pooling (although you can disable it by setting Pooling=false in your connection string).

There are many things that could go wrong if you try to cache the connection yourself. Just say no :)

Jon Skeet
What I was hoping to hear... Any ideas on how I could reproduce the exact error we are seeing? (just to "prove" that I've fixed the problem)?
Paul Hollingsworth
Hard to say, to be honest. It could easily be a race condition, if you're trying to use the same connection from multiple threads.
Jon Skeet
To disable connection pooling: add "Pooling=False;" to the connection string.
Richard
@Richard: Yup, I added that a few minutes before your comment :)
Jon Skeet
+1  A: 

You should enable connection pooling on your connection string. In that case the runtime will add back your connections to the 'pool' when you close them, instead of really disconencting. When a 'new' connection is taken out of the pool it will be reset (ie. sp_reset_connection is called ) then presented to your application as a brand new, fresh connection. The pool is handling transparently such cases as if the connection is closed while idling in the pool.

The cost of creating a new connection 'from scratch' is significant because the authentication requires several roundtrips between client and server (depending on the authentication method and on SSL settings it can be 1 roundtrip in best case vs. about 10 in worse).

And to answer your question, connection raise the OnStateChange event when their state changes, but you shouldn't care about this if you use the pooling.

Remus Rusanu
Connection pooling is the default, you don't need to do anything to enable it. You can disable it, but they is a rare requirement.
Richard