Screenshot severe memory leak

[remcoros]

Hey,

I don't know if you're still active on this project, but I have been playing with your code and found a severe memory leak.

The Screenshot (MarshalByRefObject) object is never released and it doesn't implement IDisposable.

If capturing a lot of screenshots, memory in the target process keeps growing.

In my case, this resulted in a OOM exception after ~30 seconds of getting consecutive screenshots.

Fix is simple, implement IDisposable, and call RemotingServices.Disconnect(this).

Then, make sure to call Dispose (or the using() construct) in the client application.

public class Screenshot : MarshalByRefObject, IDisposable
{
    private bool _disposed;

    public Screenshot(Guid requestId, byte[] capturedBitmap)
    {
        _requestId = requestId;
        _capturedBitmap = capturedBitmap;
    }

    ~Screenshot()
    {
        Dispose(false);
    }

    /// <summary>
    /// Disconnects the remoting channel(s) of this object and all nested objects.
    /// </summary>
    private void Disconnect()
    {
        RemotingServices.Disconnect(this);
    }

    [SecurityPermissionAttribute(SecurityAction.Demand, Flags = SecurityPermissionFlag.Infrastructure)]
    public override object InitializeLifetimeService()
    {
        //
        // Returning null designates an infinite non-expiring lease.
        // We must therefore ensure that RemotingServices.Disconnect() is called when
        // it's no longer needed otherwise there will be a memory leak.
        //
        return null;

        //var lease = (ILease)base.InitializeLifetimeService();
        //if (lease.CurrentState == LeaseState.Initial)
        //{
        //    lease.InitialLeaseTime = TimeSpan.FromSeconds(2);
        //    lease.SponsorshipTimeout = TimeSpan.FromSeconds(5);
        //    lease.RenewOnCallTime = TimeSpan.FromSeconds(2);
        //}
        //return lease;
    }

    Guid _requestId;
    public Guid RequestId
    {
        get
        {
            return _requestId;
        }
    }

    byte[] _capturedBitmap;
    public byte[] CapturedBitmap
    {
        get
        {
            return _capturedBitmap;
        }
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            if (disposing)
            {
                Disconnect();
            }
            _disposed = true;
        }
    }
}

Hope that helps.

[justinstenning]

Thanks,

Someone else pointed this out also, I just didn't have time to address it
yet.

I plan to do some major overhauls of the project in the coming months and
will incorporate your fix (or if you want to submit a fix To github I will
merge it in).

Cheers,
Justin

On Wednesday, 15 January 2014, Remco Ros wrote:

Hey,

I don't know if you're still active on this project, but I have been
playing with your code and found a severe memory leak.

The Screenshot (MarshalByRefObject) object is never released and it
doesn't implement IDisposable.

If capturing a lot of screenshots, memory in the target process keeps
growing.

In my case, this resulted in a OOM exception after ~30 seconds of getting
consecutive screenshots.

Fix is simple, implement IDisposable, and call
RemotingServices.Disconnect(this).

Then, make sure to call Dispose (or the using() construct) in the client
application.

public class Screenshot : MarshalByRefObject, IDisposable
{
private bool _disposed;

public Screenshot(Guid requestId, byte[] capturedBitmap)
{
    _requestId = requestId;
    _capturedBitmap = capturedBitmap;
}

~Screenshot()
{
    Dispose(false);
}

/// <summary>
/// Disconnects the remoting channel(s) of this object and all nested objects.
/// </summary>
private void Disconnect()
{
    RemotingServices.Disconnect(this);
}

[SecurityPermissionAttribute(SecurityAction.Demand, Flags = SecurityPermissionFlag.Infrastructure)]
public override object InitializeLifetimeService()
{
    //
    // Returning null designates an infinite non-expiring lease.
    // We must therefore ensure that RemotingServices.Disconnect() is called when
    // it's no longer needed otherwise there will be a memory leak.
    //
    return null;

    //var lease = (ILease)base.InitializeLifetimeService();
    //if (lease.CurrentState == LeaseState.Initial)
    //{
    //    lease.InitialLeaseTime = TimeSpan.FromSeconds(2);
    //    lease.SponsorshipTimeout = TimeSpan.FromSeconds(5);
    //    lease.RenewOnCallTime = TimeSpan.FromSeconds(2);
    //}
    //return lease;
}

Guid _requestId;
public Guid RequestId
{
    get
    {
        return _requestId;
    }
}

byte[] _capturedBitmap;
public byte[] CapturedBitmap
{
    get
    {
        return _capturedBitmap;
    }
}

public void Dispose()
{
    GC.SuppressFinalize(this);
    Dispose(true);
}

protected virtual void Dispose(bool disposing)
{
    if (_disposed)
        return;

    Disconnect();
    _disposed = true;
}

}

Hope that helps.

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