[jira] [Resolved] (CAMEL-25612) 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

["Claus Ibsen (Jira)" <ji...@apache.org>]

     [ 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)