Memory leak inside AsyncSignal / AsyncSignalWaiter

[mario-zelger]

Describe the bug

When calling the Set() Method of an AsyncSignal Object the AsyncSignalWaiter is not correctly disposed:

public void Set()
{
    lock (_syncRoot)
    {
        _isSignaled = true;

        Cleanup();
        
        // If there is already a waiting task let it run.
        if (_waiter != null)
        {
            _waiter.Approve();
            _waiter = null; // Waiter is never disposed

            // Since we already got a waiter the signal must be reset right now!
            _isSignaled = false;
        }
    }
}

When I dispose the AsyncSignalWaiter after the approval inside Set() or inside the Approve() or Cancel() method of the AsyncSignalWaiter no memory leaks can be found (see screenshots).

public void Set()
{
    lock (_syncRoot)
    {
        _isSignaled = true;

        Cleanup();
        
        // If there is already a waiting task let it run.
        if (_waiter != null)
        {
            _waiter.Approve();
            _waiter.Dispose(); // Dispose waiter
            _waiter = null;

            // Since we already got a waiter the signal must be reset right now!
            _isSignaled = false;
        }
    }
}

public void Approve()
{
    _promise.TrySetResult(true);
    Dispose(); // Dispose itself
}

public void Dispose()
{
    _cancellationTokenRegistration.Dispose();

    _promise.TrySetException(new ObjectDisposedException(nameof(AsyncSignalWaiter)));
}

void Cancel()
{
    _promise.TrySetCanceled();
    Dispose();  // Dispose itself
}

Which component is your bug related to?

• Client
• ManagedClient
• RpcClient (unsure)

To Reproduce

Steps to reproduce the behavior:

1. Using version 4.1.3.436 of MQTTnet or the master branch.
2. Run a simple console app with either a Client or ManagedClient (see code below).
3. Run a memory profiler for some time (JetBrains dotMemory in my case)
4. Watch Gen 2 Heap growing and growing.

Expected behavior

The CancellationTokenRegistration inside the AsyncSignalWaiter should always be disposed.

Screenshots

Without disposing the AsyncSignalWaiter

With disposing of the AsyncSignalWaiter

Code example

Simple console app used for memory profiling:

using MQTTnet;
using MQTTnet.Client;
using MQTTnet.Extensions.ManagedClient;
using MQTTnet.Packets;

var clientOptions = new ManagedMqttClientOptionsBuilder()
    .WithAutoReconnectDelay(TimeSpan.FromSeconds(5))
    .WithClientOptions(new MqttClientOptionsBuilder()
        .WithClientId(Guid.NewGuid().ToString())
        .WithTcpServer("localhost")
        .Build())
    .Build();

using var mqttClient = new MqttFactory().CreateManagedMqttClient();
await mqttClient.StartAsync(clientOptions);

var topic = new MqttTopicFilterBuilder()
    .WithTopic("#")
    .Build();
await mqttClient.SubscribeAsync(new List<MqttTopicFilter> { topic });

Console.ReadLine();

await mqttClient.StopAsync();