datareader - connection leaks in DB

  • Thread starter Thread starter sri_san
  • Start date Start date
S

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
 
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
 
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.
|
 
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.
 
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.
 
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
 
Back
Top