Skip to content

camel-openai: close OpenAI client on endpoint stop to prevent OkHttp thread leak#23531

Merged
Croway merged 1 commit into
apache:mainfrom
Croway:fix/openai-client-close
May 26, 2026
Merged

camel-openai: close OpenAI client on endpoint stop to prevent OkHttp thread leak#23531
Croway merged 1 commit into
apache:mainfrom
Croway:fix/openai-client-close

Conversation

@Croway
Copy link
Copy Markdown
Contributor

@Croway Croway commented May 26, 2026

Summary

  • Call client.close() in OpenAIEndpoint.doStop() before nulling the reference so that OkHttp's dispatcher executor, connection pool, and cache are properly shut down.
  • Without this, OkHttp threads (OkHttp Dispatcher, OkHttp ConnectionPool) linger after route stop, especially visible under exec:java or Camel JBang dev mode where the JVM does not exit.

@github-actions
Copy link
Copy Markdown
Contributor

🌟 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
Copy link
Copy Markdown
Contributor

🧪 CI tested the following changed modules:

  • components/camel-ai/camel-openai

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

  • components/camel-ai/camel-openai: 6 test(s) disabled on GitHub Actions
All tested modules (8 modules)
  • Camel :: AI :: OpenAI
  • Camel :: JBang :: MCP
  • Camel :: JBang :: Plugin :: Route Parser
  • Camel :: JBang :: Plugin :: TUI
  • Camel :: JBang :: Plugin :: Validate
  • Camel :: Launcher :: Container
  • Camel :: YAML DSL :: Validator
  • Camel :: YAML DSL :: Validator Maven Plugin

⚙️ View full build and test results

returnDirectTools.clear();
}
if (client != null) {
client.close();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we enclose in a try catch as it is done for the mcp clients?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closes this client, relinquishing any underlying resources.
This is purposefully not inherited from AutoCloseable because the client is long-lived and usually should not be synchronously closed via try-with-resources.

this is from the .close() java doc, should be safe without a try catch

Copy link
Copy Markdown
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct fix — calling client.close() before setting client = null in OpenAIEndpoint.doStop() ensures the HTTP client resources are properly released. Without this, the client would be leaked on endpoint stop.

LGTM.

Fully automatic review from Claude Code

@Croway Croway merged commit 294dbf8 into apache:main May 26, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants