?
Solved

What's the right way to close the database connection in this SQL and C# code

Posted on 2012-09-06
10
Medium Priority
?
415 Views
Last Modified: 2012-10-25
I am using this code on page_load in ASP.NET 2.0 and C# with SQL server 2005 express.

   string query = "select MetaDescription, MetaKeywords, H1Text, Ptitle, H2Text FROM TitleText Where CatalogItemId ='" + this.CurrentEnsemble.ItemId + "'";
                    SqlConnection myconnection = new SqlConnection(ConfigurationManager.ConnectionStrings["Dbconn"].ConnectionString);
                    SqlCommand SqlCmd = null;
                    SqlCmd = new SqlCommand(query, myconnection);
                    SqlCmd.Connection.Open();

                    SqlCmd.ExecuteNonQuery();

                    SqlDataAdapter ad = new SqlDataAdapter(SqlCmd);
                    DataTable dt = new DataTable();
                    ad.Fill(dt);
                    if (dt.Rows[0]["Ptitle"].ToString() == "")
                    {
                        this.Page.Title = this.CurrentEnsemble.Title + " | from Mysite.com";
                    }
                    else
                    {
                        this.Page.Title = this.CurrentEnsemble.Title + " " + dt.Rows[0]["Ptitle"].ToString();
                    }
                    HtmlMeta metaDesc = (HtmlMeta)Page.Header.FindControl("metaDesc");
                    if (dt.Rows[0]["MetaDescription"].ToString() == "")
                    {
                        metaDesc.Attributes.Add("Content", this.CurrentEnsemble.Title + " - from Mysite.com");
                    }
                    else
                    {
                        metaDesc.Attributes.Add("Content", dt.Rows[0]["MetaDescription"].ToString() + " from Mysite.com");
                    }
                    HtmlMeta metaKey = (HtmlMeta)Page.Header.FindControl("metaKey");
                    if (dt.Rows[0]["MetaKeywords"].ToString() == "")
                    {
                        metaKey.Attributes.Add("Content", "some description about the page");
                    }
                    else
                    {
                        metaKey.Attributes.Add("Content", dt.Rows[0]["MetaKeywords"].ToString());
                    }
                    if (dt.Rows[0]["H1Text"].ToString() == "")
                    {
                        h1.InnerText = "some description about the page";
                    }
                    else
                    {
                        h1.InnerText = this.CurrentEnsemble.Title + " " + dt.Rows[0]["H1Text"].ToString();
                    }

                    HtmlGenericControl h2 = (HtmlGenericControl)Master.Master.Master.FindControl("h2bottom");
                    if (dt.Rows[0]["H2Text"].ToString() == "")
                    {
                        h2.InnerText = "some description about the page";
                    }
                    else
                    {
                        h2.InnerText = this.CurrentEnsemble.Title + " " + dt.Rows[0]["H2Text"].ToString();
                    }

Open in new window


Right now when its the site is being viewed by many customers, its throwing this error

365534	System.InvalidOperationException: Timeout expired.  The timeout period elapsed prior to obtaining a connection from the pool.  This may have occurred because all pooled connections were in use and max pool size was reached.     at System.Data.ProviderBase.DbConnectionFactory.GetConnection(DbConnection owningConnection)     at System.Data.ProviderBase.DbConnectionClosed.OpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory)     at System.Data.SqlClient.SqlConnection.Open()     at Microsoft.ApplicationBlocks.Data.SqlHelper.PrepareCommand(SqlCommand command, SqlConnection connection, SqlTransaction transaction, CommandType commandType, String commandText, SqlParameter[] commandParameters, Boolean& mustCloseConnection)     at Microsoft.ApplicationBlocks.Data.SqlHelper.ExecuteNonQuery(SqlConnection connection, CommandType commandType, String commandText, SqlParameter[] commandParameters)     at MysitePage.HandlePageError(Exception ex) in c:\Websites\gn4f4ada\App_Code\MystitePage.cs:line 183     at Mysite.products.Page_Load(Object sender, EventArgs e) in c:\Websites\gn4f4ada\browse\products.aspx.cs:line 73     at System.Web.Util.CalliHelper.EventArgFunctionCaller(IntPtr fp, Object o, Object t, EventArgs e)     at System.Web.Util.CalliEventHandlerDelegateProxy.Callback(Object sender, EventArgs e)     at System.Web.UI.Control.OnLoad(EventArgs e)     at System.Web.UI.Control.LoadRecursive()     at System.Web.UI.Page.ProcessRequestMain(Boolean includeStagesBeforeAsyncPoint, Boolean includeStagesAfterAsyncPoint)	2012-09-06 08:37:00.317

Open in new window



the line: 73 in the products.aspx.cs is where the error handling catch Exception(ex) is and the exception is being stored in the database using a Stored Procedure which is being executed from line 183 in MysitePage.cs file

I am open to suggestions and want to know what's the best way to handle this.

Is closing the connection will solve this problem, if so please tell me what's the correct way to close the connection.

Thanks and appreciate it
0
Comment
Question by:niceoneishere
  • 4
  • 3
  • 2
  • +1
10 Comments
 
LVL 8

Expert Comment

by:cubaman_24
ID: 38372355
Hello:
TimeOut is a property of your SqlCommand instance. Increase it if needed.
The rigth way to handle connections is more or less like this:

SqlConnection con = null;
SqlCommand cmd = null;
SqlDataAdapter adt = null
   try{
        //Initialize your objects here;
   }catch(Exception ex){
        //Handle errors
   }finally{
      //Release resources
      if(con != null)
         con.Dispose();
         //Same for adapter and command
   }

Open in new window

Also, check your database design. Maybe some missing index is slowing your queries..
Best regards.
0
 
LVL 12

Accepted Solution

by:
rajapandian_81 earned 1600 total points
ID: 38372461
Hi,

There is no need to open connection when you are using sqldataadapter. So delete the following lines from your code.
SqlCmd.Connection.Open();
SqlCmd.ExecuteNonQuery();

Open in new window

0
 
LVL 2

Author Comment

by:niceoneishere
ID: 38372542
Thanks for replying, So by removing the open and executenonquery will reduces my errors.

just trying to understand sorry I am little new to this,

Thanks and appreciate it

And also Mr. cubaman_24 can you please explain what you mean by this

Also, check your database design. Maybe some missing index is slowing your queries..

Thanks Guys
0
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
LVL 12

Expert Comment

by:rajapandian_81
ID: 38372629
Hi,

No problem. I'm ready to explain.

Removing that lines will solve the issue.

You haven't written code to close the connection (SqlCmd.Connection.Open();). So the maximum number of connections has been reached and you are getting the error. When we use connected architecture (for eg., datareader.), we should open connection - do process - close connection.

But when we use dataadapter, it is disconnected architecture. So there is no need to open and close connection.
0
 
LVL 16

Expert Comment

by:ToddBeaulieu
ID: 38372688
If you DO need to open/close connections, or commands or anything else that implements IDispoable, you should do yourself a big favor and skip the old "try...finally" approach and use the USING construct, which will automatically handle closing of resources when done.

http://www.w3enterprises.com/articles/using.aspx
0
 
LVL 2

Author Comment

by:niceoneishere
ID: 38372723
ok I got it now, I removed the open and executenonquery and will test it out.

I have no need to open/close connections as long as the data is being displayed from the database. I think I might have made a very beginners mistake here.

I really appreciate you guys explaining in this very detail and simple terms. Appreciate it.

I will also test the USING construct, may be tomorrow as we are in middle of sending an email campaign to 15000 customers and dont want them to see the too many error pages or none at all.


Thanks once again
0
 
LVL 8

Assisted Solution

by:cubaman_24
cubaman_24 earned 400 total points
ID: 38372755
Hello niceoneishere:
You are receiving TimeOutExceptions. It means your query took too long to complete and caused the error. Your query can take too much time to complete for many reasons: Too much data, server overloaded, etc..
But if your queries are optimized and your DDBB design is correct, there is a big chance your query can complete and avoid the time out.
One more thing: Your code is asking for an Sql Injection:
string query = "select MetaDescription, MetaKeywords, H1Text, Ptitle, H2Text FROM TitleText Where CatalogItemId ='" + this.CurrentEnsemble.ItemId + "'";

Open in new window

You must use parameters instead of concatenating strings. See this:
http://www.csharp-station.com/Tutorial/AdoDotNet/lesson06
http://en.wikipedia.org/wiki/SQL_injection

Best regards
0
 
LVL 2

Author Comment

by:niceoneishere
ID: 38372786
Thanks for explaining that but ItemId is in an a abstract class where its defined as

public int ItemId { get; protected set; }

Does this still make it vulnerable, as the user wont be entering any input at all

Thanks
0
 
LVL 8

Expert Comment

by:cubaman_24
ID: 38372859
Hello niceoneishere.
If this field's value is not coming from user input, then "maybe" is safe..
However, you better get used to always use parametric queries instead of string concatenation to be sure you're safe.

Best regards.
0
 
LVL 2

Author Closing Comment

by:niceoneishere
ID: 38534877
Thanks
0

Featured Post

Windows Server 2016: All you need to know

Learn about Hyper-V features that increase functionality and usability of Microsoft Windows Server 2016. Also, throughout this eBook, you’ll find some basic PowerShell examples that will help you leverage the scripts in your environments!

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Problem Hi all,    While many today have fast Internet connection, there are many still who do not, or are connecting through devices with a slower connect, so light web pages and fast load times are still popular.    If your ASP.NET page …
Entity Framework is a powerful tool to help you interact with the DataBase but still doesn't help much when we have a Stored Procedure that returns more than one resultset. The solution takes some of out-of-the-box thinking; read on!
Please read the paragraph below before following the instructions in the video — there are important caveats in the paragraph that I did not mention in the video. If your PaperPort 12 or PaperPort 14 is failing to start, or crashing, or hanging, …
Loops Section Overview

807 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question