views:

1529

answers:

2

The following method in a static class gives me a time out exception because the connection pool is maxed out.

While in debug mode I looked in sql Management studio and saw there were 150 sleeping processes.

I expected the connections to be closed automatically...I also tried putting as a static member and still got the same Error.

Any Ideas? heres the code:

public static Decimal ExecuteScalarDec(string procName, params object[] parameters)
{
    try
    {
        return (Decimal)DatabaseFactory.CreateDatabase().ExecuteScalar(procName, parameters);
    }
    catch (Exception ex)
    {
        throw new Exception(procName.ToString() + " " + parameters.ToString(), ex);
    }
}
+3  A: 
public static Decimal ExecuteScalarDec(string procName, params object[] parameters)
{
    try
    {
        using (Database database = DatabaseFactory.CreateDatabase())
        {
            return (Decimal)database.ExecuteScalar(procName, parameters);
        }
    }
    catch (Exception ex)
    {
        throw new Exception(procName.ToString() + " " + parameters.ToString(), ex);
    }
}

Update

OK, since this is EnterpriseLibrary code. The Database class implements ExecuetScalar like this (other signatures will collapse to this eventually):

 public virtual object ExecuteScalar(DbCommand command)
        {
            if (command == null) throw new ArgumentNullException("command");

            using (ConnectionWrapper wrapper = GetOpenConnection())
            {
                PrepareCommand(command, wrapper.Connection);
                return DoExecuteScalar(command);
            }
        }

and the ConnectionWrapper disposes the connection (end of source file in link) so, the theory goes, your call should be OK and dispose the connection.

The GetOpenConnection() method returns a wrapper that does dispose the connection... except if one exists in the current TransactionScopeConnections:

 protected ConnectionWrapper GetOpenConnection(bool disposeInnerConnection)
    {
        DbConnection connection = TransactionScopeConnections.GetConnection(this);
        if (connection != null)
        {
            return new ConnectionWrapper(connection, false);
        }

        return new ConnectionWrapper(GetNewOpenConnection(), disposeInnerConnection);
    }

And here is how TransactionScopeConnections returns the connection:

  public static DbConnection GetConnection(Database db)
    {
        Transaction currentTransaction = Transaction.Current;

        if (currentTransaction == null)
            return null;

        Dictionary<string, DbConnection> connectionList;
        DbConnection connection;

        lock (transactionConnections)
        {
            if (!transactionConnections.TryGetValue(currentTransaction, out connectionList))
            {
                // We don't have a list for this transaction, so create a new one
                connectionList = new Dictionary<string, DbConnection>();
                transactionConnections.Add(currentTransaction, connectionList);

                // We need to know when this previously unknown transaction is completed too
                currentTransaction.TransactionCompleted += OnTransactionCompleted;
            }
        }

        lock (connectionList)
        {
            // Next we'll see if there is already a connection. If not, we'll create a new connection and add it
            // to the transaction's list of connections.
            // This collection should only be modified by the thread where the transaction scope was created
            // while the transaction scope is active.
            // However there's no documentation to confirm this, so we err on the safe side and lock.
            if (!connectionList.TryGetValue(db.ConnectionString, out connection))
            {
                // we're betting the cost of acquiring a new finer-grained lock is less than 
                // that of opening a new connection, and besides this allows threads to work in parallel
                connection = db.GetNewOpenConnection();
                connectionList.Add(db.ConnectionString, connection);
            }
        }

        return connection;
    }

Now unless I'm mistaken, the TransactionsScopeConnections will always create a fresh conenction for a brand new Database object (as in your case) and keep them in it internal dictionary. The Database object does not implement Disposable, so I'm lost in determining who exactly is supposed to clean up the connections from this TransactionScopeConnecitons internal list.

Matt, is it possible to follow the steps in this article about CLR leaks and see if there are large numbers of Database objects in your process? Load SOS and do a !dumpheap -type Microsoft.Practices.EnterpriseLibrary.Data.Database. If you find many objects, can you trace the pin stack on some of them with !gcroot <AddressOfObject>

Remus Rusanu
This doesn't work because Database isn't Disposable
Matt
then you better make it Disposable or you'll be tracking leaked connections from now till the day after.
Remus Rusanu
Then that's the fault of your Database class: if it's not disposable then it should make sure that *it* disposes the connection in its ExecuteScalar method.
Jon Skeet
@Remus: It doesn't need to be Disposable if it's meant to be a one-shot class... it *could* make ExecuteScalar open the connection, execute and close. It sounds like it's not doing that though :( Basically if the Database class has a reference to the database connection as state, it should implement IDisposable.
Jon Skeet
@Remus: We just don't know about the Database class at the moment, unfortunately. @Matt: we need more information.
Jon Skeet
EnterpriseLibrary actualy
Remus Rusanu
Is it EnterpriseLibrary, matt? http://msdn.microsoft.com/en-us/library/dd203144.aspx
Remus Rusanu
"By design, most of the Database class methods handle the opening and closing of connections to the database on each call. Therefore, the application code does not need to include code for managing connections.". ExecuteReader is an exception (because it returns a resource). ExecuteScalar is in limbo: it returns a 'scalar'. However, I guess the scalar can be quite heavy, eg. a Stream constructed from a large datatype return, and that *would* require to keep the conenction open.
Remus Rusanu
+7  A: 

"By design, most of the Database class methods handle the opening and closing of connections to the database on each call. Therefore, the application code does not need to include code for managing connections.". ExecuteReader is an exception (because it returns a resource). ExecuteScalar is in limbo: it returns a 'scalar'. However, I guess the scalar can be quite heavy, eg. a Stream constructed from a large datatype return, and that would require to keep the conenction open. – Remus Rusanu

I couldn't comment on your answer because it says "commenting requires 50 reputation" After I registered my user...

I'm returning a column Id in executeScalar() and the value is returned - I know this because the next call to execute scalar is only called after I recieve a value... It doesn't seam to make sense that the stream will remain open forever And I saw in sql Management that all the processes are sleeping.

Matt
+1 for u to get points fast so you can comment
Remus Rusanu
Thanks :)It doesn't seem to make sense to me that the stream remains open since I get a result from the method
Matt