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.

score 2 · Answer 1 · edited Jul 11 '11 at 13:58

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.

Cheers, This is exactly what I normally do (and almost certainly what I will do eventually once I get my question answered). I just really need to specifically know whether I should be focusing on this particular scenario (the unload one). — Mikey Hogarth, Jul 11 '11 at 11:41

score 2 · Accepted Answer · answered Jul 11 '11 at 14:13

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.

marto · Answer 3 · 2011-07-12T11:59:17.990

1

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.

edited Jul 12 '11 at 11:59

answered Jul 11 '11 at 14:06

marto

4,170
1
28
39

This isn't actually true (see other comments) – Mikey Hogarth Jul 12 '11 at 08:40

Neil Moss · Answer 4 · 2011-07-11T16:20:44.717

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:

Call _conn.Close() directly. If the connection is not open, it is not disposed in EndConnection.
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.

score 0 · Answer 5 · edited May 23 '17 at 12:19

0

No. Take the following example.

What happens if the user closes the browser? The Unload function will then not be called and you will have an open connection to the database.

This Unload question on StackOverflow has a similar problem that you are having.

edited May 23 '17 at 12:19

Community

1
1

answered Jul 11 '11 at 11:08

Jethro

5,896
3
23
24

The unload method is server side though, so surely wouldn't matter what the user did once the request has been received? – Mikey Hogarth Jul 11 '11 at 11:26
"For the page itself, use this event to do final cleanup work, such as closing open files and database connections, or finishing up logging or other request-specific tasks." From http://msdn.microsoft.com/en-us/library/ms178472.aspx. So you might be onto something, but then again if you look at the Unload link in my post you will see it doesn't always get called. I wouldn't count on it being called, if possible rather do what Pranay Rana suggested, and that would be the correct way to dispose your connection. – Jethro Jul 11 '11 at 11:39
Here's also an interesting SO http://stackoverflow.com/questions/302149/is-it-acceptable-to-keep-a-db-connection-open-for-the-life-of-the-page link, maybe the "Time out expired" is being caused by something else, try using SQL Profiler to see whats going on in the background. – Jethro Jul 11 '11 at 11:43
Cheers for this man, I'm going to sit out for a bit and see if I get any more responses - don't want to spend the time implementing the using() pattern if the unload event is just doing the exact same thing. – Mikey Hogarth Jul 11 '11 at 13:35

Closing a conneciton in the "unload" method

5 Answers5

Linked