shashank created CAMEL-26445:
--------------------------------
Summary: 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
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)
