gnodet-bot commented on code in PR #26996:
URL: https://github.com/apache/camel/pull/26996#discussion_r4124284963
##########
components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsProducer.java:
##########
@@ -262,8 +262,16 @@ public Message createMessage(Session session) throws
JMSException {
try {
doSend(exchange, true, destinationName, destination,
messageCreator, messageSentCallback);
} catch (Exception e) {
- // send failed after reply was registered, cancel to prevent
double callback on timeout
- replyManager.cancelCorrelationId(registeredCorrelationId[0]);
+ // send failed after the reply was registered: cancel it to
prevent a second callback from the timeout
+ String registered = registeredCorrelationId[0];
+ if (registered != null &&
!replyManager.cancelCorrelationId(registered)) {
Review Comment:
⚠️ **`useMessageIDAsCorrelationID` gap:** `registeredCorrelationId[0]` holds
the provisional correlation ID assigned at line 246. When
`useMessageIDAsCorrelationID=true`, the
`UseMessageIdAsCorrelationIdMessageSentCallback.sent()` callback runs _before_
`commitIfNecessary()` inside `doSendToDestination` and moves the handler to the
real JMS message ID via `updateCorrelationId()`. If the commit then fails, this
cancel uses the stale provisional ID → returns `false` → the code thinks the
timeout/reply completed the exchange, but the handler is actually orphaned
under the real JMS message ID.
As @davsclaus noted: update `registeredCorrelationId[0]` to the new JMS
message ID after `sent()` runs (e.g. in the callback itself, or after `doSend`
returns), and add a test with `useMessageIDAsCorrelationID=true` and a failing
commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
