0

Maybe this is not a good question to post but I am kinda desperate. I have a little piece of code, and i have a memory leak, but i don't know how to overcome it. please help.

        var nPiece = stream.Length / BufferLen;
        var lastPieceLen = stream.Length - (nPiece * BufferLen);

        for (int i = 0; i < nPiece + 1; i++)
        {
            var buffer = new byte[i != nPiece ? BufferLen : lastPieceLen];
            stream.Read(buffer, 0, buffer.Length); 
            using (var chunk = new MemoryStream(buffer))
                SsClient.SendChunk(i != nPiece ? (i + 1).ToString() : ((i + 1) + "last"), SsSession, chunk);
            buffer = null;
            GC.Collect();
        }

I split a large stream into smaller chunks and send them to a WCF service over SsClient.SendChunk() method. Let's say I have a file which is 700 mb, i split it into 7 pieces of 100 mb chunks, and send them one by one. but after this method is done there is around 400 mb in memory which i see as memory leak. when i debug it i see memory gets filled with chunk right after SendChunk method of web service. When the method finished here I'm left with the memory leak. GC.collect() doesn't seem to work either. I didn't even understand why there is 400 mb leftover of 700 mb file? maybe that might give some clues i don't know.

any ideas?

Tolga Evcimen
  • 7,112
  • 11
  • 58
  • 91
  • Perhaps the WCF client is holding on to it. What is the behavior if you create a new client on every iteration? – leppie May 05 '13 at 14:33
  • are you saying that i should create a new client on each iteration? It sounds like a good idea. There is no problem on doing that, I just made it that way because it looked neater to me. – Tolga Evcimen May 05 '13 at 14:35
  • 2
    What do you mean you "see it as a memory leak"? (And why are you not waiting for pending finalizers?) And are you closing the stream? – Eric Lippert May 05 '13 at 14:36
  • I claim that it is a memory leak :) that i don't use that memory. And yes, I am creating the stream in a using(var stream = new FileStream(...)) statement so i believe it gets closed after all. how much does it take to wait for pending finalizers? they don't seem to work in my case :( – Tolga Evcimen May 05 '13 at 14:41
  • Creating and disposing client on each iteration didn't work :/ – Tolga Evcimen May 05 '13 at 14:47
  • If the `buffer` variable is not used in another thread (e.g. maybe in a `BeginWrite` call in the `SsClient.SendChunk` method) then you could just create the buffer once and reuse it for every iteration. – Dirk May 05 '13 at 15:04
  • 1
    `buffer = null; GC.Collect();` are a nonsensical solution to a non-existing problem. You do not have a leak. – H H May 05 '13 at 15:09
  • 1
    What memory usage counter at you looking at? – leppie May 05 '13 at 15:42
  • i know that buffer = null; GC.Collect(); aren't making any sense. If i don't have a leak then what is that 400 mb in ram? before that process it is only 30 mb. I see it when i check memory usage of my application after that job is done. – Tolga Evcimen May 05 '13 at 20:31

1 Answers1

3
    for (int i = 0; i < nPiece + 1; i++)
    {
         var buffer = new byte[i != nPiece ? BufferLen : lastPieceLen];

You don't have a memory leak. You've merely got a bad case of the munchies. You are gobbling up memory in a hurry, allocating the buffer inside the for() loop. It is not subtle either, buffers of a 100 megabytes don't fall from the sky. Any object larger than 85,000 bytes is allocated from the Large Object Heap, not the regular generational GC heap. LOH allocations do not get compacted, they are too large, and do not get collected frequently. It requires a gen #2 collection, they don't happen very often.

And allocating like this has delayed effects, the reason that you don't see GC.Collect() do much to reduce the number. You are also using a lot of RAM. Windows doesn't unmap memory pages until it has to. Usually because another process needs RAM. And you are gobbling up a lot of virtual memory address space. Again the Windows memory manager is not a hurry to de-allocate that space. Which is okay, it is virtual. It doesn't cost anything. It otherwise isn't clear what number you are looking at. The biggest reason that GC.Collect() has little effect is because the MemoryStream still has a reference to the array. Calling its Dispose() method doesn't change that. Last but not least, the jitter removes null assignments to local variables when you build and run the Release version.

A simple workaround is re-use the buffer. Allocate it outside of the for() loop. It will help a lot if you also adjust SendChunk() to take a length argument. And simply reduce the chunk size, 100 megabytes is far too much. This goes over a network in most WCF scenarios, network chunk sizes are typically only 1500 bytes at best. A buffer size of 4096 bytes for I/O is about right in most cases.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • Thank you for your comprehensive answer. I will re-use the buffer as you suggested. But the thing I don't completely understand is what disadvantage would it cause me to use 100 mb of chunks in terms of WCF, or network. And the lenght argument is not so useful in my case here i guess, because I use this approach only when I got files over 100 mb. – Tolga Evcimen May 07 '13 at 07:12
  • The disadvantage is that there is no advantage at all to using so much memory. – Hans Passant May 07 '13 at 08:47
  • I was thinking maybe it would exhaust the WCF service less since there will be less calls while sending for example 4 gb of files. Am I wrong thinking that? – Tolga Evcimen May 07 '13 at 09:46
  • What would exhaust you more: a hundred trips down a hill carrying 10 pounds of rock each or one trip carrying a 1000 pound rock? You are exhausting me by not making a trivial change in your program and trying it. You know everything you need to fix your problem, time to use it. – Hans Passant May 07 '13 at 11:56
  • thanks a lot for your comprehensive answer. I implemented system as you suggested, and it works like a charm. – Tolga Evcimen Jul 04 '13 at 08:53