DbConnection and Finalizer

Hello, Newsgroupians:

I've created a class that wraps DbConnection.

public class CSQL
{
  protected System.Data.Common.DbConnection m_conn;

  public CSQL
  {
    // Initialize the m_conn in here
  }
  ...
}

In my constructor, I create the DbConnection.  Now I've read a thousand 
times in MSDN and online that I'm not to call DbConnection.Close() in my 
finalizer.  However, is the following acceptable?

~CSQL
{
  if (this.m_conn.State != System.Data.ConnectionState.Closed)
  {
    this.m_conn.Close();
  }
}

Most times -- when I'm debugging my code -- I've noticed that when my 
finalizer is called, the state of m_conn is already closed, for I have 
pooling enabled.  Therefore, as long as I have pooling enabled, is this code 
valid?

If it is not valid, how might I solve the problem of creating a connection 
everytime I'd like to perform a query?  For instance, suppose I have the 
possibility of performing 1 to 1000 queries.  With one query, I can see that 
a quick connect and disconnect is valid.  However, if I have 1000 queries in 
a short period of time, obtaining a connection and maintaining that 
connection might be advantageous.  Thank you all for your time and 
consideration.


Trecius

Protected fields considered harmful. Make the base class responsible for 
managing the field, and have derived classes only use it:

   private readonly DbConnection m_conn;
   protected DbConnection Conn { get { return m_conn; } }

There might be a legitimate scenario where derived classes need to change 
the connection instance after construction, but this makes things harder to 
reason about.


This is correct. Finalizers should only release unmanaged resources. If you 
don't own any, don't implement a finalizer.

That still calls .Close() in a finalizer, it just sometimes doesn't. You 
want to avoid finalizers altogether here.

What you want to do is implement IDisposable, just like DbConnection does:

   class CSQL : IDisposable {
     public void Dispose() {
       dispose(true);
       GC.SuppressFinalize(this);
     }

     protected virtual void Dispose(bool disposing) {
       if (disposing) {
         if (m_conn != null) m_conn.Dispose();
       }
     }
   }

See  http://www.**--****.com/ 
this boilerplate. You will almost never need to implement a finalizer.


If connection pooling is enabled (it should be and usually is) there is no 
additional overhead for creating physical connections when you create 
logical ones. There is a slight overhead associated with resetting the 
connection for every logical "open" operation, but it's rarely significant 
compared to the workload.

In other words, when you do this:

   using (var csql = new CSQL()) {
     csql.PerformQuery();
   }

A new CSQL object is created (and discarded), but not a new physical 
connection. Of course, if you really want to execute 1000 queries on one 
connection, you can, but if you do you have to consider the possibility that 
your batch fails halfway, as your connection will be unusable after most 
errors. In this case it's usually appropriate to wrap those 1000 queries in 
a transaction, to ensure they either all of them happen or none of them do.

Reusing logical connection objects buys you little if anything, and it makes 
error handling more complicated. You'll usually want to stick to the "one 
batch, one connection" model, where a batch might be as small as a single query.

-- 
J.

> However, is the following acceptable?

It is both risky and unnecessary. Let the connection worry about
finalizing itself; you can't know (in a finalizer) whether the
reference is still valid. You might, however, choose to implement
IDisposable and close the wrapped connection in the Dispose() for
deterministic cleanup.



What connection are you wrapping? Some (such as SqlConnection) provide
pooling internally, so simply don't worry about it - the time to
aquire a connection from the pool each time is nil compared to the
time to execute a remote query. Just use the "open late, close early"
approach...

Marc

No, that is not a good solution, and it doesn't really serve any 
purpose, as the finalizer in the DBConnection object will close the 
connection.

By the time the finalizer runs, both the CSQL instance and the 
DbConnection instance are unreachable. That means that the DbConnection 
finalizer will be run at about the same time as the CSQL finalizer, and 
there is nothing that says that the CSQL finalizer will run before the 
DbConnection finalizer.

Actually, you don't want to run any finalizers at all. You should call 
the Dispose method of the DbConnection class when you are done with it, 
which will close the connection and suppress the finalizer. If you don't 
call the Dispose method, the finalizer works as a fallback, but this is 
a more expensive way to clean up the resources. If the GC detects that 
the object still has a finalizer, it will placed it in the freachable 
queue instead of being collected, where a background thread will 
eventually run the finalizer. After that the object can be collected.


No, that has nothing to do with pooling. That's because the DbConnection 
finalizer happened to run before the CSQL finalizer. The connection pool 
can't save you from the connections that you leave open.


No.


Usually the connection pool takes care of that for you. When you close 
the DbConnection object, the low level database connection is returned 
to the pool instead of being disconnected. When you create a new 
DbConnection object using the same connection string, the low level 
database connection will just be reset and reused, which is a lot faster 
than establishing a new connection.

If you really have performace problems because you have a lot of 
database connections, you may need to find a way to keep the connection 
open between database operations, but using a finalizer to close the 
connection is not the way to do that.

-- 
G枚ran Andersson
_____
 http://www.**--****.com/