Does .NET Framework 1.0 have unmanaged resource leaks?

[Barry Anderberg]

I'm using a tool by Sci-Tech called the .NET Memory Profiler.

We have a massive .NET/C# application here and it has been exhibiting
memory leak behavior for some time. Attempting to remedy the problems
I am employing the aforementioned software in trying to track down the
problems.

After an hour or so of program execution, a snapshot of the managed
heap shows 27,025 BinaryReader objects as having been garbage
collected but never having been disposed.

The .NET Memory Profiler shows that these objects are being allocated
by the constructor of __BinaryParser.

I used another tool called Reflector to look behind the scenes and
sure enough, BinaryReader has resources such as its stream that are
only closed in its Dispose method. The class does not have a
finalizer, nor do any of its parent classes.

Therefore, if BinaryReader.Dispose() is not called, its stream will
never be closed. Granted, the stream in question is passed into the
constructor upon instantiation so the caller can still close the
stream when the time comes, but this isn't how it should be done.

I've seen lots of other classes that aren't being properly disposed
of. The ManualResetEvent isn't disposed of but luckily it has a
finalizer that is called by the GC- of course this alone is bad
programming because it will take longer to free the memory back to the
managed heap since objects that have a finalizer method will require a
minimum of two passes by the GC.

Any comments on this? I'm really getting tired of having to go around
and make sure the .NET Framework isn't making my software bloat over
time.

 

[Daniel O'Connell [C# MVP]]

I'm using a tool by Sci-Tech called the .NET Memory Profiler.

We have a massive .NET/C# application here and it has been exhibiting
memory leak behavior for some time. Attempting to remedy the problems
I am employing the aforementioned software in trying to track down the
problems.

After an hour or so of program execution, a snapshot of the managed
heap shows 27,025 BinaryReader objects as having been garbage
collected but never having been disposed.

The .NET Memory Profiler shows that these objects are being allocated
by the constructor of __BinaryParser.

I used another tool called Reflector to look behind the scenes and
sure enough, BinaryReader has resources such as its stream that are
only closed in its Dispose method. The class does not have a
finalizer, nor do any of its parent classes.

The GC will still clean up the stream when its no longer referenced. It
might take a bit longer but it *will* get cleaned up.

Therefore, if BinaryReader.Dispose() is not called, its stream will
never be closed. Granted, the stream in question is passed into the
constructor upon instantiation so the caller can still close the
stream when the time comes, but this isn't how it should be done.

Actually, it is. If BinaryReader hasn't been disposed, then the underlying
stream wasn't disposed. You should never assume finalization means disposal,
instead finalizers should just do their own unmanaged cleanup and move
along, IMHO. Calling underlying classes dispose methods is a bad idea,
because the runtime may have finalized that class before it finalized you.

I've seen lots of other classes that aren't being properly disposed
of. The ManualResetEvent isn't disposed of but luckily it has a
finalizer that is called by the GC- of course this alone is bad
programming because it will take longer to free the memory back to the
managed heap since objects that have a finalizer method will require a
minimum of two passes by the GC.

The ManualResetEvent where? A class can't dispose itself, someone has to
call Dispose()

 

[Barry Anderberg]

Actually your assertion that the GC will clean up the stream is false.

The classes that derive from stream happen to have finalizers, which the
GC will call if they are not disposed of, however this will take a
minimum of two passes through the GC and it will require that the
finalizer method actually does clean up the unmanaged resources of the
stream.

Second, it is incorrect to assert that if the BinaryReader wasn't
disposed then its underlying stream wasn't disposed. The stream is
passed into the BinaryReader so there is nothing stopping the caller
from disposing of the stream.

What underlying class are you talking about? The stream? You say it's
a bad idea to dispose of an "underlying class" but I have no idea what
you mean by an "underlying class". If you're talking about the stream
that proves my point- the programmer should not have to dispose of the
stream. The BinaryReader does it when it is disposed! If you'll
recall, my original point was that the .NET Framework is making use of a
BinaryReader in __BinaryParser and it IS NOT calling dispose, therefore
the only way the stream is cleaned up is if the caller cleans it up, or
if it is disposed which takes longer- either way, it's bad form. The
..NET Framework should dispose of objects it no longer needs. Read any
document on disposing, finalizing, and garbage collection and the
cardinal rule is that you should dispose of any object that implements
IDisposable.

As to your question about ManualResetEvent, do this.

Create a new C# application in VS.NET. Spawn a thread, and in the
thread's target method, call this.BeginInvoke, passing in the
appropriate arguments. In the method invoked by the delegate, just do
something like this.Text = "hello!"; Make a button on the form that
will spawn the thread each time you click it. Click it a bunch of
times. You will see, if you use .NET Memory Profiler, a
ManualResetEvent object that is not disposed- one for every time you
clicked the button.

The ManualResetEvent makes use of unmanaged resources, and thus it must
be finalized, which requires a minimum of two passes through the GC.
This is BAD PROGRAMMING on the part of Microsoft.

 

[Daniel O'Connell [C# MVP]]

(next time, please include quotes from the orginal message, I don't remember
what I wrote and I lost the message here)

Actually your assertion that the GC will clean up the stream is false.

The classes that derive from stream happen to have finalizers, which the
GC will call if they are not disposed of, however this will take a
minimum of two passes through the GC and it will require that the
finalizer method actually does clean up the unmanaged resources of the
stream.

And how does that prove that the GC won't clean up the stream? A given
stream may or may not have a finalizer, that is a matter of implementation.
Merely having a finalizer and having to be queued for finalization doesn't
make the fact that the GC eventually does do that finalization go away, does
it now? It is possible that data will be lost due to irresponsible disposal,
but the class will be finalized and the memory will be reclaimed. The issues
are different.

Second, it is incorrect to assert that if the BinaryReader wasn't
disposed then its underlying stream wasn't disposed. The stream is
passed into the BinaryReader so there is nothing stopping the caller
from disposing of the stream.

That wasn't what I meant. If you did *NOT* call Dispose on your BinaryReader
reader implemetantion, then the underlying stream wasn't disposed. If the
user chose to do it elsewhere that is his or her choice, but its not
inherently disposed simply because the BinaryReader is finalized.

What underlying class are you talking about? The stream? You say it's
a bad idea to dispose of an "underlying class" but I have no idea what
you mean by an "underlying class". If you're talking about the stream
that proves my point- the programmer should not have to dispose of the
stream. The BinaryReader does it when it is disposed! If you'll
recall, my original point was that the .NET Framework is making use of a
BinaryReader in __BinaryParser and it IS NOT calling dispose, therefore
the only way the stream is cleaned up is if the caller cleans it up, or
if it is disposed which takes longer- either way, it's bad form. The
NET Framework should dispose of objects it no longer needs. Read any
document on disposing, finalizing, and garbage collection and the
cardinal rule is that you should dispose of any object that implements
IDisposable.

That is a cardinal rule, however, that doesn't preclude that the GC will
reclaim memory. Nothing you are showing here constitutes a memory or
resource leak, not even potential data corruption, just an extra GC pass.
Beyond that, if you have read any of those documents you are refering to,
you would know that it is generally not a valid pattern to call
IDisposable:ispose on any object in your finalizer, infact you shouldn't
be accessing any unsealed managed types or any managed types with finalizers
as they may already have been finalized. The finalization queue does not
guarentee any given order for execution so you can't safely call outside of
the current class. As such, finalization does not mean disposal. If you let
one class drift off without disposing it, it will almost never dispose
anything inside of it, period(It'd take a hell of a case to convince me that
it was safe to allow a call to a managed class during finalization, but it
might be possible).
Now, __BinaryParser. I do wonder why so many are hanging around, to be clear
have these been GC'd and the profiler is just telling you how many havn't
been disposed or are all of these instances still in memory?

__BinaryParser is a class that is used in serialization. There does appear
to be some fundamental problems with it(it itself doesn't implement
IDisposable), although I'm hesitant to assume it is a resource problem
rather than a minor bug until I find out more(if the backing stream is a
memory stream or some other form of stream which doesn't use unmanaged
resources, its much less of an issue. Disposing a MemoryStream, for example,
only stops you from changing the backing memory, it doesn't clear that
memory or anything). However, I've examined the most recent version of the
2.0 framework I have and this appears to still exist. I will do some
research on it and see if I can find anything out.

Can you tell me what kind of streams it is using in your situation?

As to your question about ManualResetEvent, do this.

Create a new C# application in VS.NET. Spawn a thread, and in the
thread's target method, call this.BeginInvoke, passing in the
appropriate arguments. In the method invoked by the delegate, just do
something like this.Text = "hello!"; Make a button on the form that
will spawn the thread each time you click it. Click it a bunch of
times. You will see, if you use .NET Memory Profiler, a
ManualResetEvent object that is not disposed- one for every time you
clicked the button.

The ManualResetEvent makes use of unmanaged resources, and thus it must
be finalized, which requires a minimum of two passes through the GC.
This is BAD PROGRAMMING on the part of Microsoft.

Again, this isn't a leak. While its not the greatest thing I've seen, I
seriously doubt its a real issue. I don't suspect you are calling
BeginInvoke often enough(and if you are, I imagine there are other
performance issues in your design that will overshadow it) and events
themselves aren't so scarce that waiting for the GC to get around to freeing
it is such a big deal. Also, luckily, ManualResetEvent will always have a
very small graph attached, you aren't going to be promoting 50k objects,
juts maybe 50 bytes. Its not great but its not as serious as you make it out
to be. However, again, I will look into it and see what I can find out.

 

[Barry Anderberg]

Actually, BinaryReader doesn't have a finalizer!

That's my point. If you don't dispose of it, the stream will NOT be
cleaned up!

__BinaryParser never disposes of its BinaryReader, therefore the user
MUST clean up the stream but in this case the user can't because the
stream is encapsulated within the .NET framework classes that ultimately
use __BinaryParser.

That's a problem!

 

[Daniel O'Connell [C# MVP]]

Actually, BinaryReader doesn't have a finalizer!

That's my point. If you don't dispose of it, the stream will NOT be
cleaned up!

No, the GC *WILL* clean it up. The underlying handle will be freed and sinc
ethis is a BinaryReader, you can't lose any data. Your assertion is
incorrect, as is your logic.

__BinaryParser never disposes of its BinaryReader, therefore the user
MUST clean up the stream but in this case the user can't because the
stream is encapsulated within the .NET framework classes that ultimately
use __BinaryParser.

Again, the GC will handle that. You are blowing something that doesn't
matter way out of proportion. I think you are more interested in finding
problems than anything else, personally.