gnodet-bot commented on code in PR #26657:
URL: https://github.com/apache/camel/pull/26657#discussion_r4060748545
##########
core/camel-base-engine/src/main/java/org/apache/camel/impl/engine/DefaultErrorRegistry.java:
##########
@@ -449,7 +455,7 @@ static final class DefaultBacklogErrorEventMessage
implements BacklogErrorEventM
private final String threadName;
private final JsonObject data;
private final Throwable exception;
- private final boolean handled;
+ private boolean handled;
Review Comment:
⚠️ **Visibility gap — `handled` should be `volatile`**
`markHandled()` writes `this.handled = true` from the exchange-processing
thread (step 2 of the dedup sequence). `isHandled()` and `asJSon()` are read
from monitoring threads: `ErrorRegistryConsole.doCall()` calls `entry.asJSon()`
(line 116) and `entry.isHandled()` (line 85, 167) from the dev-console/JMX
thread, and `ManagedErrorRegistry` similarly. The JMM does not guarantee
visibility of a plain-field write across threads —
`ConcurrentLinkedDeque.addFirst()` establishes happens-before only for the
initial safe publication of the entry, not for subsequent mutations to its
fields. Between step 1 (copy's failure captured) and step 2 (fallback fires
`markHandled()`), a monitoring thread can read stale `false`. Declaring
`handled` `volatile` closes the gap at zero functional cost — a one-word fix.
```suggestion
private volatile boolean handled;
```
--
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]
