Am I Leaking ADO.NET Connections?

Here is an example of my code in a DAL. All calls to the database's Stored Procedures are structured this way, and there is no in-line SQL.

Friend Shared Function Save(ByVal s As MyClass) As Boolean

    Dim cn As SqlClient.SqlConnection = Dal.Connections.MyAppConnection
    Dim cmd As New SqlClient.SqlCommand

    Try
        cmd.Connection = cn
        cmd.CommandType = CommandType.StoredProcedure
        cmd.CommandText = "proc_save_my_class"

        cmd.Parameters.AddWithValue("@param1", s.Foo)
        cmd.Parameters.AddWithValue("@param2", s.Bar)

        Return True

    Finally
        Dal.Utility.CleanupAdoObjects(cmd, cn)
    End Try

End Function

Here is the Connection factory (if I am using the correct term):

Friend Shared Function MyAppConnection() As SqlClient.SqlConnection

    Dim cn As New SqlClient.SqlConnection(ConfigurationManager.ConnectionStrings("MyConnectionString").ToString)
    cn.Open()

    If cn.State <> ConnectionState.Open Then
        ' CriticalException is a custom object inheriting from Exception.
        Throw New CriticalException("Could not connect to the database.")
    Else
        Return cn
    End If

End Function

Here is the Dal.Utility.CleaupAdoObjects() function:

Friend Shared Sub CleanupAdoObjects(ByVal cmd As SqlCommand, ByVal cn As SqlConnection)
    If cmd IsNot Nothing Then cmd.Dispose()
    If cn IsNot Nothing AndAlso cn.State <> ConnectionState.Closed Then cn.Close()
End Sub

I am getting a lot of "Timeout expired. The timeout period elapsed prior to completion of the operation or the server is not responding." error messages reported by the users. The application's DAL opens a connection, reads or saves data, and closes it. No connections are ever left open - intentionally!

There is nothing obvious on the Windows 2000 Server hosting the SQL Server 2000 that would indicate a problem. Nothing in the Event Logs and nothing in the SQL Server logs.

The timeouts happen randomly - I cannot reproduce. It happens early in the day with only 1 to 5 users in the system. It also happens with around 50 users in the system. The most connections to SQL Server via Performance Monitor, for all databases, has been about 74.

The timeouts happen in code that both saves to, and reads from, the database in different parts of the application. The stack trace does not point to one or two offending DAL functions. It's happened in many different places.

Does my ADO.NET code appear to be able to leak connections? I've goolged around a bit, and I've read that if the connection pool fills up, this can happen. However, I'm 开发者_如何学编程not explicitly setting any connection pooling. I've even tried to increase the Connection Timeout in the connection string, but timeouts happen long before the 300 second (5 minute) value:

<add name="MyConnectionString" connectionString="Data Source=MyServer;Initial Catalog=MyDatabase;Integrated Security=SSPI;Connection Timeout=300;"/>

I'm at a total loss already as to what is causing these Timeout issues. Any ideas are appreciated.

EDIT: This is a WinForms application.

One way to check for connection leaks is to add max pool size to the connection string, like:

"integrated security=SSPI;server=MyHost;Max Pool Size=1;"

In development I usually run with this setting (or size 2 if the application uses two connections simultaneously.)

From here:

Unlike Finalize, developers should call Dispose explicitly to free unmanaged resources. In fact, you should call the Dispose method explicitly on any object that implements it to free any unmanaged resources for which the object may be holding references.

SqlConnection, SqlCommand, SqlDataReader, etc... all implement IDisposable. If you enclose all these instances in a Using block, then Dispose will be called automatically, your connections will be closed, and you won't have to worry about issues like this. Fire up Reflector and take a look for yourself: (SqlConnection.Dispose)

protected override void Dispose(bool disposing)
{
    if (disposing)
    {
        this._userConnectionOptions = null;
        this._poolGroup = null;
        this.Close();
    }
    this.DisposeMe(disposing);
    base.Dispose(disposing);
}

This also makes the code shorter in that you don't have to manually add a Finally block to clean up your ADO.NET objects.

Using connection As SqlConnection("your connection string")
    Using command As New SqlCommand("your sql", connection)
        connection.Open()
        Using dataReader As SqlDataReader = command.ExecuteReader()
            'Your stuff here
        End Using
    End Using
End Using

Using the Using approach forces you to keep your ADO.NET objects local, which to me is a good thing.

You should always Dispose the Connection, regardless of its State:

 'If cn IsNot Nothing AndAlso cn.State <> ConnectionState.Closed Then cn.Close()
 If cn IsNot Nothing Then cn.Dispose()

I'm not sure it this could cause your timeouts but it certainly is an improvement.

How do your stored procedures perform outside of your application?

What if you moved the 'return true' out of your try/finally block in Save()?

Monitor DB connections in perf monitor to see if they grow.

But the first place I would look is your sprocs themselves - do they access tables in consistent order or might you be running into locks? For example, if proc1 manipulates table1 and then table2, while proc2 hits table2 and then table1 you might run into locking issues.

Is this a windows app or a web app?

Do the timeouts happen with simple stored procedures or just with more complicated ones?

Have you tried running a sql profiler trace to see if any queries really are taking a long time?

Also, have you tried converting to the "using" syntax, which ensures that the objects get closed and disposed of properly?