From 27fd9ae0a686aee8489de1dd91c13e2a4f1ce89b Mon Sep 17 00:00:00 2001 From: agustin18 Date: Tue, 20 Oct 2026 15:45:50 +0000 Subject: [PATCH] fix(presidio): release session lock before analyzer and anonymizer calls (#47579) --- .../guardrails/guardrail_hooks/presidio.py | 8 ++-- .../guardrail_hooks/test_presidio.py | 37 +++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/litellm/proxy/guardrails/guardrail_hooks/presidio.py b/litellm/proxy/guardrails/guardrail_hooks/presidio.py index 7e2f6bfec33..6c82ab77dc1 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/presidio.py +++ b/litellm/proxy/guardrails/guardrail_hooks/presidio.py @@ -389,7 +389,8 @@ class _OPTIONAL_PresidioPIIMasking(CustomGuardrail): async with self._session_lock: if self._http_session is None or self._http_session.closed: self._http_session = aiohttp.ClientSession() - yield self._http_session + session = self._http_session + yield session else: # Background thread/loop -> use loop-bound session cache # This avoids "attached to a different loop" or "no running event loop" errors @@ -638,9 +639,8 @@ class _OPTIONAL_PresidioPIIMasking(CustomGuardrail): # The semaphore is shared per event loop across every chunked call on # this instance, so many oversized blocks in one request (or many # concurrent requests) still hold at most this many analyzer calls in - # flight. On the proxy's main thread the shared-session lock in - # _get_session_iterator additionally serializes the HTTP calls; the - # bound matters for loop-bound sessions (background threads). + # flight. This bound applies to both the shared session and the + # loop-bound sessions used by background threads. analyze_semaphore: Final = self._get_chunk_semaphore() async def _analyze_chunk_bounded( diff --git a/tests/unit/proxy/guardrails/guardrail_hooks/test_presidio.py b/tests/unit/proxy/guardrails/guardrail_hooks/test_presidio.py index e312378969a..fd3dd254309 100644 --- a/tests/unit/proxy/guardrails/guardrail_hooks/test_presidio.py +++ b/tests/unit/proxy/guardrails/guardrail_hooks/test_presidio.py @@ -5068,3 +5068,40 @@ def test_get_session_iterator_reuses_one_session_per_background_loop( assert len(sessions) == 10 assert all(session is sessions[0] for session in sessions) assert presidio_guardrail._http_session is None + + +@pytest.mark.asyncio +async def test_session_lock_is_not_held_while_request_uses_session( + presidio_guardrail: OPTIONAL_PresidioPIIMasking, +) -> None: + """ + The shared session lock must only protect session creation. + + If it is held while callers use the session, every Presidio HTTP request in + a proxy process is serialized even though async_pre_call_hook fans out with + asyncio.gather. + """ + + class DummySession: + closed = False + + async def close(self): + self.closed = True + + presidio_guardrail._http_session = DummySession() + + entered = 0 + both_entered = asyncio.Event() + + async def use_session(): + nonlocal entered + async with presidio_guardrail._get_session_iterator() as session: + assert session is presidio_guardrail._http_session + entered += 1 + if entered == 2: + both_entered.set() + await asyncio.wait_for(both_entered.wait(), timeout=1.0) + + await asyncio.gather(use_session(), use_session()) + + await presidio_guardrail._close_http_session()