Memory leak in RefreshAsync while using a CancellationToken?

[ghost]

Hi,

it is me or is RefreshAsync creates a memory leak if using with a CancellationToken? If i use it like the sample (CancellationToken.None) it workes like a charm. But if i pass through the CancellationToken into the function the memory use increase (faster on fast responed devices like Emulators(Nox,Bluestacks) and slowly on normal devices). My Test-Function:

static void Main(string[] args)
{
    var ct = new CancellationTokenSource();
    TestCancellationToken(ct.Token);
    Console.ReadKey();
}

private static async void TestCancellationToken(CancellationToken ct)
{
    var device = AdbClient.Instance.GetDevices().First();

    var framebuffer = AdbClient.Instance.CreateRefreshableFramebuffer(device);

    while (!ct.IsCancellationRequested)
        await framebuffer.RefreshAsync(ct).ConfigureAwait(false);
}

[qmfrederik]

Hi,

I had a quick look at the code and couldn't immediately spot a different code path based on the cancellation token being set (actually, the code never really checks for CancellationToken.None).

Did you run both tests in Debug or Release mode? Do you have an indication of how much memory you see being leaked?

[ghost]

The increase is very slow but look here:

(TestCancellationToken(ct.Token))

(TestCancellationToken(CancellationToken.None))

The Screenshots are both in DEBUG. But i run in Release an got the same Values. I take two memory snapshots (while the memory use increases) and it looks like this:

And it looks like the "SocketExtensions"-Class could be the Problem.

I think the cancellationToken.Register (row 57 in SocketExtensions.cs) is the probleme. Because every call, the event get registered but never deregistered.

The CancellationTokenRegistration instance that can be used to deregister the callback.
(https://msdn.microsoft.com/en-us/library/dd321635.aspx)

Could btw. also a problem in AdbClient.cs row 296. But never test it^^

[qmfrederik]

Great catch, thanks! That does indeed seem to be the problem - CancellationToken.Register returns a CancellationTokenRegistration which should be disposed of, something we currently don't do.

I'm trying to find a Roslyn analyzer that catches these kind of exceptions, so I can add them to the project and make sure these kinds of issues don't come up again. It seems that Microsoft.Maintainability.Analyzers (which includes DoNotIgnoreMethodResults) together with SonarLint (which includes "IDisposables" should be disposed
) should do it, but I'm having some issues setting it up.

I'll keep you posted!

[qmfrederik]

Hi,

I've prepared a PR - #52, and an updated NuGet release, 2.1.0-beta301, that should address this issue.

Can you have a look and let me know if this resolves the issue?

Thanks!

[ghost]

The Memory-Use is now as expected. Perfect :)

But is SonarLint needed for the enduser?

[qmfrederik]

Ah, my bad!

I've pushed 2.1.0-beta303 to NuGet which should fix this.

Thanks!

[ghost]

Perfekt :)

Thx for this