Strange GC behaviour - or - my inexperience :-)

[lothar.behrens]

Hi,

I have figured out that .NET CLR implements object destruction on the
knowledge of usage of
these objects. My DBHelper class closes the connection when refcount
goes to zero. That happens inside Finalize.

The sample code at the end crashes when I not do a call to getReader
with an empty string.
That is used as a Dummy for ' This object is used until here '
information.

If I use the last getReader line, All is fine.

I came from C++ and expected that GC doesn't touch objects in the same
scope (stack).

This makes me struggling on how to close connections automatically in
Finalize whithout
explicitly set the object to NULL. (In C++ I have smart pointers that
do this for me)

Any Ideas ?

Thanks, Lothar

Public Class DBHelper
Shared conn As SqlClient.SqlConnection
Shared refcount As Integer

Public Sub New(ByVal where As String)
refcount = refcount + 1
End Sub

Public Function getReader(ByVal SQL As String) As
SqlClient.SqlDataReader
Dim mycmd As SqlClient.SqlCommand
Dim reader As SqlClient.SqlDataReader = Nothing

' Return on empty query

If SQL = "" Then
Return Nothing
End If

Try
If conn Is Nothing Then
conn = New
SqlClient.SqlConnection("connectionstring")
End If
conn.Open()
Catch ex As Exception

' Log some information
conn = Nothing
Return Nothing
End Try

mycmd = conn.CreateCommand
mycmd.CommandText = SQL

Try
reader = mycmd.ExecuteReader()
'CommandBehavior.CloseConnection)
Catch ex As Exception

' Log some information
reader = Nothing
Return Nothing
End Try
Return reader
End Function

Protected Overrides Sub Finalize()
If refcount > 0 Then
refcount = refcount - 1

If refcount = 0 Then
Try
conn.Close()
conn = Nothing
Catch ex As Exception

End Try
End If
End If
MyBase.Finalize()
End Sub


Sample:

Public Sub SomeFunction()
Dim dbHelper As DBHelper = New DBHelper()
Dim reader As SqlClient.SqlDataReader

Dim SQL As String = "SELECT * from SomeTable"
reader = dbHelper.getReader(SQL)

GC.Collect() ' A
Collect to be appear explicitly or indirectly in

' my code
System.Threading.Thread.Sleep(10) ' Some time
consumption

If Not reader Is Nothing Then
While
reader.Read() ' Crash
Dim SomeField As String = reader.GetValue(0)
End While
End If

'dbHelper.ExecuteScalar("") ' Doesn't crash when
this line is uncommented.
End Sub

 

[Duracel]

Implement the IDisposable pattern, disposing the connection in "Dispose",
not Finalize.

Public MyClass
Implements IDisposable

...

End Class

 

[lothar.behrens]

Implement the IDisposable pattern, disposing the connection in "Dispose",
not Finalize.

Public MyClass
Implements IDisposable

...

End Class































- Zitierten Text anzeigen -

As of the documentation it does the same as Finalize. I cannot
guarantee, that the reader is usable after getReader(...).

It is the time, when GC calls Finalize or now Dispose through
Finalize. It appears BEFORE my function block is leaving.

I have to go through my whole project and add this dummy call to avoid
too early object destructions.

That's bad.

 

[Duracel]

imho, where you are using resources that implement IDisposable, you should
dispose of them yourself, rather than waiting for the GC to kick in. From a
C++ background, you are used to deterministic object destruction - whereas
you never really know with .NET when the object will be disposed so in cases
where its important to manage resources, you should make sure it happens
when you want it to:



Using g as Graphics = myControl.CreateGraphics ()

End Using

' g has automagically been disposed, even if an exception occurs


or


Dim g as Graphics = Nothing

Try

g = myControl.CreateGraphics ()

Finally

' g gets disposed, even if an exception occurs

If g IsNot Nothing Then
g.Dispose ()
End If

End Try

 

[lothar.behrens]

imho, where you are using resources that implement IDisposable, you should
dispose of them yourself, rather than waiting for the GC to kick in. From a
C++ background, you are used to deterministic object destruction - whereas
you never really know with .NET when the object will be disposed so in cases
where its important to manage resources, you should make sure it happens
when you want it to:

Using g as Graphics = myControl.CreateGraphics ()

End Using

' g has automagically been disposed, even if an exception occurs

or

Dim g as Graphics = Nothing

Try

g = myControl.CreateGraphics ()

Finally

' g gets disposed, even if an exception occurs

If g IsNot Nothing Then
g.Dispose ()
End If

End Try

Is it possible to force a class to be only used by Using ?
I mean, to help avoid making mistakes in the future.

A quick hack would be a regex replace of Dim [A-Z*][a-z*] As DBHelper
with Using dbhelper as DBHelper over the whole project.

Right ?

 

[Duracel]

Is it possible to force a class to be only used by Using ?
I mean, to help avoid making mistakes in the future.

A quick hack would be a regex replace of Dim [A-Z*][a-z*] As DBHelper
with Using dbhelper as DBHelper over the whole project.

Right ?

Well if you implement IDisposable, you know your connection will dissapear
when the GC collects it, you just don't know when. I would be loathe to
search/replace something like this - rather Find In Files, all instances of
New DBHelper and take it from there. Either use "Using" or use Try/Finally
combination. Take a look at the "Using" keyword either online or in MSDN to
get more information about it before you decided on replacing all instances
of DBHelper in code.

Hope this helps.

 

[Brian Gideon]

Hi,

I have figured out that .NET CLR implements object destruction on the
knowledge of usage of
these objects. My DBHelper class closes the connection when refcount
goes to zero. That happens inside Finalize.

The sample code at the end crashes when I not do a call to getReader
with an empty string.
That is used as a Dummy for ' This object is used until here '
information.

If I use the last getReader line, All is fine.

I came from C++ and expected that GC doesn't touch objects in the same
scope (stack).

This makes me struggling on how to close connections automatically in
Finalize whithout
explicitly set the object to NULL. (In C++ I have smart pointers that
do this for me)

Any Ideas ?

Thanks, Lothar

Public Class DBHelper
Shared conn As SqlClient.SqlConnection
Shared refcount As Integer

Public Sub New(ByVal where As String)
refcount = refcount + 1
End Sub

Public Function getReader(ByVal SQL As String) As
SqlClient.SqlDataReader
Dim mycmd As SqlClient.SqlCommand
Dim reader As SqlClient.SqlDataReader = Nothing

' Return on empty query

If SQL = "" Then
Return Nothing
End If

Try
If conn Is Nothing Then
conn = New
SqlClient.SqlConnection("connectionstring")
End If
conn.Open()
Catch ex As Exception

' Log some information
conn = Nothing
Return Nothing
End Try

mycmd = conn.CreateCommand
mycmd.CommandText = SQL

Try
reader = mycmd.ExecuteReader()
'CommandBehavior.CloseConnection)
Catch ex As Exception

' Log some information
reader = Nothing
Return Nothing
End Try
Return reader
End Function

Protected Overrides Sub Finalize()
If refcount > 0 Then
refcount = refcount - 1

If refcount = 0 Then
Try
conn.Close()
conn = Nothing
Catch ex As Exception

End Try
End If
End If
MyBase.Finalize()
End Sub

Sample:

Public Sub SomeFunction()
Dim dbHelper As DBHelper = New DBHelper()
Dim reader As SqlClient.SqlDataReader

Dim SQL As String = "SELECT * from SomeTable"
reader = dbHelper.getReader(SQL)

GC.Collect() ' A
Collect to be appear explicitly or indirectly in

' my code
System.Threading.Thread.Sleep(10) ' Some time
consumption

If Not reader Is Nothing Then
While
reader.Read() ' Crash
Dim SomeField As String = reader.GetValue(0)
End While
End If

'dbHelper.ExecuteScalar("") ' Doesn't crash when
this line is uncommented.
End Sub

First, it looks like you're using DBHelper as a mechanism for keeping
a connection open. ADO.NET already supports connection pooling so
DBHelper isn't really buying you anything.

Second, assuming you want to continue using DBHelper, you should be
implementing IDisposable since it is logically holds unmanaged
resources. I don't actually see a problem with the Finalize method
per se, but I get the feeling that it is okay by accident. There are
certain things you cannot do from Finalize. Read the following on
implementing IDisposable correctly.

http://msdn2.microsoft.com/en-us/library/fs2xkftw.aspx

Third, the Finalize is partly responsible for the crash. You said
that by uncommenting the dbHelper.ExecuteScalar line the crash no
longer occurs. This is very confusing, but let me try to explain.
The Finalize method is executed when the object is collected the GC.
The GC is *very* aggressive. It can see that dbHelper is no longer
used after the call to getReader. At that point it becomes eligible
for collection immediately. That in turn causes Finalize to run which
closes the SqlConnection and invalidates the SqlDbReader. By
uncommenting the dbHelper.ExecuteScalar call the GC sees that it is
used later and so it is not yet available for collection.

 

[Branco Medeiros]

lothar.behrens wrote:
<backposted/>

The problem is that in SomeFunction you make an explicit call to the
GC. The GC sees that DBHelper isn't being used anymore and naturally
collects the object, thus triggering your finalize and hence the
demise of the connection.

In other words, you're doing it wrong... =)))))

Why? Because the connection is still being used -- even after DBHelper
has been shutdown: a DataReader *requires* an open connection to
operate. Because DBHelper didn't provide for that, the connection was
already dead when the reader tried to use it. That's why you crashed.

Add an explicit CloseConnection method to your DBHelper, so users know
that it must be closed *after* the connection was used (that is, after
any DataReaders fade out). Or create a ReaderAdapter of some kind,
shielding the actual DataAdapter, to allow for you to correctly track
the connection's life time (this would require that the adapter kept a
reference to the original DBHelper, so to prevent it from a premature
death).

And ditch that refcount. You have no way of controlling that (yes, you
probably do, but it's too much work).

HTH.

Regards,

Branco.