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