[
https://issues.apache.org/jira/browse/CAMEL-25612?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen resolved CAMEL-25612.
---------------------------------
Resolution: Fixed
> camel-sjms - when a session or consumer cannot be created during connection
> recovery, the next attempt reports success without creating any consumer: the
> route stays started and receives nothing
> --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-25612
> URL: https://issues.apache.org/jira/browse/CAMEL-25612
> Project: Camel
> Issue Type: Bug
> Components: camel-sjms
> Reporter: shashank
> Assignee: shashank
> Priority: Major
> Fix For: 4.24.0
>
>
> {{SimpleMessageListenerContainer.initConsumers}} (lines 355-374 at main
> a7169d7c6ab6) assigns the two sets before it fills them:
> {code:java}
> if (consumers == null) {
> 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);
> }
> }
> {code}
> When {{createSession}} or {{createMessageConsumer}} throws, the sets stay:
> empty, or with the consumers created so far.
> During connection recovery this is permanent. {{onException}} (lines 198-204)
> sets both fields to {{null}} and schedules the recovery task;
> {{recoverConnection}} (lines 208-233) calls {{refreshConnection()}} and
> {{initConsumers()}}:
> # attempt n: the new connection is created, {{initConsumers}} assigns the
> sets, one of the create calls fails, the attempt fails ("Failed to recover
> JMS Connection (attempt: n)", WARN) and is repeated after
> {{recoveryInterval}};
> # attempt n+1: {{refreshConnection}} closes that connection (and with it the
> sessions of attempt n) and creates another one; {{initConsumers}} finds
> {{consumers != null}} and creates nothing; {{recoverConnection}} sets
> {{recoverTask = null}} and returns {{true}} ("Successfully recovered JMS
> Connection", DEBUG).
> The container now has a started connection without any consumer. The route is
> started, nothing above DEBUG is logged after the WARN of attempt n, no
> further recovery is scheduled, and no message is consumed until the route or
> the context is restarted. The same container is the reply listener of the
> InOut producer ({{QueueReplyManager}}, {{TemporaryQueueReplyManager}}): there
> the replies are then not received (read in the code, not tested).
> When does a create call fail on a connection that was just created:
> * the connection fails again between {{refreshConnection}} and the end of the
> loop (for example a broker that is still restarting; not reproduced with a
> real broker). The JMS provider then also calls {{onException}}, which sets
> the fields to {{null}} again; the defect needs that call to arrive before
> {{initConsumers}} assigns the sets (it is a race with a narrow window);
> * the connection is fine and the destination or the permission is not there
> yet (for example a broker that accepts connections before its queues are
> deployed, or an authorization that is loaded later; not reproduced with a
> real broker, the test makes the create call fail). No {{onException}}
> follows, so every such failure ends in the state above.
> h3. Reproduction
> New {{SjmsConsumerRecoveryFailedAttemptTest}} with the embedded Artemis
> broker of the module and a connection factory that tracks the connections,
> sessions and consumers of the route and can make one create call fail
> ({{concurrentConsumers=3}}, {{recoveryInterval=200}}). After a simulated
> connection failure the first session (one test) or the last session (another
> test) of the next connection cannot be created; the test waits for the
> following attempt (the third connection) and sends a message. On main both
> tests fail with {{mock://first Received message count. Expected: <1> but was:
> <0>}} ({{mock://last}} for the other one): the third connection has no
> consumer.
> h3. Proposed fix
> {{initConsumers}} closes what it created and sets the two fields back to
> {{null}} when the loop fails, so the next attempt creates all the consumers.
> Two related things in the same two methods:
> * When the listener container fails to start because a consumer cannot be
> created, the session of that consumer stayed open on the connection, which is
> kept for the next start (the sessions that got their consumer are closed by
> {{stopConsumers}}). The same {{catch}} block closes it. A third test covers
> it; on main it fails with {{open sessions after the failed start ==>
> expected: <0> but was: <1>}}.
> * {{onException}} does not schedule a recovery while one is running
> ({{scheduleConnectionRecovery}}, line 242). When it drops the consumers while
> the running attempt creates them, the attempt has to run again. On main this
> mostly happened by accident: the loop added to the fields that
> {{onException}} had set to {{null}}, and the {{NullPointerException}} failed
> the attempt (only an {{onException}} after the last pass of the loop was
> lost). The loop no longer fails that way, so {{recoverConnection}} now
> returns {{false}} (run again after {{recoveryInterval}}) when the consumers
> are {{null}} after {{initConsumers}}. A fourth test covers this; it passes on
> main too, for the reason above.
> No retry loop is added: the attempts are the ones of the existing recovery
> task, {{recoveryInterval}} apart, with one WARN line (no stack trace) per
> failed attempt as before.
> camel-sjms tests with the fix: 138 run, 0 failures, 5 skipped. camel-sjms2
> (it uses the same container) was not run.
> How it was found: a small TLA+ model of the container (connection alive or
> failed, the pending {{onException}} call of the provider, the {{consumers}}
> field, the consumers that exist on the current connection, the steps of one
> recovery attempt). For main TLC reports a state where no recovery is
> scheduled, the connection is alive and it has no consumer: after two
> connection failures (11 states), and after one connection failure and one
> create call that fails on a live connection (9 states). With the fix the
> invariant holds, and "eventually all consumers exist for good" holds for 3
> consumers, 3 connection failures and 2 failing create calls (540 states). The
> model is a sketch of the code and does not cover suspend / resume or the stop
> of the container; the test above is the evidence.
> Affected: main (a7169d7c6ab6; tested). camel-4.23.x, camel-4.22.x and
> camel-4.18.x have the same loop in {{initConsumers}} and the same
> {{recoverConnection}} / {{onException}} (read, not run). On camel-4.14.x the
> loop is the same, but {{recoverConnection}} is different (it returns
> {{false}}; the fix of CAMEL-23646 is not there); not checked further.
> Overlap: the open pull request apache/camel#27173 (CAMEL-16039) changes
> {{onException}} in the same file (it moves the three lines that reset the
> fields into a method). It does not touch {{initConsumers}} or
> {{recoverConnection}}; the two changes merge without a conflict today.
> Duplicate check (2026-10-13): JIRA text "sjms initConsumers" (CAMEL-25312 and
> CAMEL-21705, neither about a failed attempt), "sjms recover connection
> consumers" (none), "SimpleMessageListenerContainer" in camel-sjms
> (CAMEL-25312, CAMEL-24626, CAMEL-22026, CAMEL-16039); GitHub pull requests
> "initConsumers sjms": #27388, #23639, #23641 (the return value of the task),
> none for this.
> _Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
