Major Description
Hello,
MQTT message IDs are currently generated by incrementing ids variable in OutboundStore class and do not perform any additional checks before trusting that it is unique. This works great until one or more MQTT messages are not acknowledged in timely manner while receiving and acknowledging other messages. A different message with the same ID will be sent after 32767 additional messages are received and acknowledged.
There are also more issues with MQTT message ID generation:
- AtomicInteger is used for ids variable, but all accesses and modifications are done in synchronized context, so there is no need for it to be atomic.
- Maximum allowed ids value before wraparound is Short.MAX_VALUE, but MQTT protocol allows values in the range of 1 to 65535 (all unsigned short values except 0).
- Wraparound is done to value 1 and is immediately incremented thus skipping message ID 1 and using 2 as first ID after wraparound.
- There is a harmless but wrong attempt to remove from mqttToServerIds hash map by non-MQTT message ID in the following code: mqttToServerIds.remove(p.getA());. It is harmless because p.getA() returns Long value and will never match any key in mqttToServerIds hash map which uses Integer keys.{}
Attachments
Issue Links
- links to
