[
https://issues.apache.org/jira/browse/CAMEL-26445?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Work on CAMEL-26445 started by shashank.
----------------------------------------
> camel-core - RedeliveryErrorHandler: an AsyncProcessor that throws hangs the
> exchange when the error handler has no redelivery (SimpleTask.run has no
> try/catch)
> ----------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-26445
> URL: https://issues.apache.org/jira/browse/CAMEL-26445
> Project: Camel
> Issue Type: Bug
> Components: camel-core
> Reporter: shashank
> Assignee: shashank
> Priority: Major
>
> Follow-up of CAMEL-26430, found while working on it.
> {{RedeliveryErrorHandler}} runs a step through its {{SimpleTask}} when it has
> nothing to do on a failure but give it back: no redelivery, no exception
> policies ({{onException}}), no dead letter channel and no
> {{onPrepareFailure}} processor ({{doStart}}: {{simpleTask = deadLetter ==
> null && !redeliveryEnabled && exceptionPolicies.isEmpty() &&
> onPrepareProcessor == null}}). That is the default error handler of a route
> without {{onException}}, so it is the path of most steps.
> {{SimpleTask.run()}} runs as reactive work
> ({{reactiveExecutor.scheduleMain(task)}} in
> {{RedeliveryErrorHandler.process}}) and calls the processor in
> {{handleFirst()}} ({{processOutput(exchange, this)}}) without a try/catch.
> When the processor is an {{AsyncProcessor}} that throws from
> {{process(exchange, callback)}} instead of setting the exception on the
> exchange, the exception reaches {{DefaultReactiveExecutor}}, which logs it
> ({noformat}Error executing reactive work due to ... This exception is
> ignored.{noformat}) and drops it. The callback is never called, so the caller
> waits forever: a synchronous {{template.send}} or a consumer thread blocks,
> an asynchronous caller never completes.
> {code:java}
> // default error handler, no onException: the error handler uses its
> SimpleTask
> from("direct:start")
> .process(new AsyncProcessorSupport() {
> @Override
> public boolean process(Exchange exchange, AsyncCallback callback) {
> throw new IllegalStateException("thrown"); // instead of
> exchange.setException(...) + callback.done(true)
> }
> })
> .to("mock:result");
> template.send("direct:start", e -> e.getMessage().setBody("Hello")); //
> never returns
> {code}
> On main (0fd7d9abf9c2, 2026-10-31) the send above times out; the same
> processor in a route with a handled {{onException}} or a dead letter channel
> (no redelivery) completes, because those use {{RedeliveryTask}}, whose
> {{run()}} has a try/catch. A plain {{Processor}} that throws does not hang
> either, as its converter to {{AsyncProcessor}} catches the exception and sets
> it on the exchange. CAMEL-26301 (the tokenizer, fixed in
> {{TokenizerProcessor}}) was this path; any other {{AsyncProcessor}} with the
> same bug hangs the same way. CAMEL-26430 fixes the same hang for a redelivery
> ({{RedeliveryTask.redeliver()}}).
> *Proposal*
> * Guard {{SimpleTask.handleFirst()}}: when {{processOutput}} throws before
> the callback was called, set the exception on the exchange and continue as
> when the processor sets the exception and calls the callback ({{run()}} then
> sees the failure and prepares the exchange after the failure, as for any
> failure there). Use the same exactly-once rule as CAMEL-26430: the callback
> and the takeover compete for one state (the number of the call that is
> awaited, in a field of the task), the takeover goes through the reactive
> executor after the work already queued there (so a callback that is already
> queued still wins, as now), a callback that comes after the takeover is
> ignored with a WARN, and an exception thrown after the callback was called is
> left to the reactive executor as now.
> * {{SimpleTask}} is the success path of nearly every step (see CAMEL-25132),
> so the guard must not add an allocation per step: for example a field updated
> with an {{AtomicIntegerFieldUpdater}} in the task (one CAS per step), and
> allocations only when the processor throws.
> * Tests: an {{AsyncProcessor}} that throws from {{process}} with the default
> error handler (the send must complete with the exception), the same in a
> route called from a route with {{onException}} (the called route fails as
> with a plain {{Processor}} that throws: its error handler gives the failure
> back as exhausted, so the caller does not handle it again), asynchronous
> (after an async step), transacted and with pooled exchanges, plus the
> controls of CAMEL-26430 (the callback called before the throw, a callback
> queued through a {{direct}} route, a callback called later from another
> thread).
> * A processor that calls its callback twice: the second call must be ignored
> too (on main it continues the exchange a second time, or with pooled tasks
> fails with a {{NullPointerException}} on the released task), and a stress
> test of a callback called from another thread while the processor throws
> (each exchange continued once).
> * Related, not part of this: a route with {{noErrorHandler()}} does not use
> {{RedeliveryErrorHandler}}, and the same {{AsyncProcessor}} hangs there too.
> * Related, not part of this (a separate follow-up): {{RedeliveryTask.run()}}
> (the first delivery when the error handler has redelivery or an
> {{onException}}) calls the callback directly when the processor throws, so
> that failure skips redelivery, the dead letter channel and {{onException}},
> while a throw on a redelivery goes through them after CAMEL-26430.
> CAMEL-24980 proposes to share the failure handling of the two tasks. Its
> catch has no exactly-once guard either (a callback handed to another thread
> before the throw continues the exchange a second time).
> _Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
