Skip to content

fix(oauth): narrow context.lock scope in async_auth_flow (rebase of #2660 by @peisuke, closes #2847)#2858

Open
Bartok9 wants to merge 1 commit into
modelcontextprotocol:mainfrom
Bartok9:salvage/2660-oauth-lock-rebase
Open

fix(oauth): narrow context.lock scope in async_auth_flow (rebase of #2660 by @peisuke, closes #2847)#2858
Bartok9 wants to merge 1 commit into
modelcontextprotocol:mainfrom
Bartok9:salvage/2660-oauth-lock-rebase

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jun 13, 2026

Copy link
Copy Markdown

Summary

Conflict-free salvage of #2660 by @peisuke onto current main, opened as a fresh, mergeable PR so the fix can land without waiting on the original draft to be un-conflicted. All credit for the fix goes to @peisuke — the commit is authored by them; I only resolved the rebase against main and squashed to a single commit.

Closes #2847. Addresses the same RuntimeError: The current task is not holding this lock reported in #2644 and #2847.

Why a new PR

#2660 has been a CONFLICTING draft since May 22 with no maintainer review. Issue #2847 (June 12) is a second, independent production repro of the exact bug, and the reporter explicitly asks for #2660 to be reviewed/merged. Rather than let the fix keep sitting, this PR provides a ready-to-merge version. If maintainers prefer to merge #2660 directly, or @peisuke wants to push the rebase themselves, this PR can be closed — the goal is just to unblock the fix.

The bug

OAuthContext.lock is an anyio.Lock, which records task identity at acquire() and enforces same-task release(). async_auth_flow holds this lock across yield points. When httpx drives the generator from a different task during concurrent OAuth connections (e.g. Notion's frequent token refresh behind a gateway), release() raises RuntimeError: The current task is not holding this lock.

The fix (@peisuke's approach)

Narrow the lock scope so no HTTP yield (including the long-poll GET SSE and the token-refresh round trips) runs while holding context.lock, plus a single-flight refresh_lock with a re-check under the lock. Keeps trio portability (unlike swapping to asyncio.Lock).

Verification

Rebased onto current main. Single commit (authored by @peisuke), touches only src/mcp/client/auth/oauth2.py and tests/client/test_auth.py (+348/-18).

$ uv run pytest tests/client/test_auth.py -q
131 passed, 1 xfailed

$ uv run ruff check src/mcp/client/auth/oauth2.py tests/client/test_auth.py
All checks passed!

…poll requests

Closes modelcontextprotocol#2847.

OAuthContext.lock is an anyio.Lock, which records task identity at acquire()
and enforces same-task release(). async_auth_flow held this lock across yield
points; when httpx drives the generator from a different task during concurrent
OAuth connections, release() raises 'RuntimeError: The current task is not
holding this lock'.

Narrows the lock scope so no HTTP yield (long-poll GET SSE, token-refresh round
trips) runs while holding context.lock, plus a single-flight refresh_lock with
a re-check under the lock. Keeps trio portability (no asyncio.Lock swap).

Salvage of modelcontextprotocol#2660 by @peisuke, rebased onto current main.
@Bartok9 Bartok9 force-pushed the salvage/2660-oauth-lock-rebase branch from 425161d to a9be0b9 Compare June 23, 2026 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants