Replay and retained message logic need work (biggest issue: inactive client sessions grow RAM forever)

[shipmints]

Looks like this wasn't a fork but a copy of the repo. The outstanding issues won't have carried over so carrying over this reference from https://github.com/beerfactory/hbmqtt/issues/237

Session Message Replay Issues

This is a spot that is broken and currently (and correctly) disabled for qos 0 messages. See https://www.hivemq.com/blog/mqtt-essentials-part-7-persistent-session-queuing-messages/ for persistent session discussion where only QOS 1/2 should be retained for session-reconnect replay.

Here when a client has an active connection to the broker, it will be sent the pending message:

https://github.com/beerfactory/hbmqtt/blob/07c4c70f061003f208ad7e510b6e8fce4d7b3a6c/hbmqtt/broker.py#L709

When an old client session is recorded as having disconnected, every single message is enqueued waiting for the client to reconnect. If the client never reconnects, memory is consumed forever.

https://github.com/beerfactory/hbmqtt/blob/07c4c70f061003f208ad7e510b6e8fce4d7b3a6c/hbmqtt/broker.py#L719-L726

• The logic assumes that when the client reconnects (assuming it's the same client by client id), it will be a dirty connection and will want the same subscriptions as it did the first time. I feel this is erroneous as one can only know the client's intentions after it has established its subscriptions for a clean session or stated clearly that it wants a non-clean, reused session.
• The logic should not bother to retain session replay messages for anonymous client sessions. This is a related bug: If a client does not specify a client id, a random UUID is generated and that is stored forever in the broker session cache. This id (and session) will never be reused by the same client which will always send empty client id forcing a new random id to be created. The unreachable session will collect messages for a client connection that can never be reestablished, further exhausting memory.
• Only messages with QOS 1 or QOS 2 should be put into this queue (hence the QOS 0 suppression in the current patched logic).
• Configurable caps could be available to control the broker's behavior for the retained message queue either by: total bytes, number of messages, number of topics, list of specific topics.
• The session collection of these messages probably should not be named retained_messages since its confusing. These are for high QOS guaranteed-delivery attempts.

Retained Topic Message Issues

Persistent topic subscription restoration looks like it needs work to ensure retained messages are provided when a non-clean session is restored the list of session topics is not reprocessed through the retained topic replay logic as it does when a new subscription is made.

Client session retrieved from cache but does not correctly reestablish topic subscriptions and will miss retained messages for those topics upon reconnection:

https://github.com/beerfactory/hbmqtt/blob/07c4c70f061003f208ad7e510b6e8fce4d7b3a6c/hbmqtt/broker.py#L381-L387

Here is where the logic for replaying the retained topic cache for new subscriptions is that should be reused for sessions pulled from the session cache:

https://github.com/beerfactory/hbmqtt/blob/07c4c70f061003f208ad7e510b6e8fce4d7b3a6c/hbmqtt/broker.py#L473-L480

Configurable caps could be available to control the broker's behavior for the topic retained messages similar to those proposed for the session message cache, above.

[FlorianLudwig]

@shipmints with the first (pre) release out the door I finally got around to look into this.

First of all, thank you for your in depth analysis!

To me it looks like this is more than one issue to tackle.

A proper solution will needs architectural changes. As a first step I separated the anonymous session issue into it's own ticket: https://github.com/Yakifo/amqtt/issues/27

Overall, are you going to work on this issue?

[shipmints]

As time is free in my schedule, I will aim to contribute. If other people want to attempt work in the areas mention above, I'm also happy to assist.