[cgrs5722]

Description

GatewayService.assetIdGatewayIdMap is a plain HashMap that is written under synchronized in processGatewayChildAssetChange, but read without any synchronization from:

• onAttributeEventIntercepted (get, attribute event interceptor thread)
• deleteGateway (entrySet().stream() while writes can happen concurrently)
• getLocallyRegisteredGatewayId (get, called from the Camel route predicate)

This is a data race: unsynchronized reads against synchronized writes on a HashMap can throw ConcurrentModificationException while streaming the entry set, or observe inconsistent state.

The fix switches the map to ConcurrentHashMap, consistent with the adjacent gatewayConnectorMap and tunnelInfos fields.

Fixes #3449.

Verification

• ./gradlew :manager:compileJava passes (JDK 21).
• A standalone probe replicating the exact access pattern (one thread doing synchronized put/remove as in processGatewayChildAssetChange, another thread doing get + entrySet().stream().filter(...) as in deleteGateway) fails with ConcurrentModificationException on HashMap within ~1000 reads, while ConcurrentHashMap completes hundreds of thousands of operations without error.
• The integration tests in the test project (e.g. GatewayTest) require a running PostgreSQL + Keycloak stack via docker compose, which I don't have available locally.

🤖 Generated with Claude Code

[CLAassistant]

All committers have signed the CLA.

[richturner]

Hi @cgrs5722 thanks for picking this up, by switching to ConcurrentHashMap any existing uses of synchronized on the map can also be removed as described in the linked issue.

[richturner]

@cgrs5722 would be great to get this finished off, otherwise we'll have to close this off.

[wborn]

This is an additional AI-assisted review.

Switching to ConcurrentHashMap addresses the concurrency issue described in #3449, but introduces a regression when looking up a null asset ID, causing the integration tests to fail during application setup. See the inline comment for details.

Also, as already noted by @richturner, the synchronized blocks in processGatewayChildAssetChange() (lines 928–935) can now be removed. Both operations are already thread-safe with ConcurrentHashMap, so the switch can be simplified to:

switch (persistenceEvent.getCause()) {
  case CREATE, UPDATE -> assetIdGatewayIdMap.put(childAsset.getId(), gatewayId);
  case DELETE -> assetIdGatewayIdMap.remove(childAsset.getId());
}

Please address the null handling, remove the redundant synchronization, and rerun the integration tests.

[cgrs5722]

Thanks for the detailed review! Both points are addressed in the latest commit:

• getLocallyRegisteredGatewayId() is now null-safe, preserving the previous HashMap behaviour of falling back to parentId
• the redundant synchronized blocks in processGatewayChildAssetChange() have been removed

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[richturner]

Thanks for making the final updates

[wborn]

Thanks for the fix!