allthingssecurity commented on code in PR #27799:
URL: https://github.com/apache/camel/pull/27799#discussion_r4267168694
##########
components/camel-sjms/src/main/java/org/apache/camel/component/sjms/consumer/SimpleMessageListenerContainer.java:
##########
@@ -358,14 +365,31 @@ protected void initConsumers() throws Exception {
if (consumers == null) {
LOG.debug("Initializing {} concurrent consumers as JMS
listener on destination: {}", concurrentConsumers,
destinationName);
- sessions = new HashSet<>(concurrentConsumers);
- consumers = new HashSet<>(concurrentConsumers);
- for (int i = 0; i < this.concurrentConsumers; i++) {
- Session session = createSession(connection, endpoint);
- MessageConsumer consumer = createMessageConsumer(session);
- configureConsumer(consumer, session);
- sessions.add(session);
- consumers.add(consumer);
+ Set<Session> newSessions = new HashSet<>(concurrentConsumers);
+ Set<MessageConsumer> newConsumers = new
HashSet<>(concurrentConsumers);
+ sessions = newSessions;
+ consumers = newConsumers;
Review Comment:
Thanks, I read the fix again against this and it holds: `onException` nulls
both fields under `connectionLock`, and
the check in `recoverConnection` reads `consumers` under the same lock after
`initConsumers` returns, so a null set
during the loop is seen and the attempt returns `false`. If the loop fails,
the `catch` nulls the fields itself and
the exception reaches `recoverConnection`. Done in 902cc2b5d447: your
comment is above the two assignments, as suggested. No code change. camel-sjms:
138 run, 0 failures, 5 skipped.
On point 2 of the review (an `onException` for the new connection before
`initConsumers` assigns the sets): agreed
that the signal is lost there if the provider creates sessions and consumers
without noticing. I would do the failure
counter as a follow-up, with its own test, rather than in this approved PR.
_Claude Code on behalf of allthingssecurity_
--
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]
