Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server subscriber sessions hash set can contain sessions with no subscriptions #2062

Open
sidhoda-nortech opened this issue Aug 13, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@sidhoda-nortech
Copy link

Describe the bug

In the server implementation, MqttClientSessionsManager keeps an internal hash set of "subscriber sessions" that have one or more subscriptions.

However, there is a bug that can result in this hash set containing sessions that have no subscriptions. If a subscribe message is intercepted and ProcessSubscription = false is set for all topic filters, then there will be no subscriptions to add. MqttClientSubscriptionsManager.Subscribe will still call MqttClientSessionsManager.OnSubscriptionsAdded with an empty topics list. This method will blindly add the session to _subscriberSessions.

https://github.com/dotnet/MQTTnet/blob/master/Source/MQTTnet/Server/Internal/MqttClientSubscriptionsManager.cs#L215
https://github.com/dotnet/MQTTnet/blob/master/Source/MQTTnet/Server/Internal/MqttClientSessionsManager.cs#L455

As a result, the server will need to check the session for subscriptions for every subsequent application message received which incurs some performance overhead for locking. There may also be other side effects due to this inconsistency.

Which component is your bug related to?

Server

To Reproduce

Steps to reproduce the behavior:

  • Setup application to start up a server to intercept subscribe messages and set ProcessSubscription = false
  • Confirm in debugger that session has been added to _subscriberSessions

Expected behavior

Session should only be added to the _subscriberSessions if there was at least one subscription added. This then also avoids taking the _sessionsManagementLock when there is no work to do and avoids any other potential side effects.

if (addedSubscriptions.Count > 0)
{
    _subscriptionChangedNotification?.OnSubscriptionsAdded(_session, addedSubscriptions);
}
@sidhoda-nortech sidhoda-nortech added the bug Something isn't working label Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant