.NET Forum / .NET Framework / ADO.NET / January 2006
Close - No Dispose - Memory Leak?
|
|
Thread rating:  |
darren.pruitt@gmail.com - 09 Jan 2006 21:43 GMT I am working with a large legacy ASP.NET application that does not (in my opinion) properly close or dispose of its ADO.NET objects (connections, commands, transactions, etc.). The web site is under heavy load so database objects are being created like mad.
My question is this: Will not properly disposing of these objects lead to memory leaks? Or just garbage collection overload? Or will the system eventually be brought to its knees gasping for more memory?
A typical code example is:
public static string SomeFunction(int someId) { string returnValue = ""; SqlConnection conn = new SqlConnection(AStaticConnectionString);
try { conn.Open();
string commandText = "storedProcName";
SqlParameter[] param = new SqlParameter[1]; param[0] = new SqlParameter("@some_id", SqlDbType.Int, 4); param[0].Value = someId;
// Using the Micorsoft SqlHelper Object returnValue = SqlHelper.ExecuteScalar(conn, CommandType.StoredProcedure, commandText, param).ToString(); }
catch (SqlException sx) { throw sx; }
catch (Exception ex) { throw ex; } finally { conn.Close(); }
return returnValue; }
Cowboy (Gregory A. Beamer) - MVP - 09 Jan 2006 22:36 GMT Before looking at the code, I almost had a knee jerk reaction.
I would prefer to see a Dispose() in the finally, but the Close() will handle returning objects to the connection pool, so it is not as bad as doing nothing. Since ASP.NET is stateless, these objects would eventually free up, but you could hold them much longer than you would like.
 Signature Gregory A. Beamer MVP; MCP: +I, SE, SD, DBA
*************************** Think Outside the Box! ***************************
> I am working with a large legacy ASP.NET application that does not (in > my opinion) properly close or dispose of its ADO.NET objects [quoted text clipped - 43 lines] > return returnValue; > } Shawn Wildermuth - 09 Jan 2006 23:03 GMT Hello darren.pruitt@gmail.com,
I would prefer to see the dispose on all ADO.NET objects, but not because of "memory" leaks. The GC will clean those up, its for *unmanaged* resources (connections, socket connections, etc.). My stance is (and will be) if it supports IDispoable, you should be disposing them. Dispose and the GC are not really related in that Dispose is meant to deal with deterministic destruction for *unmanaged* resources.
HTH
Shawn Wildermuth C# MVP, Author and Speaker http://adoguy.com
> I am working with a large legacy ASP.NET application that does not (in > my opinion) properly close or dispose of its ADO.NET objects [quoted text clipped - 37 lines] > return returnValue; > } Miha Markic [MVP C#] - 10 Jan 2006 07:29 GMT HI Shawn,
> Hello darren.pruitt@gmail.com, > > I would prefer to see the dispose on all ADO.NET objects, but not because > of "memory" leaks. The GC will clean those up, its for *unmanaged* > resources (connections, socket connections, etc.). My stance is (and will > be) if it supports IDispoable, you should be disposing them. You might wonder seeing how much people doesn't think like you and me :-)
 Signature Miha Markic [MVP C#] RightHand .NET consulting & development www.rthand.com Blog: http://cs.rthand.com/blogs/blog_with_righthand/
William (Bill) Vaughn - 09 Jan 2006 23:39 GMT I expect that in many cases close vs dispose issues are a red herring. They are usually not the reason for issues with memory or object (connection pool) overflows. If your application is not properly designed or is simply under too much demand, the system cannot complete the existing tasks before new tasks are placed in the demand queue. Ideally, your system would complete N tasks/second and if the load stays below that level it works fine. Objects are created, closed and the objects they create are released for further use or marked for disposal. If the load increases past that point, the system begins to thrash as it hunts for resources (swapping, running the GC, asking other applications to release memory). I expect that if the load only momentarily exceeds capacity, that the system would settle back and work fine again. However, if the trend continues it would fall over the edge and not recover.
hth
 Signature ____________________________________ William (Bill) Vaughn Author, Mentor, Consultant Microsoft MVP INETA Speaker www.betav.com/blog/billva www.betav.com Please reply only to the newsgroup so that others can benefit. This posting is provided "AS IS" with no warranties, and confers no rights. __________________________________
>I am working with a large legacy ASP.NET application that does not (in > my opinion) properly close or dispose of its ADO.NET objects [quoted text clipped - 43 lines] > return returnValue; > } Cor Ligthert [MVP] - 10 Jan 2006 09:13 GMT Darren,
What do you mean with dispose. The method dispose is to set disposable the object for the GC. As it is with all managed object intrensic done as the time is there for that (by instance because it goes out of scope). Dispose is the methode to place your code for disposing unmanaged resources in by overloading your class.method.
The Method Dispose is almost as much in the mainly used classes as the method GetHashCode. GetHashCode is inherited from Object while Dispose is inherited from Component.
I never see those who write forever that you should use Dispose becose it is in the class, write the same for GetHashCode.
I hope this gives some ideas
Cor
Miha Markic [MVP C#] - 10 Jan 2006 09:55 GMT > Darren, > > What do you mean with dispose. The method dispose is to set disposable the > object for the GC. As it is with all managed object intrensic done as the > time is there for that (by instance because it goes out of scope). Not sure if I follow.
Dispose
> is the methode to place your code for disposing unmanaged resources in by > overloading your class.method. And to dispose managed resources held by instance being disposed.
> The Method Dispose is almost as much in the mainly used classes as the > method GetHashCode. GetHashCode is inherited from Object while Dispose is > inherited from Component. What has GetHashCode to do with Dispose?
> I never see those who write forever that you should use Dispose becose it > is in the class, write the same for GetHashCode. What has GetHashCode to do with Dispose? Perhaps I've misunderstood you,
 Signature Miha Markic [MVP C#] RightHand .NET consulting & development www.rthand.com Blog: http://cs.rthand.com/blogs/blog_with_righthand/
Cor Ligthert [MVP] - 10 Jan 2006 10:06 GMT Miha.
> What has GetHashCode to do with Dispose? GetHashCode is in every object. Following the often used sentence by you. "It is not for nothing there so you should use that", than that applies for the GetHashCode as well in my opinion.
If I would agree with your statement by the way, what I don't.
Cor
Miha Markic [MVP C#] - 10 Jan 2006 10:22 GMT > Miha. > [quoted text clipped - 5 lines] > > If I would agree with your statement by the way, what I don't. Cor, I really don't know why you are saying that nonsense. You do realize the difference in importance of disposing an instance and getting a hashcode out of it, do you?
 Signature Miha Markic [MVP C#] RightHand .NET consulting & development www.rthand.com Blog: http://cs.rthand.com/blogs/blog_with_righthand/
Cor Ligthert [MVP] - 10 Jan 2006 11:03 GMT Miha,
> Cor, I really don't know why you are saying that nonsense. > You do realize the difference in importance of disposing an instance and > getting a hashcode out of it, do you? Because I find it nonsense to read everytime from you as standard answer that dispose should be used forever just because it is as method in a class.
Disposing is only important as unmanaged resources are used. Which are almost not in the system.data namespace (AdoNet).
(By the way, I did not reply or critizese you as you are doing to me now, you did it at my reply to the OP)
Cor
Lasse Våqsæther Karlsen - 10 Jan 2006 11:02 GMT Hello Cor Ligthert [MVP],
> Darren, > [quoted text clipped - 14 lines] > > Cor The Dispose method is part of the IDisposable interface (contract). Component implements it but there's a ton of other classes that implements it as well.
That a class implements the IDisposable interface means you should dispose of it. Classes that are implemented properly will clean up its unmanaged resources if you leave them to finalization but the end result might not be what you want (ref: BufferedStream will not flush its buffer to the underlying stream for instance as there is no finalizer), and you certainly loose the opportunity to control when those resources are cleaned up.
Now, Dispose has nothing to do with GC except that it has to do with managing resources. GC manages memory, Dispose manages other resources. If you fail to dispose of an object you are basically saying that "I'll just leave this object for the garbage collector to pick up at a later time". However, when you call Dispose you are just saying "I'll make sure it closes that file, and that socket, and the memory it occupies I'll just leave for the garbage collector to pick up at a later time".
Doing a dispose will not make the object be collected earlier but you might save it from going into the finalization queues, and you pick the cleanup point of the unmanaged resources yourself.
So... basically, if you can Dispose of it, at one point you should. You will not have long-term leaks if you don't (and the class is properly implemented) but you might hold on to resources (like files and sockets) longer than you like, and you might make the object go through the finalization queue which might prolong the life of the memory the object occupies.
Now, as for GetHashCode, that method is inherited from System.Object and is part of every class. Other than that I don't see what this has to do with the issue originally posted.
 Signature Lasse Vågsæther Karlsen http://usinglvkblog.blogspot.com/ mailto:lasse@vkarlsen.no PGP KeyID: 0x2A42A1C
Cor Ligthert [MVP] - 10 Jan 2006 11:25 GMT Lasse,
> Now, as for GetHashCode, that method is inherited from System.Object and > is part of every class. Other than that I don't see what this has to do > with the issue originally posted. It has to do with the sentence "It is there so you should use it", as is often written in this newsgroup
It does not mean, that I tell never to use dispose. For objects that uses many handles (by instance bitmaps) or unmanaged resources (modal forms) it is surely better to do.
But that is AFAIK not the fact with most classes from system.data (AdoNet).
I tell forever use dispose as it is needed, not because the in my opinion nonsense is written. "Dispose is not for nothing in almost every object". And that is why I used as sample GetHashCode. I took that because I think that it is seldom used by an end developer.
Every method in a program uses processing time and gives less easy to maintain programs.
For the rest does your message not contains much that I disagree. Did you know that there is now a very nice page about dispose on MSDN2
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/cpref/html/frlr fsystemidisposableclasstopic.asp
Beneath the table is in the remarks almost the same written as you do. (And I try to tell).
Cor
Darren - 10 Jan 2006 16:33 GMT Gentlemen thank you for your responses, it was very informative. I think that William identified for me what I needed to know. After profiling the application I found that there were a lot of long lived objects and, surprisingly, there were a lot more String objects then ADO.NET objects.
So I will keep beating the system and try to find out why it is thrashing the server.
FYI, here is how I would prefer to do the data access:
public static string SomeFunction(int someId) {
string commandText = "storedProcName";
SqlParameter[] param = new SqlParameter[1]; param[0] = new SqlParameter("@some_id", SqlDbType.Int, 4); param[0].Value = someId;
string returnValue = string.Empty;
using(SqlConnection conn = new SqlConnection(AStaticConnectionString)) { // Using the Micorsoft SqlHelper Object returnValue = SqlHelper.ExecuteScalar( conn ,CommandType.StoredProcedure , commandText , param).ToString(); } return returnValue; }
Free MagazinesGet these publications absolutely FREE for up to 12 months. There are no hidden fees and no obligation. Simply choose a title, complete the application form and submit it. Read more ...
|
|
|