shashank created CAMEL-26327:
--------------------------------
Summary: camel-core - Routing Slip / Dynamic Router: a redelivered
step hangs forever when the route it called used a routing slip or dynamic
router itself
Key: CAMEL-26327
URL: https://issues.apache.org/jira/browse/CAMEL-26327
Project: Camel
Issue Type: Bug
Components: camel-core
Reporter: shashank
Assignee: shashank
A routing slip or dynamic router step that is redelivered by the error handler
never completes when the route it called ran a routing slip or dynamic router
on the same exchange and then failed: the caller waits forever (main
8e0ce2d21371, also 4.22.1 and 4.23.0):
{code:java}
errorHandler(defaultErrorHandler().maximumRedeliveries(1).redeliveryDelay(0));
from("direct:start").routingSlip(constant("direct:sub"));
from("direct:sub")
.doTry()
.routingSlip(constant("mock:inner"))
.throwException(new IllegalStateException("boom"))
.endDoTry()
.doFinally()
.end();
template.send("direct:start", e -> e.getMessage().setBody("in")); // never
returns
{code}
It needs three things: a routing slip or dynamic router step with redelivery, a
called route (direct passes the same exchange) that runs a routing slip or
dynamic router, and an exception that leaves that called route. The exception
can leave through a {{doTry}} without a matching {{doCatch}}, a route with
{{noErrorHandler()}} or a rethrow. A dead letter channel never gets the
message, and with {{onException(...).handled(true)}} the result is the same. A
{{recipientList}} in the called route, or no redelivery, returns as expected.
*Cause*
* {{RoutingSlip.processExchange}} stores the producer of the step in the
exchange property {{CamelSlipProducer}} ({{RoutingSlip.java:424}}). The error
handler that wraps each step does not call that producer directly: it calls
{{RoutingSlipProcessor}}, which reads the producer back from the property on
every attempt ({{RoutingSlip.java:582-583}}). {{DynamicRouter}} extends
{{RoutingSlip}} and shares this code. (The error handler per step comes from
CAMEL-8393; the single error handler around {{RoutingSlipProcessor}} and the
property come from CAMEL-12120, which replaced the cache of error handlers that
leaked in CAMEL-10048.)
* When a step completes, its callback removes the property
({{RoutingSlip.java:429}}). A routing slip in the called route runs on the same
exchange: it overwrites the outer step's producer and then removes it.
* The exception then reaches the outer step's error handler, which redelivers
({{RedeliveryErrorHandler.java:1360}}), and {{RoutingSlipProcessor.process}}
throws {{NullPointerException: "producer" is null}} ({{RoutingSlip.java:583}}).
* The redelivery runs as reactive work: {{DefaultReactiveExecutor}}
({{:205-207}}) logs a WARN ending with "This exception is ignored" and the
callback is never called. The caller parks in
{{DefaultAsyncProcessorAwaitManager.await}} and the exchange stays in flight.
CAMEL-26301 is the same lost callback in the tokenizer EIP.
*Fix* (proposed): {{processExchange}} remembers the producer that was in the
property before it sets its own, and the step's callback puts it back (it
removes the property only when there was none, so nothing is left on the
exchange after the outermost slip, as CAMEL-12120 intended).
{{RoutingSlipProcessor}} also fails the exchange with an
{{IllegalStateException}} and calls the callback when the property is missing,
instead of throwing, so a missing producer cannot turn into a hang again. No
other production code sets or reads {{CamelSlipProducer}}.
Not changed: the nested slip also overwrites the {{CamelSlipEndpoint}} property
of the outer slip, so an outer dynamic router that reads it after such a step
sees the inner slip's last endpoint. That is not a hang and changing it is
visible to routes; it can get its own issue. Also not changed:
{{RedeliveryErrorHandler.RedeliveryTask.redeliver()}} calls the output without
the try/catch that {{RedeliveryTask.run()}} has for the first attempt, so any
asynchronous processor that throws from {{process(exchange, callback)}} on a
redelivery hangs the exchange the same way (on the first attempt the exchange
fails instead). That is wider than this EIP and can get its own issue.
*Tests*: {{RoutingSlipNestedSlipRedeliveryTest}} (camel-core, 15 tests, every
send bounded with {{assertTimeoutPreemptively}}): routing slip and dynamic
router, each calling a route with a routing slip or a dynamic router, with a
{{doTry}} at the top of the called route (default error handler) or
{{noErrorHandler()}} and a dead letter channel; a handled {{onException}}; an
inner step that completes asynchronously; an inner slip step redelivered by the
called route's error handler before the outer step is redelivered; the original
generated route; the property missing (the new {{IllegalStateException}}); and
three controls (a recipient list in the called route, no redelivery, no
property left after nested slips). The 12 non-control tests time out on main
and all pass with the fix. The RoutingSlip, DynamicRouter, redelivery, error
handler and dead letter tests of camel-core pass (388 tests).
*How it was found*: a Rocq specification of Camel's EIPs written from the
documentation, compared with real Camel on generated routes. One generated
route (a routing slip calling a route with nested {{doTry}}/{{doFinally}} that
runs a routing slip, and {{onException}} with redelivery) hung; it was
minimised and traced on HotSpot.
*Affected*: main, 4.23.0 and 4.22.1 reproduced. camel-4.22.x, camel-4.18.x and
camel-4.14.x have the same code (set and remove of the property, and the
unchecked read in {{RoutingSlipProcessor}}).
_Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
