Bug: anyio.Lock in async_auth_flow causes RuntimeError under concurrent OAuth MCP connections #2847

Activity

1. Bartok9 commented on Jun 13, 2026

   

   Bartok9

   on Jun 13, 2026

   More actions

   Confirmed the root-cause analysis here is correct and the existing fix is the right one.

   Verified on current main: OAuthContext.lock is anyio.Lock (records task identity at acquire(), enforces same-task release()), and async_auth_flow holds it across yield points (oauth2.py — the async with self.context.lock: wraps the yield refresh_request / yield request / full 401 flow). When httpx drives the generator from a different task during concurrent connections, release() raises RuntimeError: The current task is not holding this lock — exactly the trace here and in #2644.

   PR #2660 fixes this correctly by narrowing lock scope (no HTTP yield runs while holding context.lock) plus a single-flight refresh_lock. It was sitting CONFLICTING since May 27; I rebased it onto current main — conflict is small and isolated to the refresh block, and the rebased branch is green (108 auth tests pass, ruff clean). Details + branch on #2660. The asyncio.Lock swap mentioned here works for asyncio-only deployments but loses trio portability, so #2660's narrow-the-scope approach is preferable.
2. Bartok9 commented on Jun 13, 2026

   

   Bartok9

   on Jun 13, 2026

   More actions

   Opened #2858 with a ready-to-merge fix: a conflict-free rebase of @peisuke's #2660 onto current main (all credit to @peisuke for the fix). Narrows context.lock scope so no HTTP yield runs while the lock is held, with a single-flight refresh_lock — keeps trio portability. Green: 108 auth tests pass, ruff clean. This should unblock the concurrent-OAuth RuntimeError reported here.
3. Bartok9 commented on Jun 13, 2026

   

   Bartok9

   on Jun 13, 2026

   More actions

   @Ben-Home — this is really valuable production data, thank you. The "~15% failures → zero after jitter" number is exactly the kind of real-world signal worth acting on, and we did: I just opened #2859 to bring proactive refresh + jitter into the SDK, motivated directly by your comment.

   A couple of clarifications on the current architecture, since they shape where each fix belongs:

   On lock sharing across connectors: In this SDK the lock isn't actually shared between connectors. OAuthContext (which owns the lock) is instantiated per OAuthClientProvider, and there's one provider per server connection — so Notion and Shopify already have independent locks. There's no global/module-level lock; it's field(default_factory=anyio.Lock) on a per-instance context. So "Notion refresh blocks Shopify" shouldn't occur from lock contention today — the per-connector isolation you'd want is already the structure.

   On auth vs refresh lock (your point #3): #2858 already does this. It keeps context.lock for short state reads/writes only, adds a dedicated single-flight refresh_lock, and runs all HTTP yields (including the long-poll GET SSE) outside both locks. So the initial PKCE flow and token refresh no longer contend, and a slow refresh round-trip doesn't block unrelated requests. (#2858 fixes the RuntimeError correctness bug; it isn't a Lock→Semaphore swap.)

   On your jitter idea (#2) — this is the genuinely new lever, so I built it: #2859 adds calculate_token_refresh_time() — refresh at ~80% of TTL minus bounded random jitter (≤30s, always subtracted so it can never overshoot hard expiry), tracked as token_refresh_time with a should_refresh_token() predicate. async_auth_flow now refreshes proactively when past the jittered window (not only reactively at hard expiry). Because each provider's refresh point is independently jittered, a fleet's tokens stop clustering into the same ~60s window — which is precisely the thundering-herd you described. Fully backward-compatible (no expires_in → stays purely reactive). 12 new tests, full auth suite green, ruff + pyright clean.

   Would genuinely love to see your refresh-stagger implementation — if your approach differs (e.g. different fraction/jitter bounds, or per-fleet coordination rather than per-connector), that's worth folding in or discussing on #2859. Thanks again for pushing on this.
4. Bartok9

   added 2 commits that reference this issue on Jun 21, 2026

   fix(oauth): narrow async_auth_flow lock scope to avoid blocking long-…

   ...

   This issue will close once commit 425161d is merged into the 'main' branch.

   425161d

   fix(oauth): narrow async_auth_flow lock scope to avoid blocking long-…

   ...

   This issue will close once commit a9be0b9 is merged into the 'main' branch.

   a9be0b9
5. diaanhw commented on Jul 8, 2026

   

   diaanhw

   on Jul 8, 2026

   More actions

   Independent production confirmation on a long-running gateway (hermes-agent 0.16.0), with an escalation path worth noting for long-lived processes.

   Environment: mcp==1.26.0, anyio==4.12.1, httpx==0.28.1, Python 3.11.14, macOS arm64. Four OAuth 2.1/PKCE streamable-HTTP MCP servers (Reportify) connected concurrently from one process.

   Trace (verbatim; three separate incidents on 2026-07-04/06/07):

   ERROR asyncio: Task exception was never retrieved
   future: <Task finished name='Task-6716' coro=<<async_generator_athrow without __name__>()> exception=RuntimeError('The current task is not holding this lock')>
   Traceback (most recent call last):
     File ".../mcp/client/auth/oauth2.py", line 503, in async_auth_flow
       response = yield request
   GeneratorExit
   
   During handling of the above exception, another exception occurred:
   
   Traceback (most recent call last):
     File ".../mcp/client/auth/oauth2.py", line 484, in async_auth_flow
       async with self.context.lock:
     File ".../anyio/_core/_synchronization.py", line 166, in __aexit__
       self.release()
     File ".../anyio/_backends/_asyncio.py", line 1854, in release
       raise RuntimeError("The current task is not holding this lock")
   RuntimeError: The current task is not holding this lock
   

   This is the generator being closed (GeneratorExit delivered at the yield) from a task other than the one that entered async with self.context.lock: — so it reproduces without retries or redirects; connection teardown during a suspended auth flow is enough. Immediately followed by:

   WARNING tools.mcp_tool: MCP server 'reportify-agent' connection lost (attempt 1/5), reconnecting in 2s: unhandled errors in a TaskGroup (1 sub-exception)
   

   Why this is worse than an aborted request in long-lived deployments: after the RuntimeError the server's connection is dead inside the still-running gateway, so periodic token refresh for it stops. Our provider's refresh token has a bounded lifetime (~25 h); once it ages past that, refresh is rejected (Token refresh failed: 400) and the only way back is a full interactive browser re-auth. An "intermittent" failure escalates into credential loss whenever the process outlives the refresh-token lifetime.

   Also confirming the earlier comment about current state: v1.28.1 and main still hold context.lock across the yields (oauth2.py:493 on v1.28.1), so no released version avoids this today. +1 for maintainer review of #2858.
6. maxisbey

   added

   bugSomething isn't workingSomething isn't working

   P1Significant bug affecting many users, highly requested featureSignificant bug affecting many users, highly requested feature

   needs confirmationNeeds confirmation that the PR is actually required or needed.Needs confirmation that the PR is actually required or needed.

   authIssues and PRs related to Authentication / OAuthIssues and PRs related to Authentication / OAuth

   on Aug 14, 2026
7. miggitty commented on Sep 3, 2026

   

   miggitty

   on Sep 3, 2026

   More actions

   Confirming this, and adding a second trigger path that is more common than concurrency and considerably more severe than an intermittent RuntimeError.

   The lock is not just noisy — it is permanently poisoned

   anyio.Lock.release() raises before clearing _owner_task (anyio/_backends/_asyncio.py:1933). So when the cross-task release fails, the lock remains owned by a task that no longer exists. Any consumer that caches its OAuthClientProvider — reusing it across reconnects, as several do — then deadlocks in async with self.context.lock on every subsequent request, forever, without ever reaching the network.

   In our case that turned into an 11-hour silent outage: reconnect attempts every ~5.5 minutes, all expiring on connect_timeout, with zero HTTP requests emitted the entire time. It reads like a network failure and isn't one. Only a process restart recovered it.

   A trigger that does not require concurrency

   The reported path is httpx resuming the generator from a different task under concurrent connections. There is a second one that needs only a single connection:

   Any consumer that abandons the async_auth_flow generator rather than closing it. async_auth_flow suspends at yield request while holding the lock. If it is dropped, CPython hands it to asyncio's async-generator finalizer, which runs athrow(GeneratorExit) in a newly created task — so the cross-task release is not a race, it is guaranteed to fail 100% of the time.

   Abandonment is easy to hit accidentally. Ours came from a wrapper that bridged the bidirectional protocol and held the inner generator in a local without an aclose(). It also arises whenever a streaming request is torn down mid-flight — for us, the streamable-HTTP GET stream against a server that answers 405 to GET, so handle_get_stream took the exception path on every single connect.

   Standalone repro (no server, no network, no concurrency)

   import asyncio, gc, anyio
   
   class Ctx:
       def __init__(self):
           self.lock = anyio.Lock()
   
   async def auth_flow(ctx):                 # mirrors oauth2.py:582
       async with ctx.lock:
           response = yield "request"        # oauth2.py:601
           if response == 401:
               yield "retry"
   
   async def main():
       ctx = Ctx()
       gen = auth_flow(ctx)
       await gen.__anext__()                 # acquires the lock, suspends at yield
       del gen                               # abandoned instead of closed
       gc.collect()
       await asyncio.sleep(0.3)              # asyncio finalizes it in a NEW task
       print("lock still held:", ctx.lock.locked())
   
   asyncio.run(main())

   Task exception was never retrieved
   future: <Task finished coro=<<async_generator_athrow without __name__>()>
           exception=RuntimeError('The current task is not holding this lock')>
     ...
     File "anyio/_backends/_asyncio.py", line 1935, in release
       raise RuntimeError("The current task is not holding this lock")
   RuntimeError: The current task is not holding this lock
   
   lock still held: True
   

   Reproduced on Python 3.11.14 / anyio 4.14.2 with both mcp 2.0.0 and the current oauth2.py, which still holds anyio.Lock across the yield — so upgrading is not a workaround today.

   Implication for the fix

   This argues for #2858's approach specifically: narrowing the scope so no lock is held across a yield removes both trigger paths at once. Swapping in asyncio.Lock also stops the poisoning (no owner affinity, so the finalizer's release succeeds) and works as a local stopgap, but it costs trio portability and converts any future same-task re-acquisition from a loud error into a silent deadlock — so it isn't a good upstream answer.

   Downstream consumers who cache providers may also want to rebuild the provider when an OAuth connection exhausts its retries, since nothing in-process can currently un-poison a cached one.

   Happy to test a patch against a real 405-on-GET OAuth server if that's useful.
8. AkshayShah03 commented on Oct 13, 2026

   

   AkshayShah03

   on Oct 13, 2026

   More actions

   Confirmed on main (754254c), but I think the trigger is different from the one in the original report.

   With httpx2 I couldn't get it to fail through concurrent connections: 2000 concurrent requests, with forced refreshes and random client timeouts, gave no errors and never left the lock held. _send_handling_auth drives the auth generator and calls aclose() on it in the same task.

   What does reproduce it every time is an auth-flow generator that is suspended at yield request and then dropped without aclose(). asyncio finalizes it in a new task. That task tries to release context.lock, which _auth_flow holds across the yield (oauth2.py:606), and you get the exact error from the report:

   Task exception was never retrieved: RuntimeError('The current task is not holding this lock')
   

   The worse part: anyio raises before it clears the owner, so the lock stays held. The next request on the same provider then hangs forever in async with self.context.lock, without sending any HTTP request.

   Minimal shape:

   gen = provider.async_auth_flow(request)
   await gen.__anext__()        # takes context.lock, suspends at `yield request`
   del gen; gc.collect()        # finalizer runs aclose() in a new task -> RuntimeError
   await asyncio.sleep(0.2)
   assert provider.context.lock.locked()   # still held -> next request hangs

   That gives a deterministic regression test for whichever fix lands: abandon the flow in one task, then assert a request from another task completes and lock.locked() is False. #2858 narrows the lock scope, which should cover this, but it predates the move to RedirectAwareAuth and probably needs a rebase. Full script: https://gist.github.com/AkshayShah03/3cea1dead96b4792a7c3e4d2d71cd7f7

   Repro written with AI assistance; I ran it and checked the output.