[
https://issues.apache.org/jira/browse/CAMEL-24074?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen updated CAMEL-24074:
--------------------------------
Fix Version/s: 4.22.0
(was: 4.20.0)
> 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
> Assignee: Claus Ibsen
> Priority: Major
> Labels: code-review
> Fix For: 4.22.0
>
> 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)
