Repository navigation
Conversation
…exit_stack In `InferenceClient._inner_post`, non-streaming responses were entered into `self.exit_stack`, causing exit callbacks and response references to be held until `client.close()`. For long-lived clients, this resulted in unbounded memory growth across non-streaming calls. Mirror `AsyncInferenceClient` by only entering responses into `self.exit_stack` when `stream=True`, and using `get_session().post` directly when `stream=False`. Fixes huggingface#5169. Co-Authored-By: Claude Code <[email protected]>
|
I ran an AI-assisted, offline before/after check through public Tracked responses / requests / SDK callbacks before client close:
The non-streaming bodies were already closed in both versions: this measures Independent replay matched the observations. Both source snapshots report Jay Glidden |
|
Thanks for running the independent offline before/after lifecycle check and sharing the reproduction numbers @EngineeringWithJay! Glad to see the tracked object/callback retention numbers drop to 0 after GC across the non-streaming cases as expected. |
|
You're welcome—glad the extra lifecycle controls were useful. Thanks for putting the fix together! |
|
Thanks for the PR! Closing in favor of #5197, which keeps the existing |
Summary
Fixes #5169
In
InferenceClient._inner_post, every response was entered intoself.exit_stack, even for non-streaming requests (stream=False). Because non-streaming calls read the response body synchronously before returning, registering the response intoself.exit_stackkept the response exit callback (along with the request and response objects and underlying buffers) referenced untilclient.close()or context manager exit.For long-lived
InferenceClientinstances (e.g. background services or web workers making repeated non-streaming calls), this caused unbounded memory accumulation.This PR mirrors the approach used in
AsyncInferenceClient:stream=True: enterget_session().stream(...)intoself.exit_stackand returnresponse.iter_lines().stream=False: callget_session().post(...)directly and returnresponse.content, bypassingself.exit_stackentirely so no exit callbacks or response references are retained.Verification
Before fix (from reproduction script on #5169):
After fix:
Unit test output:
🤖 Generated with Claude Code
Note
Low Risk
Targeted lifecycle fix aligned with
AsyncInferenceClient; non-streaming behavior should be equivalent aside from fixing the leak, with streaming unchanged.Overview
Fixes #5169 by changing how
InferenceClient._inner_postmanages HTTP responses for non-streaming calls.Non-streaming (
stream=False) requests now useget_session().post(...)directly and returnresponse.content, without entering the response intoself.exit_stack. Previously every call went throughexit_stack.enter_context(get_session().stream(...)), which kept response teardown callbacks (and associated objects) alive untilclose()—causing unbounded memory growth on long-lived clients making many synchronous inference calls.Streaming (
stream=True) behavior is unchanged: the stream context is still registered onexit_stackanditer_lines()is returned.A regression test (
test_inner_post_exit_stack_leak_prevention) asserts that repeated non-streaming calls leaveexit_stackempty, streaming still registers one callback, andclose()clears it.Reviewed by Cursor Bugbot for commit 13a1115. Bugbot is set up for automated code reviews on this repo. Configure here.