Server disconnect client with same ID when timeout expires

[fogzot]

How to reproduce the problem:

1. Connect from a client with a given ID and that specifies a keep-alive.
2. Make sure the client exits without correctly closing the connection.
3. Re-connect from a client (it was the same client for us) with the same ID.
4. There are now 2 keep-alive services running, one for the old client and one for the new.
5. When the keep-alive of the original (now disconnected) client expires the new client is disconnected.

This happens because the client session is kept in a dictionary and retrieved by ID. The server has no way to know that the old client disconnected (the keep-alive is there exactly for this reason, after all) and when the old keep-alive expires the server retrieves the new session and closes it.

We solved the problem by refusing connections to new clients while the old session is still active because it was the easiest to implement with the current API but we would like to simply discard the old session - unfortunately we didn't find a way. Is there a way to close an existing session from the connecttion validator?

[chkr1011]

Hi,
thank you for reporting this issue and giving proper hints.

I pushed some changes which will restart the KeepAliveMonitor for the affected session. The problem may is that the initial packets for connecting are not affected by the monitor for some logical reasons.

Please let me know if this fixes your issue.

But you can also disconnect every client and cleaning sessions via the DeleteSessionAsync. It is available at the GetClientStatus method of the server.

Best regards
Christian

[fogzot]

Thank you for the quick response. I'll test your changes with our (old) code and let you know what happens.

[lawzla]

Hi,

This is a similar issue i am having with #454, with non-clean client disconnects. Workaround for me has been the client waits the timeout before reconnecting again.

[fogzot]

Yep, I added a link from #454 to here. IMHO, #454 can be closed.

[chkr1011]

Please let me know if it works. I can also build another RC if required.

[fogzot]

@chkr1011 To be completely sure I need some more tests, but the old code seems to work without problems. Thank you very much.

In the new code we use GetClientSessionStatus() and DeleteSessionAsync() to disconnect the old client with the same ID, but to do that correctly I had to make the connection interceptor return Task instead of void and modify the calling code.

Are you interested in a patch to add the option for async interceptors? IMHO, it makes sense to have the option to use an async interceptor (for example if you need to validate the client certificate or some other data on an external data source, like a database).

[fogzot]

I did some more test and I've noted some errors in the MQTTnet logs. I don't know if this is related to the changes but here is the log:

>> [2018-12-07T12:07:34.9852435Z] [13] [MqttServer.MqttClientSessionsManager.MqttClientSession] [Verbose]: Client 'Mxycv3u8l0iiUniTMRZocg:aomRw821c0qVrj-js0c5jw': Connection closed gracefully.
>> [2018-12-07T12:07:34.9853243Z] [13] [MqttTcpServerAdapter.MqttTcpServerListener.MqttChannelAdapter] [Verbose]: Disconnecting [Timeout=00:00:15]
>> [2018-12-07T12:07:34.9859991Z] [13] [MqttServer.MqttClientSessionsManager.MqttClientSession] [Info]: Client 'Mxycv3u8l0iiUniTMRZocg:aomRw821c0qVrj-js0c5jw': Disconnected (clean=False).
>> [2018-12-07T12:07:34.9862997Z] [13] [MqttServer.MqttClientSessionsManager.MqttClientSession] [Error]: Cannot access a disposed object.
Object name: 'MqttChannelAdapter'.
System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'MqttChannelAdapter'.
   at MQTTnet.Adapter.MqttChannelAdapter.ThrowIfDisposed() in /home/fog/Customers/finder/MQTTnet/Source/MQTTnet/Adapter/MqttChannelAdapter.cs:line 232
   at MQTTnet.Adapter.MqttChannelAdapter.DisconnectAsync(TimeSpan timeout, CancellationToken cancellationToken) in /home/fog/Customers/finder/MQTTnet/Source/MQTTnet/Adapter/MqttChannelAdapter.cs:line 77
   at MQTTnet.Server.MqttClientSession.TryDisposeAdapterAsync(IMqttChannelAdapter adapter) in /home/fog/Customers/finder/MQTTnet/Source/MQTTnet/Server/MqttClientSession.cs:line 175
>> [2018-12-07T12:07:34.9868740Z] [13] [MqttServer.MqttClientSessionsManager] [Verbose]: Session for client 'Mxycv3u8l0iiUniTMRZocg:aomRw821c0qVrj-js0c5jw' deleted.

[chkr1011]

@JanEggers I tried to fix the above exception. The problem is that the disconnect/dispose is called twice. One time from the workaround callback and the again in the finally block.

I fixed this in develop branch leaving some comments. Please have a quick review (MqttClientSession.cs) and let me know if you agree with this approach.

[JanEggers]

@chkr1011 looks ok, I tend to use TaskCompletionSource in such cases to let stuff happen just once. but you added a test so as long as its fixed its ok.

[x37v]

@fogzot question: are your disconnected clients still able to publish?
On v2.8.5 I'm experiencing a problem where a managed client reconnects doesn't doesn't receive any messages for its subscriptions but it can publish.
Looking at the verbose server logs it looks like a disconnect happens, and no messages get sent to that client [but there are messages to other clients] but he client thinks it is connected and is able to publish.
I'm wondering if this is the same bug or if I should write up a new one.

[x37v]

@lawzla can your workaround be managed entirely by the client? just set the auto reconnect delay to be greater than the keep alive period?
I'm thinking yes but I'm wanting to make sure I'm talking about the same settings you're talking about.

[fogzot]

@x37v nope, when the old session timeouts, the new client is disconnected and the connection is closed. We solved this by closing the connection and removing the old session when a new client with the same client id connects, as explained above.

[SeppPenner]

@fogzot Does this still apply to the latest version?

[fogzot]

I don't know, we still have the logic to disconnect the old client in place. What changed in the last version and what is supposed to happen? I can easily test if I know what to test for.

[SeppPenner]

Well, I assume you were using version 2.8.5 back then when the issue was created and @chkr1011 did a lot of changes towards the 3.x version. So there might be the chance that this doesn't occur anymore.

[fogzot]

I'll have to write a small test server to check this. I'll do that and let you know.

[chkr1011]

I close this due to inactivity.