Skip to content

[Bug] Fix TOCTOU race condition in MQClientInstance.brokerVersionTable access#10235

Open
Senrian wants to merge 1 commit intoapache:developfrom
Senrian:fix-toctou-race-broker-version
Open

[Bug] Fix TOCTOU race condition in MQClientInstance.brokerVersionTable access#10235
Senrian wants to merge 1 commit intoapache:developfrom
Senrian:fix-toctou-race-broker-version

Conversation

@Senrian
Copy link
Copy Markdown

@Senrian Senrian commented Mar 30, 2026

Bug Description

Issue: #10214

In MQClientInstance.java, brokerVersionTable (a ConcurrentHashMap) is accessed using a non-atomic check-then-act pattern that can cause NullPointerException under concurrent access.

Locations Fixed

  1. sendHeartbeatToBroker() - Race between containsKey() and get()
  2. sendHeartbeatToBrokerV2() - Same issue
  3. findBrokerVersion() - Triple get() calls with potential NPE

Fix Pattern

Use computeIfAbsent for atomic get-or-create:

// Before (race condition):
if (!this.brokerVersionTable.containsKey(brokerName)) {
    this.brokerVersionTable.put(brokerName, new ConcurrentHashMap<>(4));
}
this.brokerVersionTable.get(brokerName).put(addr, version);

// After (thread-safe):
this.brokerVersionTable.computeIfAbsent(brokerName, k -> new ConcurrentHashMap<>(4))
        .put(addr, version);

For findBrokerVersion(), use a local variable to avoid NPE if entry is removed between calls.

Issue: apache#10214

Replace non-atomic check-then-act pattern with thread-safe operations:
- sendHeartbeatToBroker(): use computeIfAbsent instead of containsKey+put
- sendHeartbeatToBrokerV2(): same fix
- findBrokerVersion(): use local variable to avoid NPE if entry removed between calls
@Kris20030907
Copy link
Copy Markdown
Contributor

@Senrian This change is not recommended as it may cause performance issues. Please refer to the documentation:https://juejin.cn/post/7094561581631012878

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants