Close - No Dispose - Memory Leak?

  • Thread starter Thread starter darren.pruitt
  • Start date Start date
D

darren.pruitt

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;
}
 
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.

--
Gregory A. Beamer
MVP; MCP: +I, SE, SD, DBA

***************************
Think Outside the Box!
***************************
 
Hello (e-mail address removed),

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 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

--
____________________________________
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.
__________________________________
 
HI Shawn,

Shawn Wildermuth said:
Hello (e-mail address removed),

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.
Click to expand...

You might wonder seeing how much people doesn't think like you and me :-)
 
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
 
Cor Ligthert said:
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).
Click to expand...

Not sure if I follow.

Dispose
is the methode to place your code for disposing unmanaged resources in by
overloading your class.method.
Click to expand...

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.
Click to expand...

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.
Click to expand...

What has GetHashCode to do with Dispose?
Perhaps I've misunderstood you,
 
Miha.
What has GetHashCode to do with Dispose?
Click to expand...
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
 
Cor Ligthert said:
Miha.

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.
Click to expand...

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?
 
Hello Cor Ligthert [MVP],
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
Click to expand...

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.
 
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?
Click to expand...
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,

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.

--
Click to expand...
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/d...ref/html/frlrfsystemidisposableclasstopic.asp

Beneath the table is in the remarks almost the same written as you do. (And
I try to tell).

Cor
 
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;
}
 
Back
Top