diff --git a/litellm/llms/custom_httpx/async_client_cleanup.py b/litellm/llms/custom_httpx/async_client_cleanup.py index f7f5c90d955..7f0f2654040 100644 --- a/litellm/llms/custom_httpx/async_client_cleanup.py +++ b/litellm/llms/custom_httpx/async_client_cleanup.py @@ -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 diff --git a/tests/test_litellm/llms/custom_httpx/test_openai_session_leak.py b/tests/test_litellm/llms/custom_httpx/test_openai_session_leak.py index fde184626ac..3f42cf03e9d 100644 --- a/tests/test_litellm/llms/custom_httpx/test_openai_session_leak.py +++ b/tests/test_litellm/llms/custom_httpx/test_openai_session_leak.py @@ -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"