Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix fd leak #6950

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Fix fd leak #6950

wants to merge 3 commits into from

Conversation

tofarr
Copy link
Collaborator

@tofarr tofarr commented Feb 25, 2025

  • This change is worth documenting at https://docs.all-hands.dev/
  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

End-user friendly description of the problem this fixes or functionality that this introduces.
This is ongoing work in file descriptor leaks, related to multiple PRs:

The previous issue was that there was some LiteLLM implementations which created multiple httpx clients and never closed any of them (Claude), some which closed their connections (Proxy), and some which used a semi global shared httpx client which is never closed (OpenAI).

In the last incarnation, the OpenAI model caused issues, which should now be resolved using the proxy client. The proxy client will close connections when asked, but transparently reopen them if requested (Similar to the requests library)

Once our LiteLLM PR is merged, we will be able to revisit (and hopefully remove) this.


Give a summary of what the PR does, explaining any non-trivial design decisions.


Link of any specific issues this addresses.


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:09e46e9-nikolaik   --name openhands-app-09e46e9   docker.all-hands.dev/all-hands-ai/openhands:09e46e9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant