mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-06 08:16:43 +00:00
fix(http_handler): remove __del__ to prevent in-flight streaming failures on TTL eviction
Fixes #24929 When the TTL cache evicts an httpx client, the __del__ handler calls close() which propagates into the TCP connection pool, killing in-flight streaming responses. Clients observe periodic httpx.ReadTimeout spikes aligned with _DEFAULT_TTL_FOR_HTTPX_CLIENTS (3600s). The __del__ handlers are unnecessary — Python's GC already cleans up the underlying httpx.AsyncClient/httpx.Client resources via their own finalizers. The explicit close() in __del__ is harmful because: 1. TTL eviction drops the last reference, triggering __del__ 2. __del__ calls close() which closes the TCP connection pool 3. In-flight streaming responses lose their connection mid-transfer 4. Clients experience ReadTimeout bursts every TTL interval Removing __del__ from both AsyncHTTPHandler and HTTPHandler lets the httpx clients be reclaimed by GC naturally without disrupting active connections. Added 2 tests verifying that deleting a handler does NOT close the underlying httpx client.
This commit is contained in:
parent
d1df4e838b
commit
495dbbe581
2 changed files with 46 additions and 12 deletions
|
|
@ -725,12 +725,6 @@ class AsyncHTTPHandler:
|
|||
response.raise_for_status()
|
||||
return response
|
||||
|
||||
def __del__(self) -> None:
|
||||
try:
|
||||
asyncio.get_running_loop().create_task(self.close())
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
@staticmethod
|
||||
def _create_async_transport(
|
||||
ssl_context: Optional[ssl.SSLContext] = None,
|
||||
|
|
@ -1182,12 +1176,6 @@ class HTTPHandler:
|
|||
except Exception as e:
|
||||
raise e
|
||||
|
||||
def __del__(self) -> None:
|
||||
try:
|
||||
self.close()
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
def _create_sync_transport(self) -> Optional[HTTPTransport]:
|
||||
"""
|
||||
Create an HTTP transport with IPv4 only if litellm.force_ipv4 is True.
|
||||
|
|
|
|||
|
|
@ -658,3 +658,49 @@ async def test_httpx_handler_uses_env_user_agent(monkeypatch):
|
|||
assert req.headers.get("User-Agent") == "Claude Code"
|
||||
finally:
|
||||
await handler.close()
|
||||
|
||||
|
||||
class TestDelHandlerRemoved:
|
||||
"""
|
||||
Verify that AsyncHTTPHandler and HTTPHandler do NOT close the underlying
|
||||
httpx client when the wrapper is garbage-collected.
|
||||
|
||||
Prior to this fix, __del__ called self.close(), which propagated into
|
||||
the TCP connection pool and killed in-flight streaming responses whenever
|
||||
the TTL cache evicted a client. See #24929.
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_async_handler_no_close_on_del(self):
|
||||
"""AsyncHTTPHandler should NOT close the httpx client on __del__."""
|
||||
handler = AsyncHTTPHandler(timeout=httpx.Timeout(timeout=10.0))
|
||||
|
||||
# Grab reference to the underlying client
|
||||
client = handler.client
|
||||
|
||||
# Simulate GC / TTL eviction
|
||||
del handler
|
||||
|
||||
# The client should still be open (not closed by __del__)
|
||||
assert not client.is_closed
|
||||
|
||||
# Cleanup
|
||||
await client.aclose()
|
||||
|
||||
def test_sync_handler_no_close_on_del(self):
|
||||
"""HTTPHandler should NOT close the httpx client on __del__."""
|
||||
from litellm.llms.custom_httpx.http_handler import HTTPHandler
|
||||
|
||||
handler = HTTPHandler(timeout=httpx.Timeout(timeout=10.0))
|
||||
|
||||
# Grab reference to the underlying client
|
||||
client = handler.client
|
||||
|
||||
# Simulate GC / TTL eviction
|
||||
del handler
|
||||
|
||||
# The client should still be open (not closed by __del__)
|
||||
assert not client.is_closed
|
||||
|
||||
# Cleanup
|
||||
client.close()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue