Is a Memory Leak Created If a Memorystream in .Net Is Not Closed

Is a memory leak created if a MemoryStream in .NET is not closed?

If something is Disposable, you should always Dispose it. You should be using a using statement in your bar() method to make sure ms2 gets Disposed.

It will eventually get cleaned up by the garbage collector, but it is always good practice to call Dispose. If you run FxCop on your code, it would flag it as a warning.

C# MemoryStream leaking memory, after disposing/close/etc?

Another side of the problem is what you are using to determine "memory leak". There are many different ways to measure "free' memory and depending on it you may get tottaly different results.

  • Memory usage show in Task manager - unlikely to go down even when all memory is considered "free" by CLR GC due to the way GC uses memory.
  • GC memory performance counters (and properties) - these ones actually will show GC's view on memory. You want to use them to detect managed memory leaks.

There is one more thing with MemoryStream (and any other large 86K+) allocations - they use Large Objects Heap that will be only collected on full GC, to trigger it you may need to run GC.Collect twice. In normal flow of an application it will happen rare enough, so you may not see this memory freed before application shutdown. To diagnose - check GC collection performance counter (number of GC).

And one more: if you are trying to solve memory leak because you are getting "out of memory" exception it may be caused by address space fragmentation (normally only for 32-bit processes). If it is the case - consider creating your own memory stream that does not allocate memory in single chunk and then have to copy it when growing the stream. Or at least try to preallocate space in the stream.

MemoryStream.Close() or MemoryStream.Dispose()

Close() and Dispose(), when called on a MemoryStream, only serve to do two things:

  • Mark the object disposed so that future accidental usage of the object will throw an exception.
  • Possibly1 release references to managed objects, which can make the GC's job a bit easier depending on the GC implementation. (On today's GC algorithms it makes no real difference, so this is a point for an academic discussion and has no significant real-world impact.)

MemoryStream does not have any unmanaged resources to dispose, so you don't technically have to dispose of it. The effect of not disposing a MemoryStream is roughly the same thing as dropping a reference to a byte[] -- the GC will clean both up the same way.

Which one do I call? Is it necessary to call both?

The Dispose() method of streams delegate directly to the Close() method2, so both do exactly the same thing.

Will the other throw an exception if I have already called one of them?

The documentation for IDisposable.Dispose() specifically states it is safe to call Dispose() multiple times, on any object3. (If that is not true for a particular class then that class implements the IDisposable interface in a way that violates its contract, and this would be a bug.)

All that to say: it really doesn't make a huge difference whether you dispose a MemoryStream or not. The only real reason it has Close/Dispose methods is because it inherits from Stream, which requires those methods as part of its contract to support streams that do have unmanaged resources (such as file or socket descriptors).


1 Mono's implementation does not release the byte[] reference. I don't know if the Microsoft implementation does.

2 "This method calls Close, which then calls Stream.Dispose(Boolean)."

3 "If an object's Dispose method is called more than once, the object must ignore all calls after the first one."

Do I need to close this memory stream?

Since the memory stream is encapsulated in the return value, you should not dispose it. If you do, it will be disposed before the FileStreamResult can access it.

The stream is already disposed inside the FileStreamResultExecutor.ExecuteAsync method, as you can see in the source.

The same is true for ASP.NET MVC, where is it FileStreamResult.WriteFile that does the disposal (source).

MemoryStream leak

There is nothing hugely wrong with that code; note that using is essentially a no-op on a MemoryStream, since it only has managed resources, and managed resources are the domain of the GC; it is normal for the GC not to worry hugely until it makes sense to collect some memory, so I wouldn't stress too much - or: if there is a problem, it probably isn't this.

One observation, though, would be that you can avoid a buffer in the encode step:

var s = Encoding.ASCII.GetString(stream.GetBuffer(), 0, (int)stream.Length);

and actually, I'd be tempted to use UTF8 by default, not ASCII.

A final thought: is your SerializerFactory itself doing something that leaks? For example, are you creating a new XmlSerializer(...) via any of the more complex constructors? The simplest form:

new XmlSerializer(typeof(SomeType));

is fine - it internally caches the internal/actual serializer per-type, and re-uses this for each XmlSerializer instance created this way. However, it does not do this caching for the more complex constructor overloads: it creates and loads a new dynamic assembly each time. And assemblies loaded in this way are never unloaded - so yes, that can cause a memory leak. I would be very keen to see how the serializer instances are created, to see if that is the actual problem. Note that such cases are usually very easy to fix by creating your own serializer-cache in the factory:

public class SerializerFactory
{
// hashtable has better threading semantics than dictionary, honest!
private static readonly Hashtable cache = new Hashtable();
public static IMessageSerializer Create(Type t)
{
var found = (IMessageSerializer)cache[t];
if(found != null) return found;

lock(cache)
{ // double-checked
found = (IMessageSerializer)cache[t];
if(found != null) return found;

var types = new List<Type> { t };
var mapper = new MessageMapper();
mapper.Initialize(types);
var serializer = new XmlMessageSerializer(mapper);

serializer.Initialize(types);

cache[t] = serializer;

return serializer;
}
}
}


Related Topics



Leave a reply



Submit