daguimu opened a new pull request, #10215: URL: https://github.com/apache/rocketmq/pull/10215
## Problem `MQClientInstance.brokerVersionTable` is accessed using non-atomic check-then-act patterns in three locations. Between `containsKey()` and `get()`, another thread could remove the entry, causing a NullPointerException. ## Root Cause The `sendHeartbeatToBroker()`, `sendHeartbeatToAllBrokerV2()`, and `findBrokerVersion()` methods use separate `containsKey`/`put`/`get` calls on a `ConcurrentHashMap`, which are individually thread-safe but not atomic as a compound operation. A concurrent removal between these calls leads to NPE. Notably, a similar operation in `HeartbeatTask.execute()` (line ~814) already uses the correct `computeIfAbsent` pattern. ## Fix - **`sendHeartbeatToBroker()`**: Replace `containsKey`/`put`/`get` with `computeIfAbsent` for atomic put-if-absent and value retrieval in one step. - **`sendHeartbeatToAllBrokerV2()`**: Same fix as above. - **`findBrokerVersion()`**: Cache the result of `get()` in a local variable before dereferencing, eliminating the TOCTOU window. ## Tests Added | Change Point | Test | |-------------|------| | `findBrokerVersion` with missing broker name | `testFindBrokerVersionWhenBrokerNameNotExist()` — verifies no NPE when broker name is absent | | `findBrokerVersion` with missing broker addr | `testFindBrokerVersionWhenBrokerAddrNotExist()` — verifies no NPE when addr is absent | | `findBrokerVersion` after concurrent removal | `testFindBrokerVersionConcurrentRemoval()` — simulates removal between check and get | | `computeIfAbsent` atomicity for heartbeat paths | `testBrokerVersionTableComputeIfAbsent()` — verifies atomic creation and idempotency | ## Impact Only affects `brokerVersionTable` access in `MQClientInstance`. No behavioral change for normal operation — only eliminates potential NPE under concurrent broker registration/removal. Fixes #10214 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
