CAMEL-24284: Reset recoverTask after successful SJMS connection recovery

[gansheer]

Description

Fix a bug where the SJMS consumer becomes permanently dead after recovering from a JMS connection failure once — any subsequent connection failure cannot trigger recovery.

Root cause

CAMEL-23646 added a !recoverTask.isRunning() guard in scheduleConnectionRecovery() to allow re-scheduling after a completed task. However, BackgroundTask.schedule() sets running=true but never resets it (running is only cleared in waitForTaskCompletion(), which is the run() path, not the schedule() path).

After the first successful recovery:

1. recoverTask is non-null and isRunning() returns true forever
2. The guard recoverTask == null || !recoverTask.isRunning() is permanently false
3. If onException() fires again (transient broker event, Artemis internal cleanup), consumers/sessions are nulled with no recovery path
4. Consumer is permanently dead

Fix

Reset recoverTask and recoverFuture to null (and cancel the scheduled future) after successful recovery. This re-arms the recovery mechanism so subsequent failures can schedule a new recovery task.

Claude Code on behalf of gansheer

[github-actions]

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

• First-time contributors require MANUAL approval for the GitHub Actions to run
• You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
• You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
• Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

[github-actions]

🧪 CI tested the following changed modules:

• components/camel-sjms

---

🔬 Scalpel shadow comparison — Scalpel: 11 tested, 29 compile-only — current: 11 all tested

Maveniverse Scalpel detected 40 affected modules (current approach: 11).

⚠️ Modules only in Scalpel (29)

• apache-camel
• camel-allcomponents
• camel-catalog
• camel-catalog-console
• camel-catalog-lucene
• camel-catalog-maven
• camel-catalog-suggest
• camel-componentdsl
• camel-csimple-maven-plugin
• camel-endpointdsl
• camel-endpointdsl-support
• camel-itest
• camel-jbang-core
• camel-jbang-it
• camel-jbang-main
• camel-jbang-plugin-edit
• camel-jbang-plugin-generate
• camel-jbang-plugin-kubernetes
• camel-jbang-plugin-test
• camel-kamelet-main
• camel-launcher
• camel-report-maven-plugin
• camel-route-parser
• camel-yaml-dsl
• camel-yaml-dsl-deserializers
• camel-yaml-dsl-maven-plugin
• coverage
• docs
• dummy-component

Skip-tests mode would test 11 modules (1 direct + 10 downstream), skip tests for 29 (generated code, meta-modules)

Modules Scalpel would test (11)

• camel-jbang-mcp
• camel-jbang-plugin-mcp
• camel-jbang-plugin-route-parser
• camel-jbang-plugin-tui
• camel-jbang-plugin-validate
• camel-launcher-container
• camel-mllp
• camel-sjms
• camel-sjms2
• camel-yaml-dsl-validator
• camel-yaml-dsl-validator-maven-plugin

Modules with tests skipped (29)

• apache-camel
• camel-allcomponents
• camel-catalog
• camel-catalog-console
• camel-catalog-lucene
• camel-catalog-maven
• camel-catalog-suggest
• camel-componentdsl
• camel-csimple-maven-plugin
• camel-endpointdsl
• camel-endpointdsl-support
• camel-itest
• camel-jbang-core
• camel-jbang-it
• camel-jbang-main
• camel-jbang-plugin-edit
• camel-jbang-plugin-generate
• camel-jbang-plugin-kubernetes
• camel-jbang-plugin-test
• camel-kamelet-main
• camel-launcher
• camel-report-maven-plugin
• camel-route-parser
• camel-yaml-dsl
• camel-yaml-dsl-deserializers
• camel-yaml-dsl-maven-plugin
• coverage
• docs
• dummy-component

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

All tested modules (40 modules)

• Camel :: All Components Sync point
• Camel :: Assembly
• Camel :: Catalog :: CSimple Maven Plugin (deprecated)
• Camel :: Catalog :: Camel Catalog
• Camel :: Catalog :: Camel Report Maven Plugin
• Camel :: Catalog :: Camel Route Parser
• Camel :: Catalog :: Console
• Camel :: Catalog :: Dummy Component
• Camel :: Catalog :: Lucene (deprecated)
• Camel :: Catalog :: Maven
• Camel :: Catalog :: Suggest
• Camel :: Component DSL
• Camel :: Coverage
• Camel :: Docs
• Camel :: Endpoint DSL
• Camel :: Endpoint DSL :: Support
• Camel :: Integration Tests
• Camel :: JBang :: Core
• Camel :: JBang :: Integration tests
• Camel :: JBang :: MCP
• Camel :: JBang :: Main
• Camel :: JBang :: Plugin :: Edit
• Camel :: JBang :: Plugin :: Generate
• Camel :: JBang :: Plugin :: Kubernetes
• Camel :: JBang :: Plugin :: MCP
• Camel :: JBang :: Plugin :: Route Parser
• Camel :: JBang :: Plugin :: TUI
• Camel :: JBang :: Plugin :: Testing
• Camel :: JBang :: Plugin :: Validate
• Camel :: Kamelet Main
• Camel :: Launcher
• Camel :: Launcher :: Container
• Camel :: MLLP
• Camel :: Simple JMS
• Camel :: Simple JMS2
• Camel :: YAML DSL
• Camel :: YAML DSL :: Deserializers
• Camel :: YAML DSL :: Maven Plugins
• Camel :: YAML DSL :: Validator
• Camel :: YAML DSL :: Validator Maven Plugin

---

⚙️ View full build and test results

[gnodet]

Correct and well-analyzed fix for a real bug where BackgroundTask.schedule() sets running=true but never resets it, causing the re-scheduling guard to permanently block after the first successful recovery. The fix properly uses connectionLock and cancel(false) for thread safety.

Highlights:

• The bug analysis in the PR description is excellent: it accurately identifies that BackgroundTask.schedule() sets running=true (line 144 of BackgroundTask.java) but only resets it in waitForTaskCompletion() (line 198), which is only reachable via the run() path, not the schedule() path. The fix of nulling recoverTask so the guard passes via the recoverTask==null path is the correct caller-side workaround.
• Thread safety is properly handled: the fix acquires connectionLock before modifying recoverTask/recoverFuture, consistent with scheduleConnectionRecovery() and other methods in this class. The use of cancel(false) (rather than cancel(true)) is correct since this code runs inside the scheduled task itself — cancel(true) would cause self-interruption.

Minor observations (non-blocking):

1. The existing SjmsConnectionRecoveryTest only triggers onException once, so it passes both with and without this fix. A test that triggers onException twice (with successful recovery between them) would directly cover the bug. Not a blocker given the fix is small and obviously correct.
2. Pre-existing: doStop() at line 264 accesses recoverFuture and recoverTask without holding connectionLock, unlike scheduleConnectionRecovery() and this fix. Mitigated by service lifecycle ordering, but worth noting for future hardening.

Claude Code on behalf of Guillaume Nodet