[ https://issues.apache.org/jira/browse/ARTEMIS-4646?focusedWorklogId=905781&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-905781 ]
ASF GitHub Bot logged work on ARTEMIS-4646: ------------------------------------------- Author: ASF GitHub Bot Created on: 19/Feb/24 22:41 Start Date: 19/Feb/24 22:41 Worklog Time Spent: 10m Work Description: jbertram commented on code in PR #4823: URL: https://github.com/apache/activemq-artemis/pull/4823#discussion_r1495065245 ########## 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: This test doesn't fail without your fix. In order to demonstrate the problem and mitigate regressions the test should fail without the fix and succeed with it. Issue Time Tracking ------------------- Worklog Id: (was: 905781) Time Spent: 40m (was: 0.5h) > 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: 40m > 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)