Skip to content

fix(http_handler): defer finalizer client close while requests are in flight - #38242

Open
aminsaedi-behavox wants to merge 1 commit into
BerriAI:litellm_internal_stagingfrom
aminsaedi-behavox:litellm_evicted_handler_del_stream_kill
Open

fix(http_handler): defer finalizer client close while requests are in flight#38242
aminsaedi-behavox wants to merge 1 commit into
BerriAI:litellm_internal_stagingfrom
aminsaedi-behavox:litellm_evicted_handler_del_stream_kill

Conversation

@aminsaedi-behavox

@aminsaedi-behavox aminsaedi-behavox commented Aug 25, 2026

Copy link
Copy Markdown

TLDR

Problem this solves:

  • litellm caches provider httpx clients (AsyncHTTPHandler) with a 1 hour TTL. On eviction the handler is finalized and __del__ closes the owned client
  • The sole-referrer refcount guard added in fix(langfuse): stop a collected httpx handler from closing a shared client #35981 cannot see requests in flight: a live request references the pooled connection, not the client object, so the guard passes and the close tears the pool down under every active SSE stream on it
  • In a production proxy this killed streams in same-second batches once per hour per process, phase-locked to process start (the client-cache TTL clock). Clients saw truncated 200 SSE responses with no message_stop; the server logged httpx.ReadError
  • The EvictedClientCloser (fix(caching): close evicted LLM clients so their connections are reclaimed #35492) fixed the cache's eviction-time close but not this finalizer path

How it solves it:

  • __del__ now defers to a new EvictedClientCloser.close_or_defer via loop.call_soon (no lock taken in GC context)
  • close_or_defer closes an idle client immediately, preserving the reclamation the finalizer used to do, and queues a busy one so the existing reap closes it once it reports no connection in flight and the grace window has passed

User Flow

A proxy operator runs streaming traffic through any provider for more than the client-cache TTL. Before: some in-flight streams die mid-turn each hour with no terminal event. After: streams complete; idle evicted clients still get closed.

Relevant issues

Fixes #38957

Same failure family as the truncated-stream reports in anthropics/claude-code#67766 style symptoms observed by proxy users (client sees a 200 SSE that ends without message_stop).

Linear ticket

Pre-Submission checklist

  • I have added meaningful tests
  • The handful of test files covering my change pass locally: uv run pytest tests/test_litellm/caching/test_evicted_client_closer.py tests/test_litellm/llms/custom_httpx/test_http_handler.py -v (75 passed; the pre-existing cookie test fails on a clean checkout in my sandbox and is unrelated)
  • My PR passes all required CI/CD checks
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • Greptile Confidence Score of at least 4/5

Screenshots / Proof of Fix

A finalizer race cannot be triggered from curl on demand, so the proof is the production signature plus a deterministic in-repo reproduction that fails on the merge base and passes on the tip.

Before (31a6756)

  1. Production deployment (litellm 1.97.0, streaming Anthropic traffic through the proxy): Cloud Run logged "Truncated response body" every hour at the same minutes-past-the-hour as process start, drifting slowly, for 17+ hours. Downstream capture showed SSE streams from unrelated clients dying in same-second batches (2 at 12:42:13, 3 at 12:46:45) with no message_stop, while the proxy recorded httpx.ReadError from response.aiter_bytes()
  2. uv run pytest tests/test_litellm/llms/custom_httpx/test_http_handler.py::test_collected_handler_never_kills_a_stream_in_flight -q (test cherry-picked onto the merge base) fails: the stream raises ReadError mid-body after del handler; gc.collect()

After (3610495)

  1. Same pytest command passes: the stream reads to completion across the handler's collection, and the client lands in the closer queue (closed later once idle and out of grace, covered by test_close_or_defer_defers_a_client_with_a_request_on_the_wire)
  2. test_close_or_defer_closes_an_idle_client_immediately pins that idle reclamation is unchanged, and the pre-existing test_exclusively_owned_async_client_pool_is_closed_when_handler_is_collected still passes
  3. Rebased onto litellm_internal_staging after the finalizer aiohttp-session handling landed there: the cross-loop and no-loop dispose paths are preserved unchanged, the live-loop path now defers to the closer, and the new upstream finalizer tests were adapted to the closer's task bookkeeping (79 passing locally)
  4. The two socket-based tests are bounded by asyncio.timeout(30) and skip Server.wait_closed(), which on Python >= 3.12.1 waits for every client transport and parked two CI shards on the pooled keepalive connection the regression test deliberately leaves open
  5. Production deployment carrying this exact change as a runtime patch: the hourly truncation clusters and batched stream deaths stop (monitoring window ongoing; can attach a longer window on request)

Type

🐛 Bug Fix

Caveats (if any)

  • Sync HTTPHandler.del still closes directly; sync streaming has the same theoretical hole, left for a follow-up to keep this isolated

Changes

  • litellm/caching/evicted_client_closer.py: new EvictedClientCloser.close_or_defer
  • litellm/llms/custom_httpx/http_handler.py: AsyncHTTPHandler.__del__ defers via call_soon to the closer instead of create_task(client.aclose())
  • tests: regression for the in-flight kill, deferral and idle-close behavior of close_or_defer

@CLAassistant

CLAassistant commented Aug 25, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR changes asynchronous HTTP-handler finalization so active clients are handed to the eviction closer instead of being closed beneath in-flight streams.

  • Adds immediate-or-deferred client cleanup based on connection activity.
  • Routes live-loop finalization through the shared closer.
  • Adds coverage for idle cleanup, active-stream deferral, and finalizer behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
litellm/caching/evicted_client_closer.py Adds a close-or-defer entry point that preserves active connections until the existing reap lifecycle can close them.
litellm/llms/custom_httpx/http_handler.py Replaces direct finalizer-triggered client closure with a scheduled handoff to the shared eviction closer.
tests/test_litellm/caching/test_evicted_client_closer.py Adds behavioral coverage for immediate idle closure and deferred closure while a request is active.
tests/test_litellm/llms/custom_httpx/test_http_handler.py Updates finalizer bookkeeping assertions and adds regression coverage for preserving an in-flight stream.

Reviews (3): Last reviewed commit: "fix(http_handler): defer finalizer clien..." | Re-trigger Greptile

Comment thread tests/test_litellm/caching/test_evicted_client_closer.py
@aminsaedi-behavox
aminsaedi-behavox force-pushed the litellm_evicted_handler_del_stream_kill branch from 05685c0 to b04a7b1 Compare August 25, 2026 20:55
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@aminsaedi-behavox
aminsaedi-behavox force-pushed the litellm_evicted_handler_del_stream_kill branch from b04a7b1 to 5cb8b9b Compare August 25, 2026 21:39
@aminsaedi-behavox

Copy link
Copy Markdown
Author

Both failures look unrelated: the together_ai params test fails from cross-test pollution (passes in isolation on staging), and auth-checks hit an Actions download outage. Rerun appreciated.

@codspeed-hq

codspeed-hq Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing aminsaedi-behavox:litellm_evicted_handler_del_stream_kill (3610495) with litellm_internal_staging (5e4b383)

Open in CodSpeed

… flight

A cache-evicted AsyncHTTPHandler is finalized the moment the cache drops it. The
finalizer's sole-referrer refcount guard proves nothing else holds the client
object, but a request in flight references only the pooled connection, so the
guard cannot see it: closing there tears the pool down under every live SSE
stream, one batch per handler-cache TTL per process. The finalizer now defers to
EvictedClientCloser.close_or_defer, which closes an idle client immediately and
queues a busy one until it reports no connection in flight and the grace window
has passed. The handoff runs via loop.call_soon so no lock is taken in GC
context.

Resolves the hourly batched mid-stream stream deaths observed in a production
proxy deployment (streams died in same-second batches at a fixed phase after
process start, matching the client-cache TTL).
@aminsaedi-behavox

Copy link
Copy Markdown
Author

@greptileai

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.

[Bug]: cached AsyncHTTPHandler finalizer closes the pooled httpx client under in-flight SSE streams (hourly batched stream deaths)

2 participants