Federico Mariani created CAMEL-24074:
----------------------------------------
Summary: camel-jms: TemporaryQueueReplyManager resolves the
temporary reply queue without synchronization - replies can be black-holed
(CAMEL-20769 follow-up)
Key: CAMEL-24074
URL: https://issues.apache.org/jira/browse/CAMEL-24074
Project: Camel
Issue Type: Bug
Components: camel-jms
Affects Versions: 4.21.0
Reporter: Federico Mariani
Attachments: TemporaryQueueReplyManagerConcurrentResolveTest.java
h3. Problem
{{TemporaryQueueReplyManager.TemporaryReplyQueueDestinationResolver.resolveDestinationName}}
(TemporaryQueueReplyManager.java:285-315) is a check-then-act on the plain,
non-volatile {{queue}} field with no synchronization. CAMEL-20769
(7313a1e53a63, May 2024) deliberately removed the previous {{synchronized
(refreshWanted)}} block to fix an endless lock during shutdown - but replaced
it with nothing.
With {{replyToConcurrentConsumers > 1}} (a supported configuration, see
{{JmsRequestReplyTempQueueMultipleConsumersTest}}), several listener-container
invoker threads resolve the destination concurrently at startup and after every
{{scheduleRefresh()}}:
* T1 sees {{queue==null}}, creates Q1; T2 concurrently creates Q2, writes
{{queue=Q2}}, calls {{setReplyTo(Q2)}}; T1's {{setReplyTo(Q1)}} lands last ->
{{replyTo=Q1}} while consumers listen on Q2.
* Once T1's cached consumer on Q1 is recycled, every reply is sent to Q1 and
*black-holed* -> all InOut requests time out until a JMS exception forces a
refresh.
* A mid-refresh {{scheduleRefresh()}} can also be swallowed
({{refreshWanted.set(false)}} before the new queue is fully published).
h3. Reproducer
Attached {{TemporaryQueueReplyManagerConcurrentResolveTest.java}} (place in
{{components/camel-jms/src/test/java/org/apache/camel/component/jms/reply/}}).
It drives the resolver from two threads with a barrier inside
{{createTemporaryQueue()}} making the race deterministic: today two temporary
queues are created (the test expects one). A correctly synchronized resolver
passes - the barrier simply times out for the single caller. No broker needed.
h3. Note on the fix
The CAMEL-20769 shutdown-deadlock motivation must be preserved - synchronize
the resolver on its own dedicated lock (the same pattern QueueReplyManager's
DestinationResolverDelegate uses, with the in-code explanation of why not
BaseService.lock).
----
_This issue was researched and filed by Claude Code on behalf of [~croway]
(GitHub: Croway), as part of a deep code review of camel-jms and camel-amqp. A
failing JUnit reproducer is attached; it fails deterministically on current
main (4.22.0-SNAPSHOT)._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
