calling SqlConnection.Close() from class destructor causes exception

Well, today I'm designing a class that's basically function is to talk to a database.  I'm having a SqlConnection member to share between all the methods and thought it would be a good idea to put a call to .Close in the class destructor.

This is causing an InvalidOperationException...I'm a little confused by that...isn't the destructor for code to be run when the object is being garbage collected?  Wouldn't that be the right time to make sure a connection to a database is closed?  If anyone has any input on the subject I'm all ears (well eyes, but you get the idea).

Comments

• Anonymous
  March 07, 2006
  I've always read that you should not do this, but I can't find any specific reason why.  The proper way to do this is to implement IDisposable in your class and perform this Close() in the dispose function.
  
  If you find out why, post it here - I'd certainly like to know.
• Anonymous
  March 07, 2006
  Will do...thanks for your comment.  :)
• Anonymous
  March 07, 2006
  My understanding is your class should implyment IDisposable and dispose of any external resourse on Dispose(). But if a user of your class forgets to call Dispose then your external resource (SQLConnection) still needs disposing of :-( BUT the good news is that if SQLConnection is implimented correctly it will have a finallizer that will call dispose on itself so you don't have to :-).
  So best not to add a finallizer to a class unless it has external resources that are not released during GC ie memory that it allocated not on the GC heap or external handles etc..
  Having a finiallizer on a class means it takes 2 GC to collect it, the first runs the finaillizer the secound collects it.
  
  PS I tend to call Dispose on things not Close, Exit or what ever, Dispose doesn't tend to throw errors when the object is already closed.
• Anonymous
  March 07, 2006
  This is actually in the Documentation for the method on MSDN:
  
  http://msdn.microsoft.com/library/default.asp?url=/library/en-us/cpref/html/frlrfsystemdatasqlclientsqlconnectionclassclosetopic.asp
  
  "CAUTION   Do not call Close or Dispose on a Connection, a DataReader, or any other managed object in the Finalize method of your class. In a finalizer, you should only release unmanaged resources that your class owns directly. If your class does not own any unmanaged resources, do not include a Finalize method in your class definition. For more information, see Programming for Garbage Collection."
  
  The destructor syntax in managed code actually renders as an override to the Finalize method.  It's not entirely like the C++ unmanaged destructor, but it is similar in that you should use it to manually cleanup unmanaged resources.  Also, in the Finalize method, if you instantiate any managed objects, it will take a second pass with the GC to clean them up (possible performance issues).
• Anonymous
  March 07, 2006
  Well there could be a lot of reasons for this exception. You should always check to see if the status is already closed and if the connection is null. The connection could very well have gone out of scope long before your actual object is being collected and already been collected itself. The garbage collection in .net is actually pretty responsive and it knows when things have gone out of scope and will try to clean them up.
  
  Also, the connection object has it's own destructor which the garbage collector will call, this in itself calls the close method as well if the connection is still open.
  
  Another thing, calling close does not really close a connection. It just sends it back to the connection pool to be used elsewhere. So this is why you should call close often, to release the connection back to the pool. Keeping the connection open just holds that connection in a state of limbo until it is finally collected.
• Anonymous
  March 07, 2006
  You SHOULD NOT release ANY managed resources in Finalizer. Finalizer is only for UNMANAGED resources.
  
  If your class hold no DIRECT references to unmanaged resources, don't define Finalizer (destructor).
  
  Why? Because there is no need for it. If your object is not reachable an can be released, your child object references are not reachable too and will be released itself without any additional effort.
• Anonymous
  March 07, 2006
  Also, you need not close SafeHandle references in .NET 2.0, they are closed itself too. So, for .NET 2.0 there are very few reasons to have Finalizer (destructor) in your custom class.
  
  If your class have finalizer in .NET 2.0, probably it should derive from SafeHandle.
• Anonymous
  March 08, 2006
  Great feedback...thanks to everyone for all your comments.  :)
• Anonymous
  July 11, 2006
  When yu close any connection always use this construct.
  
  VB.NET******
  ......
  myCnn.ConnectionString = ConnectionString
  myCnn.Open()
  myCommand.Connection = myCnn
  myCommand.CommandText = strSQL
  
  Try
             myCommand.ExecuteNonQuery()
  
  Catch ex As Exception
           
            Throw ex
         
  Finally
  
             myCnn.Close()
  
             myCommand = Nothing
             myCnn = Nothing
         End Try
  ***********************************: