Memory leak in captured program

[erikvullings]

Dear Spazzarama,

I've noticed that, when capturing VLC, the memory footprint of VLC remains stable, until I start capturing screenshots. In that case, VLC's memory consumption will quickly reach > 1 Gb (on Windows 8). How can I prevent this from happening?

Cheers, Erik

[justinstenning]

Is this using the code as is in the project or with some other changes? Either way something isn't being disposed of (probably a bitmap)..

Cheers, J

On Friday, March 22, 2013, erikvullings wrote:

Dear Spazzarama,

I've noticed that, when capturing VLC, the memory footprint of VLC remains stable, until I start capturing screenshots. In that case, VLC's memory consumption will quickly reach > 1 Gb (on Windows 8). How can I prevent this from happening?

Cheers, Erik

— Reply to this email directly or view it on GitHubhttps://github.com/spazzarama/Direct3DHook/issues/5 .

[bitterskittles]

There might be 2 reasons for this:

1. Managed memory / garbage collection: In this case, it is the normal behavior of the .NET garbage collector. GC should free up the unused memory when the memory pressure increases. You could call GC.Collect() after Interface.SendScreenshotResponse() to force a garbage collection, but it would degrade the performance and it's not recommended for managed memory anyway. It is better to leave GC alone in most cases :)
2. Large object heap fragmentation This may happen with the current BaseDXHook implementation, if MemoryStream.ToArray() method in ReadFullStream returns more than 85000 bytes as result. Eventually, this leads to uncollected managed memory in the large object heap since LOH doesn't get defragmented during garbage collection pre .NET 4.5.

To fix this, you could replace MemoryStream with an array/list of byte arrays that are 85000 bytes or less in size, so the arrays would never get allocated as Gen 2 objects in LOH.

Alternatively you could change the target platform to .NET 4.5, since LOH allocation/collection is handled better in 4.5, but I wouldn't recommend this without profiling the runtime behavior, especially if LOH memory is allocated frequently by the app.

some links about LOH and memory management: http://msdn.microsoft.com/en-us/magazine/cc534993.aspx https://www.simple-talk.com/dotnet/.net-framework/the-dangers-of-the-large-object-heap/ https://blogs.msdn.com/b/dotnet/archive/2011/10/04/large-object-heap-improvements-in-net-4-5.aspx

[erikvullings]

Yes, I use your own test program, while playing a movie file on VLC. After doing a loadtest, capturing 100 images, VLC's memory footprint is increased from 28 -> 700Mb. I also assume it's the bitmap, but how can I dispose it properly inside your code.

[bitterskittles]

I've attached capture.dll to Microsoft DirectX SDK (February 2010)\Samples\C++\Direct3D\Bin\x86\SimpleSample.exe, and profiled the memory usage with ANTS Memory Profiler, and found 2 reasons for the managed memory leaks

1. Excessive LOH utilization due to large byte[] Capture.Interface.Screenshot.CapturedBitmap byte arrays end up in LOH but they can be initialized in SOH by replacing BaseDXHook.ReadFullStream method with something similar to this:

   protected static IEnumerable<byte[]> ReadFullStream(Stream stream)
   {
      while (true)
      {
          // magic number to avoid allocating the array in LOH
          // for 32bit process: 84987
          // for 64bit process: 84975
          var buffer = new byte[84975];
          int read = stream.Read(buffer, 0, buffer.Length);
          if (read == buffer.Length)
          {
              yield return buffer;
          }
          else if (read < buffer.Length)
          {
              var temp = new byte[read];
              Array.Copy(buffer, 0, temp, 0, read);
              yield return temp;
              yield break;
          }
      }
   }

   Large screenshot arrays may not be the issue, since it won't cause LOH fragmentation if the arrays are always the same size. I wouldn't bet on this and split the arrays just in case something else ends up in LOH while allocating memory for the screenshots and causes fragmentation.

   https://www.simple-talk.com/dotnet/.net-framework/the-dangers-of-the-large-object-heap/

2. Remoting object lifetime management issues (dangling ServerIdentity objects) https://nbevans.wordpress.com/2011/04/17/memory-leaks-with-an-infinite-lifetime-instance-of-marshalbyrefobject/

   Apparently, remoting creates a ServerIdentity instance for every MarshalByRef object that gets proxied over the remoting channel, and 100 screenshot calls result in 100 ServerIdentity instances keeping 100 Capture.Interface.Screenshot instances alive in memory. Haven't really investigated beyond a simple google search, but RemotingServices.Disconnect() should be called for Capture.Interface.Screenshot after Interface.SendScreenshotResponse() in BaseDXHook.ProcessCapture() to release ServerIdentity instances that hold references to Capture.Interface.Screenshot instances

[bitterskittles]

Update to ServerIdentity leak: RemotingServices.Disconnect(Screenshot) should be called from Capture.dll once TestScreenshot.exe finishes processing the marshalled screenshot instance, so it becomes a bit hairy to implement the fix.

Instead of calling RemoteServices.Disconnect and dealing with a callback to callback to callback situation, I figured it is just easier to refactor CaptureInterface.SendScreenshotResponse(Screenshot screenshot) to CaptureInterface.SendScreenshotResponse(Guid requestId, byte[][] screenshot), and drop MarshalByRefObj base from Screenshot and use Screenshot only from the TestScreenshot.exe side.

[bitterskittles]

I decided to not change CaptureInterface.SendScreenshotResponse. Screenshot gets binaryserialized when it crosses appdomains, and it doesn't have any methods so it doesn't have to be MarshalByRefObj. So I'll drop the base class from ScreenShot and add SerializableAttribute instead

[erikvullings]

Thanks for the fix! I've just tested it, and it works really nice!

[bitterskittles]

Ah, no no no don't use the code I posted to fix leaks caused by ServerIdentity instances :)

Just remove MarshalByRefObj inheritance from Screenshot class, and then add [Serializable] attribute to it. It is simpler because Screenshot class is immutable and doesn't expose any public methods, so it doesn't necessarily need to be proxied over Remoting.

As for the LOH fragmentation: I did another test after I made those two changes to Screenshot, and there weren't any LOH issues. The byte arrays for the screenshot are allocated and deallocated one after another, and Capture.dll doesn't allocate memory in LOH anywhere else, so LOH doesn't get fragmanted because these screenshot byte arrays.

[erikvullings]

OK, that worked smoothly too! Thanks again!!!

[justinstenning]

The fix for Issue #7 addresses this correctly.