fix: remove cache eviction close that kills in-use httpx clients

LLMClientCache._remove_key() was eagerly closing httpx/OpenAI clients on
cache eviction. If any in-flight request still held a reference to the
evicted client, the request failed with:

  RuntimeError: Cannot send a request, as the client has been closed.

This manifested in production after 1-2 hours (matching the 1-hour
_DEFAULT_TTL_FOR_HTTPX_CLIENTS). The fix was previously applied in PR
#22247 but regressed when commit fb72979432 re-added the close logic to
address unawaited coroutine warnings.

The correct fix is to NOT close clients on eviction at all:
- Evicted clients may still be referenced by in-flight requests
- Without calling close(), no coroutines are created, so no warnings
- Python GC handles cleanup when all references are dropped
- close_litellm_async_clients() handles explicit shutdown cleanup

Fixes the 'Cannot send a request, as the client has been closed' error
for OpenAI, Azure, and Bedrock streaming after 1+ hours of operation.

Co-authored-by: Ishaan Jaff <ishaan-jaff@users.noreply.github.com>
This commit is contained in:
Cursor Agent 2026-03-05 18:49:59 +00:00
parent 46fa9a33da
commit 8a70b72ca4
2 changed files with 75 additions and 74 deletions

View file

@ -3,36 +3,21 @@ Add the event loop to the cache key, to prevent event loop closed errors.
"""
import asyncio
from typing import Set
from .in_memory_cache import InMemoryCache
class LLMClientCache(InMemoryCache):
# Background tasks must be stored to prevent garbage collection, which would
# trigger "coroutine was never awaited" warnings. See:
# https://docs.python.org/3/library/asyncio-task.html#creating-tasks
# Intentionally shared across all instances as a global task registry.
_background_tasks: Set[asyncio.Task] = set()
"""Cache for LLM HTTP clients (OpenAI, Azure, httpx, etc.).
def _remove_key(self, key: str) -> None:
"""Close async clients before evicting them to prevent connection pool leaks."""
value = self.cache_dict.get(key)
super()._remove_key(key)
if value is not None:
close_fn = getattr(value, "aclose", None) or getattr(value, "close", None)
if close_fn and asyncio.iscoroutinefunction(close_fn):
try:
task = asyncio.get_running_loop().create_task(close_fn())
self._background_tasks.add(task)
task.add_done_callback(self._background_tasks.discard)
except RuntimeError:
pass
elif close_fn and callable(close_fn):
try:
close_fn()
except Exception:
pass
IMPORTANT: This cache intentionally does NOT close clients on eviction.
Evicted clients may still be in use by in-flight requests. Closing them
eagerly causes ``RuntimeError: Cannot send a request, as the client has
been closed.`` errors in production after the TTL (1 hour) expires.
Clients that are no longer referenced will be garbage-collected normally.
For explicit shutdown cleanup, use ``close_litellm_async_clients()``.
"""
def update_cache_key_with_event_loop(self, key):
"""

View file

@ -1,3 +1,13 @@
"""
Tests for LLMClientCache.
The cache intentionally does NOT close clients on eviction because evicted
clients may still be referenced by in-flight requests. Closing them eagerly
causes ``RuntimeError: Cannot send a request, as the client has been closed.``
See: https://github.com/BerriAI/litellm/pull/22247
"""
import asyncio
import os
import sys
@ -33,12 +43,12 @@ class MockSyncClient:
@pytest.mark.asyncio
async def test_remove_key_no_unawaited_coroutine_warning():
async def test_remove_key_does_not_close_async_client():
"""
Test that evicting an async client from LLMClientCache does not produce
'coroutine was never awaited' warnings.
Evicting an async client from LLMClientCache must NOT close it because
an in-flight request may still hold a reference to the client.
Regression test for https://github.com/BerriAI/litellm/issues/22128
Regression test for production 'client has been closed' crashes.
"""
cache = LLMClientCache(max_size_in_memory=2)
@ -46,43 +56,19 @@ async def test_remove_key_no_unawaited_coroutine_warning():
cache.cache_dict["test-key"] = mock_client
cache.ttl_dict["test-key"] = 0 # expired
with warnings.catch_warnings(record=True) as caught_warnings:
warnings.simplefilter("always")
cache._remove_key("test-key")
# Let the event loop process the close task
await asyncio.sleep(0.1)
coroutine_warnings = [
w for w in caught_warnings if "coroutine" in str(w.message).lower()
]
assert (
len(coroutine_warnings) == 0
), f"Got unawaited coroutine warnings: {coroutine_warnings}"
@pytest.mark.asyncio
async def test_remove_key_closes_async_client():
"""
Test that evicting an async client from the cache properly closes it.
"""
cache = LLMClientCache(max_size_in_memory=2)
mock_client = MockAsyncClient()
cache.cache_dict["test-key"] = mock_client
cache.ttl_dict["test-key"] = 0
cache._remove_key("test-key")
# Let the event loop process the close task
# Give the event loop a chance to run any background tasks
await asyncio.sleep(0.1)
assert mock_client.closed is True
# Client must NOT be closed — it may still be in use
assert mock_client.closed is False
assert "test-key" not in cache.cache_dict
assert "test-key" not in cache.ttl_dict
def test_remove_key_closes_sync_client():
def test_remove_key_does_not_close_sync_client():
"""
Test that evicting a sync client from the cache properly closes it.
Evicting a sync client from the cache must NOT close it.
"""
cache = LLMClientCache(max_size_in_memory=2)
@ -92,15 +78,15 @@ def test_remove_key_closes_sync_client():
cache._remove_key("test-key")
assert mock_client.closed is True
assert mock_client.closed is False
assert "test-key" not in cache.cache_dict
@pytest.mark.asyncio
async def test_eviction_closes_async_clients():
async def test_eviction_does_not_close_async_clients():
"""
Test that cache eviction (when cache is full) properly closes async clients
without producing warnings.
When the cache is full and an entry is evicted, the evicted async client
must remain open and must not produce 'coroutine was never awaited' warnings.
"""
cache = LLMClientCache(max_size_in_memory=2, default_ttl=1)
@ -123,11 +109,41 @@ async def test_eviction_closes_async_clients():
len(coroutine_warnings) == 0
), f"Got unawaited coroutine warnings: {coroutine_warnings}"
# Evicted clients must NOT be closed
for client in clients:
assert client.closed is False
@pytest.mark.asyncio
async def test_eviction_no_unawaited_coroutine_warning():
"""
Evicting an async client from LLMClientCache must not produce
'coroutine was never awaited' warnings.
Regression test for https://github.com/BerriAI/litellm/issues/22128
"""
cache = LLMClientCache(max_size_in_memory=2)
mock_client = MockAsyncClient()
cache.cache_dict["test-key"] = mock_client
cache.ttl_dict["test-key"] = 0 # expired
with warnings.catch_warnings(record=True) as caught_warnings:
warnings.simplefilter("always")
cache._remove_key("test-key")
await asyncio.sleep(0.1)
coroutine_warnings = [
w for w in caught_warnings if "coroutine" in str(w.message).lower()
]
assert (
len(coroutine_warnings) == 0
), f"Got unawaited coroutine warnings: {coroutine_warnings}"
def test_remove_key_no_event_loop():
"""
Test that _remove_key doesn't raise when there's no running event loop
(falls through to the RuntimeError except branch).
_remove_key works correctly even when there's no running event loop.
"""
cache = LLMClientCache(max_size_in_memory=2)
@ -140,19 +156,19 @@ def test_remove_key_no_event_loop():
assert "test-key" not in cache.cache_dict
@pytest.mark.asyncio
async def test_background_tasks_cleaned_up_after_completion():
def test_remove_key_removes_plain_values():
"""
Test that completed close tasks are removed from the _background_tasks set.
_remove_key correctly removes non-client values (strings, dicts, etc.).
"""
cache = LLMClientCache(max_size_in_memory=2)
cache = LLMClientCache(max_size_in_memory=5)
mock_client = MockAsyncClient()
cache.cache_dict["test-key"] = mock_client
cache.ttl_dict["test-key"] = 0
cache.cache_dict["str-key"] = "hello"
cache.ttl_dict["str-key"] = 0
cache.cache_dict["dict-key"] = {"foo": "bar"}
cache.ttl_dict["dict-key"] = 0
cache._remove_key("test-key")
# Let the task complete
await asyncio.sleep(0.1)
cache._remove_key("str-key")
cache._remove_key("dict-key")
assert len(cache._background_tasks) == 0
assert "str-key" not in cache.cache_dict
assert "dict-key" not in cache.cache_dict