gnodet-bot commented on code in PR #29038:
URL: https://github.com/apache/camel/pull/29038#discussion_r4449375060
##########
components/camel-disruptor/src/main/java/org/apache/camel/component/disruptor/MultipleConsumerSynchronizedExchange.java:
##########
@@ -42,17 +44,23 @@ public MultipleConsumerSynchronizedExchange(Exchange
exchange, int expectedConsu
@Override
public void consumed(Exchange result) {
+ boolean failure = result.getException() != null;
+ if (failure) {
+ // before this consumer is counted, so that the consumer that is
counted last knows of the failure
+ failed.set(true);
+ }
+ int processed = processedConsumers.incrementAndGet();
Review Comment:
๐ก The ordering guarantee here is correct but subtle โ a comment explaining
*why* it's safe would save future maintainers a JMM deep-dive. The
happens-before chain is: `failed.set(true)` โ(program order)โ
`processedConsumers.incrementAndGet()` โ(volatile CAS chain)โ last consumer's
`incrementAndGet()` read, so `failed.get()` in the `else if` is guaranteed to
see the write.
```suggestion
boolean failure = result.getException() != null;
if (failure) {
// Set before incrementing processedConsumers: the volatile CAS
on processedConsumers
// establishes happens-before with the last consumer's CAS on
the same variable,
// so the last consumer's failed.get() is guaranteed to see this
write (JMM ยง17.4.4).
failed.set(true);
}
int processed = processedConsumers.incrementAndGet();
```
--
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]
