Fix S2095 resource leaks and wrap HttpClient for AutoCloseable compatibility

[gnodet]

Summary

Claude Code on behalf of Guillaume Nodet

Fix SonarCloud S2095 (Resources should be closed) across multiple modules. This PR addresses three categories:

Real resource leak fixes

• UpdateCamelReleasesMojo.java: Wrapped LineNumberReader in try-with-resources in processReleases()
• AudioTranscriptionRequestHandler.java: Close InputStream from HttpExchange.getRequestBody() using try-with-resources
• EmbeddingRequestHandler.java: Close InputStream from HttpExchange.getRequestBody() using try-with-resources
• RequestHandler.java: Close InputStream from HttpExchange.getRequestBody() using try-with-resources

HttpClient AutoCloseable wrapper (forward-compatible)

java.net.http.HttpClient does not implement AutoCloseable before Java 21 (JDK-8304165). On the project's Java 17 target, there is nothing to close. Instead of suppressing the warning, we wrap HttpClient in a CloseableHttpClient that implements AutoCloseable with an instanceof check in close() — this is a no-op on Java 17 but will call the real HttpClient.close() when the minimum JDK is raised to 21+.

Files updated to use try-with-resources via CloseableHttpClient:

• UpdateCamelReleasesMojo.java (private inner class, 2 methods)
• QdrantInfraService.java (package-private helper class)
• 8 OpenAI mock test files (package-private helper class)

Suppressed false positives (framework-managed lifecycle)

• SalesforceComponent.java: ExecutorService managed by the component lifecycle
• AggregateReifier.java: ExecutorService managed by the route lifecycle
• ThreadsReifier.java: Same as AggregateReifier
• CatalogLoader.java: ClassLoader intentionally kept open for plugin resolution
• PluginHelper.java: URLClassLoader intentionally kept open as plugin classloader
• ShellPanel.java: Streams owned by virtualTerminal, closed in stopShell()

Note on method-level vs statement-level suppression

SonarCloud tracks the resource allocation site but reports the issue at the method exit point. Statement-level @SuppressWarnings placed on the allocation line is silently ignored. Method-level suppression is required.

Test plan

• CI passes (no behavioral changes)
• SonarCloud S2095 count decreases after merge

🤖 Generated with Claude Code

[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-salesforce/camel-salesforce-component
• core/camel-core-reifier
• dsl/camel-jbang/camel-jbang-core
• dsl/camel-jbang/camel-jbang-plugin-tui
• test-infra/camel-test-infra-openai-mock
• test-infra/camel-test-infra-qdrant
• tooling/maven/camel-package-maven-plugin

ℹ️ Dependent modules were not tested because the total number of affected modules exceeded the threshold (50). Use the test-dependents label to force testing all dependents.

⚠️ Some tests are disabled on GitHub Actions (@DisabledIfSystemProperty(named = "ci.env.name")) and require manual verification:

• dsl/camel-jbang/camel-jbang-core: 1 test(s) disabled on GitHub Actions

💡 Manual integration tests recommended:

You modified dsl/camel-jbang/camel-jbang-core. The related integration tests in dsl/camel-jbang/camel-jbang-it are excluded from CI. Consider running them manually:

mvn verify -f dsl/camel-jbang/camel-jbang-it -Djbang-it-test

Build reactor — dependencies compiled but only changed modules were tested (7 modules)

• Camel :: Core Reifier
• Camel :: JBang :: Core
• Camel :: JBang :: Plugin :: TUI
• Camel :: Maven Plugins :: Camel Maven Package
• Camel :: Salesforce
• Camel :: Test Infra :: OpenAI Mock
• Camel :: Test Infra :: qdrant

---

⚙️ View full build and test results

[davsclaus]

Review: Fix S2095 resource leak warnings across multiple modules

Reverts prior review feedback without explanation (AggregateReifier, ThreadsReifier, SalesforceComponent)

The prior PR #22343 (CAMEL-23271), also by Guillaume, went through four review rounds specifically to narrow suppression scope. Reviewer @squakez explicitly asked to move @SuppressWarnings from method level to statement level to avoid masking future real leaks. The final version used:

• Statement-level @SuppressWarnings on local variable declarations (AggregateReifier, ThreadsReifier)
• Inline // NOSONAR comment (SalesforceComponent)

This PR moves all three back to method-level @SuppressWarnings, which is the exact approach the reviewer rejected. The PR description acknowledges the move but does not explain why the narrower scope was insufficient. If SonarCloud doesn't recognize statement-level annotations or // NOSONAR in some scanner configuration, that would justify the change — but it should be stated.

New suppressions look good

The 5 new suppressions (CatalogLoader, PluginHelper, ShellPanel, QdrantInfraService, UpdateCamelReleasesMojo) are well-reasoned with accurate comments explaining the lifecycle ownership.

Questions

• Is SonarCloud still flagging the three files that already had suppressions from CAMEL-23271? If so, the move to method-level is justified as a workaround, but worth noting in the PR description.
• The PR has no linked JIRA ticket. Per project conventions, SonarCloud fixes typically use commit format (chores): fix SonarCloud <rule> in <component> — minor style point.

This review does not replace specialized tools such as CodeRabbit, Sourcery, or SonarCloud.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

[davsclaus]

This @SuppressWarnings was intentionally placed at the local variable declaration level in PR #22343 after reviewer feedback from @squakez asked to narrow the scope to avoid masking future real leaks in the same method. Moving it back to method level reverses that decision.

If SonarCloud is not recognizing the statement-level annotation, that would justify this change — could you confirm?

[gnodet]

Claude Code on behalf of Guillaume Nodet

Yes — SonarCloud is still flagging this file despite the statement-level @SuppressWarnings from PR #22343. The annotation sits on line 80 (the variable declaration), but SonarCloud tracks the resource flow to a different exit point and flags line 86 instead, bypassing the annotation.

Confirmed via API: https://sonarcloud.io/api/issues/search?componentKeys=apache_camel&branch=main&rules=java:S2095 still lists AggregateReifier.java:86 as an open issue.

Moving to method-level is the only approach SonarCloud recognizes for this pattern. We acknowledge it broadens the scope, but given that this method only creates one ExecutorService with well-documented lifecycle, the risk of masking future leaks is low.

[davsclaus]

Same as AggregateReifier — this was narrowed to statement level in #22343 after explicit review feedback.

[gnodet]

Claude Code on behalf of Guillaume Nodet

Same situation — SonarCloud still flags ThreadsReifier.java:75 despite the statement-level @SuppressWarnings on line 47. The annotation doesn't cover the line SonarCloud is tracking the resource flow to.

[davsclaus]

The // NOSONAR inline comment was the accepted approach from PR #22343 review. Replacing it with method-level @SuppressWarnings widens the suppression scope. Same question: is SonarCloud not picking up the // NOSONAR?

[gnodet]

Claude Code on behalf of Guillaume Nodet

Confirmed — SonarCloud still flags SalesforceComponent.java:968 despite the // NOSONAR on line 967. The NOSONAR comment is on the constructor call line, but SonarCloud flags the next line (the argument). Method-level @SuppressWarnings is the fallback that SonarCloud actually respects for this pattern.

[Copilot]

Pull request overview

This PR addresses SonarCloud rule S2095 (Resources should be closed) warnings across several Camel modules by replacing inline suppressions with annotated @SuppressWarnings("java:S2095") plus explanatory comments, in cases where resource lifecycles are intentionally managed elsewhere (or the type is not AutoCloseable on the project’s target JDK).

Changes:

• Add/move @SuppressWarnings("java:S2095") to method scope (or other scope) with rationale comments for flagged “resource leak” patterns.
• Clarify intentional ownership/lifecycle management for executors, classloaders, and output streams.
• Suppress S2095 for java.net.http.HttpClient usage where the type is not AutoCloseable on the target Java version.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tooling/maven/camel-package-maven-plugin/src/main/java/org/apache/camel/maven/packaging/UpdateCamelReleasesMojo.java	Adds S2095 suppressions for JDK HttpClient usage in release-fetching helpers.
test-infra/camel-test-infra-qdrant/src/main/java/org/apache/camel/test/infra/qdrant/services/QdrantInfraService.java	Adds S2095 suppression for HttpClient.newHttpClient() in the default put() helper.
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ShellPanel.java	Suppresses S2095 for terminal-related streams owned by the virtual terminal lifecycle.
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/PluginHelper.java	Suppresses S2095 for a URLClassLoader intentionally retained as a plugin classloader.
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/CatalogLoader.java	Suppresses S2095 for a classloader intentionally retained by the version manager.
core/camel-core-reifier/src/main/java/org/apache/camel/reifier/ThreadsReifier.java	Moves S2095 suppression to method level for executor lifecycle managed by ThreadsProcessor.
core/camel-core-reifier/src/main/java/org/apache/camel/reifier/AggregateReifier.java	Moves S2095 suppression to method level for executor lifecycle managed by AggregateProcessor.
components/camel-salesforce/camel-salesforce-component/src/main/java/org/apache/camel/component/salesforce/SalesforceComponent.java	Replaces inline NOSONAR with method-level S2095 suppression for executor lifecycle managed by SalesforceHttpClient.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gnodet]

Claude Code on behalf of Guillaume Nodet

Fixed in 32c5573 — updated comments to remove misleading "GC'd" rationale, added try-with-resources for LineNumberReader, and corrected ShellPanel comment to reference virtualTerminal ownership.

[gnodet]

Claude Code on behalf of Guillaume Nodet

Fixed in 32c5573 — updated comments to remove misleading "GC'd" rationale, added try-with-resources for LineNumberReader, and corrected ShellPanel comment to reference virtualTerminal ownership.

[gnodet]

Claude Code on behalf of Guillaume Nodet

Fixed in 32c5573 — updated comments to remove misleading "GC'd" rationale, added try-with-resources for LineNumberReader, and corrected ShellPanel comment to reference virtualTerminal ownership.

[gnodet]

Claude Code on behalf of Guillaume Nodet

Fixed in 32c5573 — updated comments to remove misleading "GC'd" rationale, added try-with-resources for LineNumberReader, and corrected ShellPanel comment to reference virtualTerminal ownership.

[oscerd]

The resource-leak changes look correct — I went through each closure and found no premature-close of a shared or returned resource. In particular, the return hc.httpClient.send(…, ofByteArray()) inside try-with-resources in QdrantInfraService.put is safe because the body is fully buffered to byte[] before the client closes (it would only be a bug with a streaming ofInputStream handler). The @SuppressWarnings cases are genuine false positives, there's no public-API change, and the CloseableHttpClient wrapper is a no-op on Java 17 and a real close() on 21+.

Main thing: build (17) is currently red. It's a forked-JVM crash in camel-jbang-core mvn test (the last test to start was CamelReloadActionTest, which spawns subprocesses) with no assertion failure in the surefire report; build (25) was cancelled by fail-fast rather than failing on its own. Since this PR's only camel-jbang-core changes are @SuppressWarnings annotations (CatalogLoader, PluginHelper) — behaviorally inert — this looks like a flaky/infra crash rather than something the PR introduced. Could you re-run the failed job? Just can't merge while it's red.

Minor (non-blocking):

• The 34-line CloseableHttpClient wrapper is duplicated three times (camel-test-infra-openai-mock, camel-test-infra-qdrant, and a private inner class in UpdateCamelReleasesMojo) — a little ironic in a Sonar-cleanup PR since SonarCloud flags duplication, though understandable given test-infra modules deliberately don't cross-depend.
• The commit messages are free-form; the documented convention for Sonar sweeps is (chores): fix SonarCloud <rule> in <component>.

---

Reviewed with Claude Code on behalf of Andrea Cosentino. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.