Closing connections issue...novice out of his depth

[countyweb]

Hi All,
I'm a novice at .net and am not so familiar with VB.net.
I've inherited some code which is causing the connection pool to overload.
This code below is what I think is causing the problem...it doesn't seem to close the connection.
The commented out lines I have tried to add but is casues an error.
Does anyone know how I can alter this code to close this connection. I suspect that because the actual result of the datareader is being returned that I won't be able to close the connection as control has passed out of the function.

Can anyone offer any advice?

protected function runDR(byval sqlStr as String) as SqlClient.SqlDataReader
Dim objConn As SqlConnection
Dim objCmd As SqlCommand
Dim myException As SqlException
Dim i as Int16
Try
objConn = New SqlConnection(connString)
objCmd = New SqlCommand(sqlStr, objConn)
objCmd.Connection.Open()
Return objCmd.ExecuteReader
Catch ex As SqlException
Return Nothing
'Finally
'objCmd.Dispose()
'objConn.Close()
End Try
End Function

 

[Qik3Coder]

Close before Dispose (hey that's kinda nifty)

The command has the connection as an object, so you are probably getting a Null Ref / Object Does not Exist error.


You also want the Return statement after the finally, otherwise it won't execute the close.


-Sometimes the answer to your question is the hack that works

 

[pmegan]

The problem you're hitting with the Finally clause is that a datareader closes when its connection is closed/disposed. Are you getting an "Invalid attempt to call Read when reader is closed." error? Disposing of the SqlCommand is fine, but you can't close the connection until after the reader is done with it.

Here are afew things you can look into:

This function looks like it may be used pretty frequently. Is there anything in the connection string putting restrictions on the connection pool that may cause this? For example, if the program is trying to run 20 datareaders and the max pool size is set to 10 you'll get your problem.

Also, make sure the datareaders assigned with this function are closed in the calling procedure(s). By the time processing is done the connection will be out of scope, so closing the reader should kill the connection.

You may want to put some code in your Catch block to close the reader before returning Nothing. The function is using ad-hoc queries, so poorly written queries can cause syntax or timeout exceptions. Since there's nothing telling you the error happened you may have a bunch of half dead readers filling up the pool.

 

[earthandfire]

Qik3Coder

You also want the Return statement after the finally, otherwise it won't execute the close.

A simple example:

Public Class Form1

	Private Function Divide(ByVal x As Integer, ByVal y As Integer) As Integer

		Try
			Return x \ y
		Catch ex As Exception
			MessageBox.Show("Catch")
			Return -1
		Finally
			MessageBox.Show("Finally")
		End Try

	End Function

	Private Sub Button1_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click

		Dim x As Integer = 10
		Dim y As Integer = 0
		MessageBox.Show(Divide(x, y).ToString)
		y = 5
		MessageBox.Show(Divide(x, y).ToString)

	End Sub
End Class

Finally is ALWAYS executed - that is its purpose.

 

[countyweb]

Thanks all for your answers,
this is a good way to learn I think.

I've changed the to ...

protected function runDR(byval sqlStr as String) as SqlClient.SqlDataReader
Dim objConn As SqlConnection
Dim objCmd As SqlCommand
Dim myException As SqlException
Dim i as Int16
Try
objConn = New SqlConnection(connString)
objCmd = New SqlCommand(sqlStr, objConn)
objCmd.Connection.Open()
Return objCmd.ExecuteReader
Catch ex As SqlException
Return Nothing
Finally
objConn.Close()
objCmd.Dispose()
End Try
Return objCmd.ExecuteReader
End Function

...and as pmegan stated I am getting the 'Invalid attempt to FieldCount when reader is closed.' error.

The calling function looks like this:

public function getTowns(byval web_id as string) as SqlClient.SqlDataReader
dim sqlStr as string
sqlStr = "select townname from areajoin where web_id = '" & web_id & "'"
Return runDR(sqlStr)
End Function

...it seems to me that this second function passes the SQL to the first function, which opens the reader and gets the data and passes it back to the second function which in turn passes the result back to the code that populates the webpage.

I can't close the connection anywhere other than the function where it is created as it is not known outside the scope of the class in which it is created.
And I can't close it in the function where it is created becasue the reader needs to open to the function that uses it.

My current thought is that I have to someway put the functions in the same class so the readers can be seen and closed where they are used.

However, the code is unfamiliar, uncommented and appears quite convaluted. So my other plan is to increase the maximum pool size to 300 from 100 when setting up the connection. This would buy me time until I can re-write the whole thing which is on the cards.

Any advice much appreciated.
Thanks for you answers so far

 

[earthandfire]

[tt]
Try
objConn = New SqlConnection(connString)
objCmd = New SqlCommand(sqlStr, objConn)
objCmd.Connection.Open()
Return objCmd.ExecuteReader
Catch ex As SqlException
Return Nothing
Finally
objConn.Close()
objCmd.Dispose()
End Try
Return objCmd.ExecuteReader
[/tt]

The highlighted line will never execute, see my comments above. Either the Return in Try will be executed or the Return in Catch will, therefore NO CODE after End Try will be executed. Code in Finally should be clean up code as it is always guaranteed to execute.

 

[Craig0201]

The answer is quite simple....stop trying to return a Reader.

Refactor your code so you return a collection of the type you need, not the generalised data fetching class.

C

 

[countyweb]

I see what your saying earthand fire and craig.
The code is convoluted and uncommented.
So my solution will be to recode it from scrath (when time permits).
Until then we'll have to wait for the garbage collection to happen and increase the connection pool size.
thanks for your help.
DOes anyone know if it's possible to configure how often the garbagr collector is activated?

 

[countyweb]

Regarding these open connections - we never had this problem with this code until a couple of months ago whcih is about the same time our server moved from SQL 2000 to 2005.
Is this something new to 2005?
Or is the default number of connections less or the gardabe collection less frequent?

 

[pmegan]

The Max Pool Size still defaults to 100. That's what it should be unless its been changed in the connection string.

Garbage collection is part of the program accessing the DB so changing from Sql2000 to 2005 wouldn't haved changed that part of it. You can try forcing a collection using GC.Collect(), but if the connections aren't closed it won't help much.

If you're using .Net 2.0 you can also try calling the SqlConnection.ClearPool() method, maybe in an error trap that then does a one-time recursive function call. It might help by getting rid of excess connections waiting to time out, but it probably won't solve the problem with open connections.


Here are a couple of links I had bookmarked about this stuff. Maybe one of them will give you some helpful info.

http://msdn.microsoft.com/en-us/library/8xx3tyca(VS.80).aspx

http://msdn.microsoft.com/en-us/lib...nt.sqlconnection.connectionstring(VS.80).aspx