Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in captured program #5

Closed
erikvullings opened this issue Mar 22, 2013 · 10 comments
Closed

Memory leak in captured program #5

erikvullings opened this issue Mar 22, 2013 · 10 comments

Comments

@erikvullings
Copy link

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
Copy link
Owner

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//issues/5
.

@bitterskittles
Copy link

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
Copy link
Author

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
Copy link

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
Copy link

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
Copy link

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
Copy link
Author

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

@bitterskittles
Copy link

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
Copy link
Author

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

@justinstenning
Copy link
Owner

The fix for Issue #7 addresses this correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants