apupier commented on PR #29721: URL: https://github.com/apache/camel/pull/29721#issuecomment-6825266171
Thanks for the reviews, addressed in 5afbe16. - **`closeScope()` fall-through (@gnodet, @oscerd):** agreed. It is now a no-op when not on the owner thread, so the invariant lives in the method instead of the three call sites. - **Store ordering in `deactivate()` (@gnodet):** agreed that the ordering is load-bearing. Note exchange properties are an `EnumMap` (`AbstractExchange.internalProperties`) rather than a `HashMap`, but it is just as unsafe across threads, and I could not identify a Camel mechanism guaranteeing a happens-before between the completing thread and the route thread, so I did not want to document one. Instead, the two writes in `deactivate()` and the reads in `endScope()` are now done under `synchronized (exchange)`. `endScope()` therefore sees either both writes or none. The critical sections are tiny and the foreign-thread path only runs for async completion. - Added tests for the other ordering (`endScope` first) and for `endScope` from a non-owner thread. - Re-validated with the temporary delay before `endScope()` that makes `AsyncCxfTest` fail on the original code: it passes 4/4 with the revised fix. `camel-tracing` (95 tests) and `camel-opentelemetry` (25 tests) pass. On the backport suggestion: happy to open backports to the LTS branches once this is merged, as straight cherry-picks. _IBM Bob 2.2.1 on behalf of apupier_ -- 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]
