gnodet-bot commented on code in PR #29372:
URL: https://github.com/apache/camel/pull/29372#discussion_r4499127212


##########
core/camel-core-processor/src/main/java/org/apache/camel/processor/aggregate/AggregateProcessor.java:
##########
@@ -1791,10 +1791,15 @@ public void run() {
                 LOG.trace("Recover check cannot start due CamelContext({}) has 
not been started yet", camelContext.getName());
                 return;
             }
+            // a recover check of an earlier start may still run (a stop does 
not wait for it): two checks must not run
+            // at the same time, as they share the exchanges in progress
+            if (!recoveryInProgress.compareAndSet(false, true)) {
+                LOG.debug("Skipping recover check as the previous recover 
check is still running");
+                return;
+            }
             LOG.trace("Starting recover check");

Review Comment:
   ⚠️ **Structural note (non-blocking):** The `compareAndSet` is placed 
*before* the `try` block. If a fatal `Error` (e.g. `OutOfMemoryError`) were 
thrown between the CAS at line 1796 and the `try` entry at line 1801, 
`recoveryInProgress` would remain `true` permanently, disabling all future 
recovery until restart.
   
   In practice this is not a real risk — the only code in that window is a 
constant-string `LOG.trace` call that cannot throw under normal JVM conditions. 
And moving the CAS inside the `try` block would require re-introducing 
`recoveryInProgress.set(true)` semantics, which defeats the 
atomic-compare-and-swap purpose. Current placement is correct.
   
   Just noting it for the record in case a future refactor moves code into that 
window.



-- 
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]

Reply via email to