allthingssecurity opened a new pull request, #29203: URL: https://github.com/apache/camel/pull/29203
# Description [CAMEL-26576](https://issues.apache.org/jira/browse/CAMEL-26576) and [CAMEL-26569](https://issues.apache.org/jira/browse/CAMEL-26569), one PR as they change the same lines of the shared unit of work: the first commit (CAMEL-26569) makes the handover of the on completions thread-safe with parallel `shareUnitOfWork`, the second (CAMEL-26576) makes a part sent to seda hand over only its own on completions. _Claude Code on behalf of allthingssecurity_ ## Problem Since Camel 4.14 (CAMEL-22259) the parts of a Split, Multicast or Recipient List with `shareUnitOfWork` use the parent's unit of work. The 4.14 upgrade guide says the on completions of the parts now run when the parent unit of work is complete. A part that sends to an endpoint which routes a copy on its own, such as `seda` with an `InOnly` exchange, hands over the on completions of its unit of work to the copy (`ExchangeHelper.createCorrelatedCopy(exchange, true, ...)` -> `handoverCompletions`). With the shared unit of work these are all the on completions of the parent exchange, including the commit or rollback of its consumer (`GenericFileOnCompletion`, the SQS delete, ...). They then run when the seda copy is done, with the outcome of the copy: - a later part fails the split, the seda copy succeeds: the consumer commits (the file goes to `done`, not to `moveFailed`; the SQS message is deleted and not retried); - the split succeeds, the seda copy fails: the consumer rolls back (the message is redelivered and all the parts run again). ```java from("file:inbox?move=done&moveFailed=failed") .split(body().tokenize(",")).shareUnitOfWork() .choice() .when(body().startsWith("a")).to("seda:lines") .when(body().isEqualTo("bad")).throwException(new IllegalArgumentException("bad line")) .end() .end(); ``` A file with `a,bad` fails the split but is moved to `done`. This does not need parallel processing. Before 4.14 each part had its own unit of work and handed over only its own on completions. A plain `to("seda:...")` outside a split hands over the exchange's own on completions by design (CAMEL-9291): there the copy is the whole message, and that is unchanged. In a split the copy is one part, whichever part hands over first takes the commit, and the outcome of the split is lost, although the Split docs describe a shared unit of work as one unit that either succeeds or fails. ## Fix - New `ExchangeExtension.setSharedUnitOfWork(UnitOfWork)` (`@since 4.24`, default method: `setUnitOfWork`), used by `MulticastProcessor.prepareSharedUnitOfWork` (Split, Multicast, Recipient List). - `AbstractExchange` remembers the on completions it adds with `addOnCompletion` while it uses the shared unit of work, and `handoverCompletions` hands over only those (`UnitOfWork.handoverSynchronization(target, filter)`). The on completions of the parent stay with the parent and run when the parent is done, with its outcome. On completions added to the unit of work directly (such as by a route `onCompletion`) stay with the parent too. - A part's own on completions (those it added with `addOnCompletion`) are still handed over to its seda copy, as on 4.14 to 4.23 and before 4.14, so they do not change. This differs from the literal sentence of the 4.14 guide on purpose: the copy may still need what they guard. For example, after `pollEnrich("file:...")` in a part, the polled file's commit is the part's own, and moving that file when the split is done could happen before the seda route reads it. Never handing over from a part would be simpler, but it would change those on completions for every version. - The mark goes away when the exchange gets another unit of work (`setUnitOfWork`, such as the correlated copy, which routes on its own and hands over normally), and on `resetExtension` (pooled exchanges). A copy that keeps the shared unit of work keeps the mark. - Upgrade guide entry (4.24). The consumer now commits when the split is done, possibly before the seda copies are processed (as before 4.14), so a copy that still reads the input (such as a file body not loaded yet) must load it first, and a route that must not commit before the asynchronous work is done waits for the copy with `waitForTaskToComplete=Always`. The 4.14 to 4.23 behaviour cannot be configured. - A short paragraph in the "Sharing unit of work" section of the Split EIP docs (and its catalog copy). Not part of this: the resource exchange of `enrich` with `shareUnitOfWork` has always shared the unit of work of the exchange (also before 4.14), and can hand over its on completions the same way (for example to `seda` with `waitForTaskToComplete=Never`). Follow-ups (separate JIRAs): CAMEL-26579 (its own PR): with the pooled exchange factory, releasing a part resets the shared unit of work of the parent (`resetExtension` calls `unitOfWork.reset()`), which can drop the consumer's commit; `enrich` with `shareUnitOfWork` handing over the source's on completions. Target main. A backport to 4.18.x (and 4.14.x) is possible as is (the method is a default method on the internal `ExchangeExtension`), but it changes when the consumer commits, so that is the committers' call. Best opened as one PR together with the thread-safe handover (same lines, same feature). Stacked on the thread-safe handover of the shared unit of work (`camel-core-shared-uow-parallel-handover`): its test `SplitParallelShareUnitOfWorkHandoverTest` asserted that the consumer on completion runs with the seda copy, which is the behaviour changed here. Its seda parts now add their own on completion (handed over to their copies), and the consumer on completion runs with the split exchange. Its races stay reachable: a part still hands over its own on completions while other parts add or hand over theirs (without that fix the test still fails with the `ConcurrentModificationException`). ## Tests `ShareUnitOfWorkSedaHandoverTest`: - file consumer, split with a seda part and a failing part: the file goes to `failed` (on main: `done`); - Split, Multicast, Recipient List and a nested Split, each with a seda part and a failing part: the consumer on completion runs once, `onFailure`, for the parent (on main: `onComplete` for the seda copy); - the seda copy fails, the split succeeds: `onComplete` for the parent (on main: `onFailure` for the copy); - a part's own on completion still goes with its seda copy; a part that sends to seda twice: each copy takes the on completions the part added before that send; - controls that pass on main too: all parts succeed (file in `done`), the same split without the seda part, a plain `to("seda:...")` without a split (still handed over). On main 8 of the 11 tests fail (the 3 controls pass). With the fix all pass, also 20 times in a row, together with `SplitParallelShareUnitOfWorkHandoverTest` (280/280). Mutants: handing over everything (the main behaviour) fails 8 of 11 here and 3 of 3 in `SplitParallelShareUnitOfWorkHandoverTest`; handing over nothing fails the 2 own on completion tests here and 3 of 3 there. With this fix but without the thread-safe handover, `SplitParallelShareUnitOfWorkHandoverTest` still fails 2 of 3 with the `ConcurrentModificationException`. camel-core tests around split, multicast, recipient list, unit of work, seda, enrich, on completion, aggregate, resequence, wire tap, saga, idempotent, throttle, transacted, exchange, file and pooled (2251 tests): the failing classes fail on main too (`SedaConfigureTest` locale, `FileProducerChecksumOfWrittenFileTest` charset, `FilePollEnrichDynamicFileNameTest`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
