Closing a conneciton in the "unload" method

Question

I have inherited a web framework whereby the previous developer has opened and closed his database connections in the init/unload methods of the page life cycle. Essentially constructor is like this (simplified to demonstrate the point);

public class BasePage
{
   protected DBConnection _conn;

   public BasePage()
   {
      Init += StartConnection;
      Unload += EndConnection;
   }

   private void StartConnection(object sender, EventArgs e)
   {
      _conn = new DBConnection(Application["connectionstring"].ToString());   
   }

   private void EndConnection(object sender, EventArgs e)
   {
      if (_conn == null)
         return;

      if (_conn.Connection.State == ConnectionState.Open)
      {
     _conn.Close();
         _conn.Dispose();
      }
   }
}

Development has been pretty fast since i got here so I never stopped to consider it. Recently, visits have been up and we've started getting the dreaded "Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool..." error.

I'm currently going through the rest of the code looking for possible connection leaks, but the above code has never quite sat right with me and I want to eliminate it as a potential culprit. On to the question then;

Can i rely on the "unload" method ALWAYS being called, even in the event of an exception? Or can anyone see any other potential issues using the above pattern that would make it a prime suspect for these connection leaks?

Cheers,

Mikey

EDIT: In debugging, the unload method always gets called even if there is an exception. I really just need to know of any scenarios where this method would not be called so i can figure out if this is the bit i need to be refactoring first.

EDIT: Thanks to those who have responded so far but please no more recommendations about the IDisposable class, or the "using" or "catch/finally" patterns - This wasn't what my question was! My question is specifically whether a page could ever run its "Init" event but then fail to run is "Unload" event, and why this might happen.

Answer 1

I have no definitive knowledge about if this is safe, but I poked through the Source code for the System.Web.UI.Page class and the unload event is triggered by the private ProcessRequestCleanup() unless the request is asyncronous or a cross page request. The call to the clean up method is inside a finally block coupled to a try block that's surrounding the ProcessRequest. Process request is triggering all the page life cycle events from PreInit to Render. That would mean that Unload will always be triggered (except for in the async and cross page cases), even if an exception occur.

However I would feel very uneasy having this code in my pages since the behaviour of unload isn't exactly documented.

Answer 2

I always make use of using block soemthing as below

using( SqlConnection)
{

}

so that it never cause any problem

if you dont want to write code for opening connection again and again create one class

public class SqlConnectionManager
{
    public SqlConnection GetSqlConnectionManager()
    {
       //create and return connection
       //SqlConnection con = new SqlConnection();
       //return con;
     }

}

In You class files

SqlConnection conn = null;
using (conn = (new SqlConnectionManager()).GetSqlConnectionManager())
{
     //do work with connection
}

So by the above way you do not need to write code again and again and there is also no need to write code to close connection because its automatically get dispose by using block.

Answer 3

A (very) quick test (VS2010 web server, .net v4) has shown me that the Unload event is called when an unhandled exception is raised (at least if raised in Page_Load), so the principle looks like it should work.

The pattern as listed in the example is only dispose'ing the connection if the connection was open.

As _conn is protected, pages descended from BasePage can interact with _conn and modify its value. There are two ways for descendant classes to break the pattern:

1. Call _conn.Close() directly. If the connection is not open, it is not disposed in EndConnection.

2. Modify the value of _conn by either setting it to null or assigning a new instance of DBConnection to it.

Consider changing your EndConnection method, so that _conn is always disposed.

private void EndConnection(object sender, EventArgs e)
{
    if (_conn == null)
    {
       return;
    }
    if (_conn.Connection.State == ConnectionState.Open)
    {
         _conn.Close();
    }
    _conn.Dispose(); // always dispose even if not actually open. It may have been closed explicitly elsewhere.
}

Case 2 can't be caught by EndConnection. Consider making _conn private and providing a getter property:

private DBConnection _conn;

protected DBConnection Connection {
     get 
     {
         return _conn;
     }
}

which prevents descendant classes from changing the value of _conn.

Lastly, is DBConnection a class of your own? I only ask as you quote "_conn.Connection.State" rather than just _conn.State. If so, just double check that the Dispose method of DBConnection disposes its Connection instance correctly.

Answer 4

EDIT: As per PHeiberg's answer this is definitely possible in .net 4 and one can assume unload will always be called.

I checked the .net 2.0 code as well and the same is also true there.