views:

21

answers:

2

Hi,

I have BasePage.cs class which is being used by other .cs files instead of System.Web.UI.Page (public partial class page : BasePage).

I am using it for opening and closing SQL connections to make sure every SQL connection gets closed.

Code looks like this

{
    public class BasePage: System.Web.UI.Page
    {
        public SqlConnection globalConnection;

        protected override void OnInit(EventArgs e)
        {
             globalConnection = new SqlConnection();
             globalConnection.ConnectionString = ConfigurationManager.ConnectionStrings["kontemiConnectionString"].ToString();
             globalConnection.Open();
        }

        protected override void OnUnload(EventArgs e)
        {
            if (globalConnection != null)
            {
                 globalConnection.Close();
            }
        }
    }
}

So far it has worked really well for me. It means that every time a connection opens it also gets closed. Or at least I thought so.

My question is whether this solutions is bulletproof and every single connection gets closed in case there is some processing error during code execution. When tracing this code, if I on purpose create error 500 it always goes to OnUnload event and gets closed.

So, do you think is this execution safe? (To stop discussion whether I shouldn't open SQL when I actually need it, answer is that every page which uses BasePage also opens a SQL connection.)

+1  A: 

I might be wrong, but my guess would be that the connection does not get closed if the page lifecycle is ended through an exception before OnUnload fires. The very least you could do to prevent this is make sure you catch all exceptions on a global 'last-chance' level, and close the connection there. I still think using connections locally, ideally with using blocks, is the better solution, because it doesn't keep connections open longer than it needs to, and you don't have to worry about closing them (the semantics of using will do the work for you).

tdammers
Is global necessary? Couldn't you just use the Page's error event?
Jace Rhea
can I keep rest of the code? meaning that I will globally define SqlObject and ConnectionString? And then just call using block? Or should I put connection string into Global.asax as it is always same and doesnt need to get loaded with everypage?
feronovak
@Jace Rhea: You could, but global is safer. I have been in situations where the page's OnError did not fire, but a global error handler caught an exception.@feronovak: Of course not. A `using` block is used to create, then use, and ultimately dispose of, an object that implements `IDisposable`. If you don't use it that way, there is no point in using it at all.
tdammers
+2  A: 

Opening and closing SQL connections where you are using them is better practice. Under the hood, the CLR manages connection pooling anyway - reusing connections and closing them when it sees fit. Doing lots of open and closes on connections with the same connection string doesn't add up to as much overhead as you might expect.

Bill