[ https://issues.apache.org/jira/browse/ARTEMIS-4646?focusedWorklogId=906093&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-906093 ]
ASF GitHub Bot logged work on ARTEMIS-4646: ------------------------------------------- Author: ASF GitHub Bot Created on: 20/Feb/24 21:06 Start Date: 20/Feb/24 21:06 Worklog Time Spent: 10m Work Description: thezbyg commented on code in PR #4823: URL: https://github.com/apache/activemq-artemis/pull/4823#discussion_r1496507334 ########## tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java: ########## @@ -447,6 +447,59 @@ public void testSendMoreThanUniqueId() throws Exception { publisher.disconnect(); } + @Ignore + @Test(timeout = 600 * 1000) + public void testNoMessageIdReuseBeforeAcknowledgment() throws Exception { Review Comment: Fixed test by using two initial messages. Previously it would not fail as first message had ID 1 and first ID after wraparound was 2. Issue Time Tracking ------------------- Worklog Id: (was: 906093) Time Spent: 1h (was: 50m) > Unacknowledged MQTT message ID is reused after ID generator wraparound > ---------------------------------------------------------------------- > > Key: ARTEMIS-4646 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4646 > Project: ActiveMQ Artemis > Issue Type: Bug > Components: MQTT > Reporter: Albertas Vyšniauskas > Priority: Major > Time Spent: 1h > Remaining Estimate: 0h > > Hello, > MQTT message IDs are currently generated by incrementing *ids* variable in > *{color:#000000}{color:#000000}OutboundStore{color}{color}* 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.{*}{*} -- This message was sent by Atlassian Jira (v8.20.10#820010)