[
https://issues.apache.org/jira/browse/CAMEL-26327?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Work on CAMEL-26327 started by shashank.
----------------------------------------
> 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
> Priority: Major
>
> 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)
