Close async clients whose teardown is aclose() or close(), checked independently

Address review on #31246: check aclose() and close() separately rather than
`aclose or close`, so a synchronous aclose() can't shadow a real async close().
Fix Black formatting and add regression tests for the aclose() path and the
sync-aclose/async-close case.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
mpg 2026-06-26 15:33:29 -07:00
parent 673d69f094
commit 541f432d3c
2 changed files with 70 additions and 12 deletions

View file

@ -50,19 +50,21 @@ async def close_litellm_async_clients():
# Handle any other cached async client. Most expose aclose(), but some
# SDK clients -- notably the openai SDK's AsyncOpenAI / AsyncAzureOpenAI,
# which are cached here (set_cached_openai_client) and built on litellm's
# aiohttp transport -- name the async teardown close() instead. Accept
# either, guarding on iscoroutinefunction so sync close() methods
# (sync OpenAI / HTTPHandler) are left untouched.
# aiohttp transport -- name the async teardown close() instead. Try each
# candidate independently, guarding on iscoroutinefunction so sync
# teardowns (sync OpenAI / HTTPHandler) are left untouched. Checking them
# separately (rather than `aclose or close`) avoids picking a truthy but
# synchronous aclose() and then skipping a real async close() alongside it.
else:
closer = getattr(handler, "aclose", None) or getattr(
handler, "close", None
)
if closer is not None and inspect.iscoroutinefunction(closer):
try:
await closer()
except Exception:
# Silently ignore errors during cleanup
pass
for name in ("aclose", "close"):
closer = getattr(handler, name, None)
if closer is not None and inspect.iscoroutinefunction(closer):
try:
await closer()
except Exception:
# Silently ignore errors during cleanup
pass
break
# Close the global base_llm_aiohttp_handler instance (issue #12443)
# This is used by Gemini and other providers that use aiohttp

View file

@ -23,6 +23,36 @@ class _AsyncCloseOnlyClient:
self.closed = True
class _AsyncACloseOnlyClient:
"""The pre-existing path: an async client whose teardown is aclose()."""
def __init__(self):
self.closed = False
async def aclose(self):
self.closed = True
class _SyncCloseAsyncACloseClient:
"""
The `or` short-circuit case: a sync aclose() alongside an async close().
`getattr(handler, "aclose") or getattr(handler, "close")` would pick the
truthy-but-synchronous aclose(), fail the iscoroutinefunction guard, and then
never reach the real async close() -- leaking the client. Each candidate must
be checked independently.
"""
def __init__(self):
self.closed = False
def aclose(self): # sync -- a decoy that must not satisfy cleanup
pass
async def close(self):
self.closed = True
class _SyncClient:
"""A sync client (close() is not a coroutine) -- must be left untouched."""
@ -48,3 +78,29 @@ async def test_cleanup_closes_async_close_only_clients():
assert async_client.closed, "async close()-only client should be closed"
assert not sync_client.closed, "sync client should be left untouched"
@pytest.mark.asyncio
async def test_cleanup_closes_async_aclose_clients():
"""Regression guard for the pre-existing aclose() path."""
aclose_client = _AsyncACloseOnlyClient()
litellm.in_memory_llm_clients_cache.set_cache(
key="test_async_aclose_only", value=aclose_client, ttl=60
)
await close_litellm_async_clients()
assert aclose_client.closed, "async aclose() client should be closed"
@pytest.mark.asyncio
async def test_cleanup_prefers_async_close_over_sync_aclose():
"""A sync aclose() must not shadow a real async close() (the `or` gap)."""
client = _SyncCloseAsyncACloseClient()
litellm.in_memory_llm_clients_cache.set_cache(
key="test_sync_aclose_async_close", value=client, ttl=60
)
await close_litellm_async_clients()
assert client.closed, "async close() should be called when aclose() is sync"