Messages not sent by server/ not received by managed mqtt client

[RazvanEmilR]

Describe the bug

On occasion the managed mqttnet client, after connection, can send but won't receive messages.
There might be a server side issue in the code presented below.

Which component is your bug related to?

• ManagedClient
• Server

To Reproduce

Steps to reproduce the behavior:

1. Using this version of MQTTnet 'v4.1.4.563'
2. Make a client connection to the server
3. Disconnect client from the server
   (might need to repeat steps 2 and 3)
4. Managed Mqttnet client does not receive message from the server / Server does not send messaage to managed Mqttnet client

Expected behavior

The Mqttnet client(s) receive the message sent by the server.

Screenshots

Additional context / logging

This code block is from: /.../Source/MQTTnet/Server/Internal.MqttClient.cs

Notice this line and function call:
= Task.Factory.StartNew(() => SendPacketsLoop(cancellationToken), cancellationToken, TaskCreationOptions.PreferFairness, TaskScheduler.Default).ConfigureAwait(false);

IsRunning is set to true after the call of Task.Factory.StartNew()...
What if, in some instances SendPacketsLoop is triggered first and this line inside the function:

try
{
while (!cancellationToken.IsCancellationRequested && !IsTakenOver && IsRunning)
{
still has IsRunning set to false and the "while" loop body never ends up being executed?
Could this be the reason why some messages never end up being sent and thus received by the managed mqttclient?

public bool IsRunning { get; private set; }

public async Task RunAsync()
        {
            _logger.Info("Client '{0}': Session started", Id);

            Session.LatestConnectPacket = _connectPacket;
            Session.WillMessageSent = false;

            try
            {
                var cancellationToken = _cancellationToken.Token;

                _ = Task.Factory.StartNew(() => SendPacketsLoop(cancellationToken), cancellationToken, TaskCreationOptions.PreferFairness, TaskScheduler.Default).ConfigureAwait(false);

                IsRunning = true; // set to true AFTER call of function SendPacketsLoop(..) which uses the value of IsRunning

                await ReceivePackagesLoop(cancellationToken).ConfigureAwait(false);
            }
            finally
            {
                IsRunning = false;

                _cancellationToken?.TryCancel();
                _cancellationToken?.Dispose();
                _cancellationToken = null;
            }
//...
}

async Task SendPacketsLoop(CancellationToken cancellationToken)
        {
            MqttPacketBusItem packetBusItem = null;

            try
            {
               // line below might sometimes execute before IsRunning was set to true, resulting in skipping the while loop
                while (!cancellationToken.IsCancellationRequested && !IsTakenOver && IsRunning)
                {
                    packetBusItem = await Session.DequeuePacketAsync(cancellationToken).ConfigureAwait(false);

                    // Also check the cancellation token here because the dequeue is blocking and may take some time.
                    if (cancellationToken.IsCancellationRequested)
                    {
                        return;
                    }

                    if (IsTakenOver || !IsRunning)
                    {
                        return;
                    }

                    try
                    {
                        await SendPacketAsync(packetBusItem.Packet, cancellationToken).ConfigureAwait(false);
                        packetBusItem.Complete();
                    }
//...
}

[chkr1011]

Thanks for reporting the issue. I agree that the code is wrong. Does it work properly if you move the IsRunning = true; line above the start of the new task?

[RazvanEmilR]

@chkr1011 Glad we can help, in our tests the issue seems to be fixed by doing what you mentioned, namely moving the line above the start of the new task. Did not detect any other issue with that change.