Should RecordSet object be nulified in VB.NET?

634 views Asked by At

I am converting old VB6 app to newer VB.NET application and see these 3 lines often after the upgrade:

rs.Close()
'UPGRADE_NOTE: Object rs may not be destroyed until it is garbage collected. Click for more: 'ms-help://MS.VSCC.v80/dv_commoner/local/redirect.htm?keyword="6E35BFF6-CD74-4B09-9689-3E1A43DF8969"'
rs = Nothing

Where rs is of type RecordSet like so:

Dim rs As ADODB.Recordset
rs = New ADODB.Recordset

Should the last

rs = Nothing 

line still exists? Or

rs.Close() 

is enough?

1

There are 1 answers

3
Joel Coehoorn On BEST ANSWER

With VB.Net, you pretty much never set objects to Nothing. That was common in the VB6 era, but with .Net it's no longer helpful, and can in rare situations be actively harmful. What you should do is use the Finally section of a Try/Catch/Finally block (or the Using block for a shorthand) to make sure objects that implement the IDisposable interface have their Dispose() function called in a timely way, and in the case of the old ADO objects the Finally block is the place to call .Close(). You especially want to do this for the ADO connection object.

In summary, the rs = Nothing line should not exist, but rs.Close() isn't enough by itself, either.

But that's really a side issue here. If you're converting to VB.Net, the best thing is to also convert from the original ADO to the more modern ADO.Net, which is more idiomatic for VB.Net. This means using DataSet and DataReader instead of RecordSet in the first place.

Finally, this is also a good time to review your queries and make sure they are using query parameters rather than string concatenation. Hopefully you were already doing this, but there is a lot of old and scary-vulnerable classic-ASP and vb6 code out there, and a .Net migration is a good time to plug those holes. Sql injection isn't something to fool around with.