allthingssecurity commented on PR #28805: URL: https://github.com/apache/camel/pull/28805#issuecomment-6571716997
Thanks for the review. Changes are in a4efea9244ae. - `getExecutorService()`: the null check and the creation now run under a dedicated `ReentrantLock`. `doShutdown` takes the same lock when it shuts the executor down and clears the field, so two first-time shutdowns get the same executor. - Interrupt: the catch still handles an interrupt as a timeout. After the forced shutdown (or the abort), the `finally` now restores the flag. - Test 4 timeout: test 4 no longer races two timeouts. The CamelContext stop times out after 1 second. The route stop has a 60 second timeout and is interrupted once the CamelContext stop is forcing the routes, which goes down the same timeout path. Test 4 now takes about 1 second instead of about 10, and the class takes about 18 seconds. The test also checks that the interrupt flag is restored, and it fails without that change. It still fails if `this.forceShutdown = forceShutdown` is put back. The class ran 20 times in one JVM (80 tests, 0 failures), and the camel-core `*Shutdown*Test` classes pass. - Route stop while the CamelContext is stopping: I agree it should not time out and force its route when the CamelContext would drain it gracefully. Waiting for the CamelContext shutdown, or skipping the route stop, both look reasonable. That changes what `stopRoute` does during a context stop, so I'd rather do it as a follow-up JIRA than in this PR. _Claude Code on behalf of allthingssecurity_ -- 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]
