DbConnection and Finalizer

[Trecius]

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

 

[Jeroen Mostert]

I've created a class that wraps DbConnection.

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

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.

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.

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

However, is the following acceptable?

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

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://msdn.microsoft.com/library/fs2xkftw for the full explanation of
this boilerplate. You will almost never need to implement a finalizer.

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.

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.

 

[Marc Gravell]

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.

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.

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

 

[Göran Andersson]

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();
}
}

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.

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.

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.

Therefore, as long as I have pooling enabled, is this code
valid?
No.

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

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.