Is a memory leak created if a MemoryStream in .NET

Question

I have the following code:

MemoryStream foo(){
    MemoryStream ms = new MemoryStream();
    // write stuff to ms
    return ms;
}

void bar(){
    MemoryStream ms2 = foo();
    // do stuff with ms2
    return;
}

Is there any chance that the MemoryStream that I've allocated will somehow fail to be disposed of later?

I've got a peer review insisting that I manually close this, and I can't find the information to tell if he has a valid point or not.

Answer 1

Yes there's a leak, depending on how you define LEAK and how much LATER you mean...

If by leak you mean "the memory remains allocated, unavailable for use, even though you're done using it" and by latter you mean anytime after calling dispose, then then yes there may be a leak, although its not permanent (i.e. for the life of your applications runtime).

To free the managed memory used by the MemoryStream, you need to unreference it, by nullifying your reference to it, so it becomes eligible for garbage collection right away. If you fail to do this, then you create a temporary leak from the time you're done using it, until your reference goes out of scope, because in the meantime the memory will not be available for allocation.

The benefit of the using statement (over simply calling dispose) is that you can DECLARE your reference in the using statement. When the using statement finishes, not only is dispose called, but your reference goes out of scope, effectively nullifying the reference and making your object eligible for garbage collection immediately without requiring you to remember to write the "reference=null" code.

While failing to unreference something right away is not a classical "permanent" memory leak, it definitely has the same effect. For example, if you keep your reference to the MemoryStream (even after calling dispose), and a little further down in your method you try to allocate more memory... the memory in use by your still-referenced memory stream will not be available to you until you nullify the reference or it goes out of scope, even though you called dispose and are done using it.

Answer 2

I would recommend wrapping the MemoryStream in bar() in a using statement mainly for consistency:

• Right now MemoryStream does not free memory on .Dispose(), but it is possible that at some point in the future it might, or you (or someone else at your company) might replace it with your own custom MemoryStream that does, etc.
• It helps to establish a pattern in your project to ensure all Streams get disposed -- the line is more firmly drawn by saying "all Streams must be disposed" instead of "some Streams must be disposed, but certain ones don't have to"...
• If you ever change the code to allow for returning other types of Streams, you'll need to change it to dispose anyway.

Another thing I usually do in cases like foo() when creating and returning an IDisposable is to ensure that any failure between constructing the object and the return is caught by an exception, disposes the object, and rethrows the exception:

MemoryStream x = new MemoryStream();
try
{
    // ... other code goes here ...
    return x;
}
catch
{
    // "other code" failed, dispose the stream before throwing out the Exception
    x.Dispose();
    throw;
}

Answer 3

You won't leak memory, but your code reviewer is correct to indicate you should close your stream. It's polite to do so.

The only situation in which you might leak memory is when you accidentally leave a reference to the stream and never close it. You still aren't really leaking memory, but you are needlessly extending the amount of time that you claim to be using it.

Answer 4

MemorySteram is nothing but array of byte, which is managed object. Forget to dispose or close this has no side effect other than over head of finalization.
Just check constuctor or flush method of MemoryStream in reflector and it will be clear why you don't need to worry about closing or disposing it other than just for matter of following good practice.

Answer 5

I'm no .net expert, but perhaps the problem here is resources, namely the file handle, and not memory. I guess the garbage collector will eventually free the stream, and close the handle, but I think it would always be best practice to close it explicitly, to make sure you flush out the contents to disk.

Answer 6

Disposal of unmanaged resources is non-deterministic in garbage collected languages. Even if you call Dispose explicitly, you have absolutely no control over when the backing memory is actually freed. Dispose is implicitly called when an object goes out of scope, whether it be by exiting a using statement, or popping up the callstack from a subordinate method. This all being said, sometimes the object may actually be a wrapper for a managed resource (e.g. file). This is why it's good practice to explicitly close in finally statements or to use the using statement. Cheers