[
https://issues.apache.org/jira/browse/CAMEL-25095?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen reassigned CAMEL-25095:
-----------------------------------
Assignee: shashank
> camel-jms - InOut: a send that fails after the request timeout or the reply
> has completed the exchange completes it a second time
> ---------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-25095
> URL: https://issues.apache.org/jira/browse/CAMEL-25095
> Project: Camel
> Issue Type: Bug
> Components: camel-jms
> Reporter: shashank
> Assignee: shashank
> Priority: Minor
>
> {{JmsProducer.processInOut}} registers the reply handler in the correlation
> map inside {{MessageCreator.createMessage}}, before
> {{MessageProducer.send()}} runs ({{doSend}}). From that moment two other
> threads can complete the exchange:
> * the request timeout: the timeout checker evicts the handler
> ({{DefaultTimeoutMap.purge}}), and {{onTimeout}} runs
> {{ReplyManagerSupport.processReply}}, which sets an
> {{ExchangeTimedOutException}} and calls {{callback.done(false)}};
> * the reply listener, if the request already reached the broker and was
> answered ({{handleReplyMessage}} removes the handler, then {{onReply}}).
> CAMEL-24073 (4.22.0) added {{replyManager.cancelCorrelationId(...)}} to the
> catch block around the send, so that a send failure does not leave the
> handler behind for the timeout to fire later. But the catch block ignores
> whether the cancel still found the handler, and always rethrows.
> {{JmsProducer.process}} then sets the send exception on the exchange and
> calls {{callback.done(true)}}. If the timeout or the reply removed the
> handler while {{send()}} was still running, the exchange has already been
> completed and routed on, and the send failure completes it a second time.
> When does a send run longer than the request timeout and then fail? A
> blocking send (persistent messages) to a broker that stops answering: the
> Artemis client gives up after {{callTimeout}}, 30 s by default, and the
> default {{requestTimeout}} of camel-jms is 20 s. Producer flow control or a
> network partition can do the same. (An ActiveMQ Classic synchronous send
> without {{sendTimeout}} blocks forever, so it does not hit this.) The second
> variant needs no timeout: the request reaches the broker and is answered, but
> {{send()}} then throws, for example because the acknowledgement of the send
> is lost. Then the reply and the send failure both complete the exchange. When
> the broker degrades, every in-flight request can be affected at once.
> Effect on a route
> {{from("direct:start").doTry().to("jms:queue:req?requestTimeout=500").doCatch(Exception.class)...}}:
> * the exchange completes on the timeout thread: the catch block runs with
> {{ExchangeTimedOutException}}, {{ExchangeCompleted}} is emitted and the
> on-completions of the exchange run as {{onComplete}};
> * then the send failure completes it again on the caller thread:
> {{ExchangeFailed}} is emitted, the same on-completions run again as
> {{onFailure}}, and the caller gets the send exception although the catch
> block handled the failure;
> * the inflight repository removes the exchange twice, so
> {{getInflightRepository().size()}} went to -1, -2, -3 after three such
> requests. A negative inflight count can make a graceful shutdown stop waiting
> for exchanges that are really inflight (not tested).
> The test shows the double run for an on-completion added to the exchange.
> When the route starts at a consumer, the consumer's own on-completions (such
> as the commit or rollback of the consumed message) are registered the same
> way, so they would run twice as well, first as success and then as failure
> (not tested).
> Affected: 4.22.0 and later in this form. Before 4.22.0 every send failure
> after the registration completed the exchange twice, whether or not the
> timeout came first (CAMEL-24073, not backported, so 4.18.x still has that
> broader form).
> h3. Reproduction
> An embedded Artemis broker (in-VM) and a {{JmsComponent}} whose
> {{ConnectionFactory}} is wrapped in a dynamic proxy, so that
> {{MessageProducer.send()}} to the request queue sleeps 1500 ms and then
> throws a {{JMSException}}. {{requestTimeout=500}},
> {{requestTimeoutCheckerInterval=100}}.
> * Producer level ({{endpoint.createAsyncProducer().process(exchange,
> callback)}}): the callback is called twice in 3 of 3 runs: {{done(false)}}
> after about 550 ms on a {{JmsReplyManagerOnTimeout}} thread with
> {{ExchangeTimedOutException}}, then {{done(true)}} after about 1520 ms on the
> caller thread with {{UncategorizedJmsException}}.
> * Route level (the route above): 3 of 3 runs show {{onComplete}} and then
> {{onFailure}} for the same exchange, the caller sees
> {{UncategorizedJmsException}}, and the inflight count is -1, -2, -3.
> * Variant: the proxy sends the message first, then sleeps 700 ms and throws.
> The reply completes the exchange ({{done(false)}} after about 10 ms), then
> the send failure completes it again ({{done(true)}} after about 715 ms), 3 of
> 3 runs.
> * Controls: a send that fails at once (the CAMEL-24073 case) and a slow send
> that succeeds both complete the exchange exactly once.
> A TLA+ model of the producer thread, the timeout checker, the reply listener
> and the broker checks that the callback is called at most once. It is
> violated on the current code by the orders Register, SendFail, Expire, Evict,
> OnSendFailure, TimeoutRun and Register, Reach, Answer, SendFail, ReplyRecv,
> OnSendFailure. The model reproduces CAMEL-24073 on the code before that fix,
> and the fixed model holds the property and also completes every exchange.
> h3. Proposed fix
> The party that removes the handler from the correlation map owns the
> completion of the exchange. The removal is already atomic in all three paths
> (timeout eviction, reply, cancel), so the cancel only has to report it:
> * {{ReplyManager.cancelCorrelationId}} returns {{true}} if it removed a
> pending handler, {{false}} otherwise.
> * In the catch block of {{processInOut}}: if a handler was registered and
> {{cancelCorrelationId}} returns {{false}}, the timeout or the reply completes
> the exchange ({{onTimeout}} may still be queued on the timeout thread pool).
> Log the send failure at WARN and return {{false}} without touching the
> exchange. Otherwise rethrow as today, which keeps the CAMEL-24073 behaviour.
> When the reply manager stops, its correlation map evicts every pending
> handler and completes those exchanges with a {{RejectedExecutionException}},
> so a {{false}} from the cancel means that another thread completes the
> exchange. (The one exception already exists today: an {{onTimeout}} task that
> the timeout thread pool rejects because it is shutting down.)
> {{cancelCorrelationId}} was added to the public {{ReplyManager}} interface in
> 4.22.0, so the return type change needs one line in the 4.23 upgrade guide.
> Tests: deterministic cases in {{JmsInOutSendFailureCallbackTest}} (the
> CAMEL-24073 test), with a latch in the proxied send instead of sleeps: the
> timeout completes the exchange during a send that then fails (with and
> without a {{doCatch}}), and the reply completes it during a send that then
> fails. Each asserts one {{onComplete}} or one {{onFailure}}, the outcome of
> the timeout or the reply, and an inflight count of 0.
> Duplicate check (2026-09-28): JIRA component camel-jms since June 2025,
> including CAMEL-24073, CAMEL-24074, CAMEL-24124, CAMEL-24401 and the review
> umbrella CAMEL-24078 with its findings list: none covers a timeout or a reply
> during the send. GitHub PRs for {{JmsProducer}} and {{cancelCorrelationId}}:
> apache/camel#24730 (CAMEL-24073) only.
> _Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
