Share via

calling SqlConnection.Close() from class destructor causes exception

  • Article

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
    ***********************************: