Will this leave a connection open to my database?

Question

I have some legacy code I've inherited and am trying to figure out whether or not this code is causing a connection to stay open to my database. I'm not too familiar with ADO.net but from what I can see this should be causing a problem.

I have a function to get a data reader:

Public Shared Function ExecuteDataReader(ByRef ObjSqlCmd As SqlCommand, ByVal CSTRING As String) As IDataReader
    Try
        Dim cn As New SqlConnection(CSTRING)
        cn.Open()
        ObjSqlCmd.Connection = cn
        Return ObjSqlCmd.ExecuteReader(CommandBehavior.CloseConnection)
    Catch ex As Exception
        If ObjSqlCmd.Connection.State = ConnectionState.Open Then ObjSqlCmd.Connection.Close()
        Return Nothing
    End Try
End Function

And then code using this to read:

Dim cmd As New SqlCommand
'set up command
Dim idr As IDataReader = ExecuteDataReader(cmd, "database")

    If idr Is Nothing Then
        If cmd.Connection.State = ConnectionState.Open Then cmd.Connection.Close()
        Return arrResult
    End If

    While idr.Read
        'read
    End While

    If cmd.Connection.State = ConnectionState.Open Then cmd.Connection.Close()

I can see it closing the connection at the end (assuming nothing goes wrong and an exception is thrown) but from MSDN they say you should always clsoe the data reader. Does that get closed when the connection is closed?

Also from what I understand, the code

 ObjSqlCmd.ExecuteReader(CommandBehavior.CloseConnection)

Will not close the connection unless the data reader is closed. Can someone explain what is happening here and if everything might be closing correctly? I know the best practice is to use "using" and then try/catch/finally to close the reader, the the original developers did not seem to follow these practices.

Answer 1

Yes it will. Any exception that occurs while reading from the data reader will bypass the closing statement. At some point, the garbage collector will kick in and dispose and release the connection back to the connection pool.

Until that point though, the connection can't be used. Worse, any locks acquired while executing the command will remain until the connection is closed.

Using ... using isn't a best practice just because. It's actually a lot simpler and safer to manage commands and connections in this way