datareader - connection leaks in DB

[sri_san]

Hello,
I have a bunch of similar functions as listed below. This one
returns a string but lot of functions return datareader. I close the
reader, the connection and set it to nothing but the connection still
seems to be hanging.


-------------------------
Public Function Login(ByVal email As String, ByVal password As
String) As String

Dim myConnection As New
SqlConnection(ConfigurationSettings.AppSettings("connectionString"))
Dim myCommand As New SqlCommand("UserLogin", myConnection)
Dim myreader As SqlDataReader

' Mark the Command as a SPROC
myCommand.CommandType = CommandType.StoredProcedure

' Add Parameters to SPROC
Dim parameterEmail As New SqlParameter("@Email",
SqlDbType.NVarChar, 100)
parameterEmail.Value = email
myCommand.Parameters.Add(parameterEmail)

Dim parameterPassword As New SqlParameter("@Password",
SqlDbType.NVarChar, 50)
parameterPassword.Value = password
myCommand.Parameters.Add(parameterPassword)

' Open the database connection and execute the command
myConnection.Open()
myreader =
myCommand.ExecuteReader(CommandBehavior.CloseConnection)
'myConnection.Close()

If myreader.HasRows Then
myreader.Read()
Return CStr(myreader("name")) & "," &
CInt(myreader("userid"))
Else
Return String.Empty
End If

myreader.Close()
myreader = Nothing
If myConnection.State = ConnectionState.Open Then
myConnection.Close()
End If
myConnection = Nothing
End Function

 

[Guest]

try disposing ur command and connection object

myCommand.Dispose()
myConnection.Dispose()

 

[Karl Seguin]

Sri:
Couple things..
Remember that ADO.Net supports connection pooling, so closing a connection
doesn't necessarily disconnect it from the database (this is a good thing)

However, your code does seems flawed:

If myreader.HasRows Then
myreader.Read()
Return CStr(myreader("name")) & "," & CInt(myreader("userid"))
Else
Return String.Empty
End If

'THIS CODE WILL NEVER GET EXECUTED
myreader.Close()
myreader = Nothing
If myConnection.State = ConnectionState.Open Then
myConnection.Close()
End If
myConnection = Nothing


since you are returning, the bellow code will never execute...return exists
the function, you can use a try/finally block:

try
if myreader.Read() then
Return CStr(myreader("name")) & "," & CInt(myreader("userid"))
end if
return String.Empty
finally
myReader.Close()
myConnection.Dispose()
end try


the finally block will always execute, even if you have a return statement
(it'll execute right after it in that case). Also note you don't have to
set things to nothing...and I used Dispose instead of Close as a personal
(and strongly recommended) preference...

karl

 

[bruce barker]

1) you need to add try catches to handle clsing when errors happen.
2) having a function return a datareader is a bad idea, as this usually
leads to connection leaks, as you have found. one of the consumers is not
closing the connection. you should refactor to return a dataset, datatable
or custom object.

-- bruce (sqlwork.com)


| Hello,
| I have a bunch of similar functions as listed below. This one
| returns a string but lot of functions return datareader. I close the
| reader, the connection and set it to nothing but the connection still
| seems to be hanging.
|
|
| -------------------------
| Public Function Login(ByVal email As String, ByVal password As
| String) As String
|
| Dim myConnection As New
| SqlConnection(ConfigurationSettings.AppSettings("connectionString"))
| Dim myCommand As New SqlCommand("UserLogin", myConnection)
| Dim myreader As SqlDataReader
|
| ' Mark the Command as a SPROC
| myCommand.CommandType = CommandType.StoredProcedure
|
| ' Add Parameters to SPROC
| Dim parameterEmail As New SqlParameter("@Email",
| SqlDbType.NVarChar, 100)
| parameterEmail.Value = email
| myCommand.Parameters.Add(parameterEmail)
|
| Dim parameterPassword As New SqlParameter("@Password",
| SqlDbType.NVarChar, 50)
| parameterPassword.Value = password
| myCommand.Parameters.Add(parameterPassword)
|
| ' Open the database connection and execute the command
| myConnection.Open()
| myreader =
| myCommand.ExecuteReader(CommandBehavior.CloseConnection)
| 'myConnection.Close()
|
| If myreader.HasRows Then
| myreader.Read()
| Return CStr(myreader("name")) & "," &
| CInt(myreader("userid"))
| Else
| Return String.Empty
| End If
|
| myreader.Close()
| myreader = Nothing
| If myConnection.State = ConnectionState.Open Then
| myConnection.Close()
| End If
| myConnection = Nothing
| End Function
|
|
| ---------------------------
|
| I would appreciate if anyone could point out the reason.
| Thanks,
| Sam.
|

 

[Marina]

You have a return statement before all the code that closes the connection.
Once the return statement executes, the function is done! No code after it
is executed!

The lesson: close your connection first, before returning the value your
function retrieved.
Another lesson: Debug your code and step through it first, so you don't need
to post on the newsgroup. You would have seen your clean up code is never
executed.

 

[sri_san]

thanks for the reply. That seems to be working.. I have another
twister here.. not sure whats wrong with it . Below is the code :

If Request.IsAuthenticated = True Then
Dim conn As SqlConnection = New
SqlConnection(ConfigurationSettings.AppSettings("ConnectionString"))
Dim cmd As SqlCommand = New SqlCommand(sqlStr, conn)
Dim reader As SqlDataReader
Dim userType As String
Try
conn.Open()
reader =
cmd.ExecuteReader(CommandBehavior.CloseConnection)
reader.Read()
userType = CType(reader(0), String)
If userType = "S" Then
' condition
Else
'condition
Catch ex As Exception
Finally
conn.Dispose()
If Not reader Is Nothing Then
reader.Close()
End If
End Try
End If

The code goes in the page_init * event.
Is it enough to dispose the connection or do I need to close the
datareader too?


Thanks,
Sam.

 

[Karl Seguin]

It may be enough to simply close/dispose the connection, but why not get the
datareader too? It's just good practice...if something exposes close or
dispose, call it (and make sure it's being called!)

Karl