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
Open
Conversation
This was referenced Jun 13, 2026
Open
8ffed4a to
425161d
Compare
…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.
425161d to
a9be0b9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 againstmainand squashed to a single commit.Closes #2847. Addresses the same
RuntimeError: The current task is not holding this lockreported in #2644 and #2847.Why a new PR
#2660 has been a
CONFLICTINGdraft 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.lockis ananyio.Lock, which records task identity atacquire()and enforces same-taskrelease().async_auth_flowholds this lock acrossyieldpoints. When httpx drives the generator from a different task during concurrent OAuth connections (e.g. Notion's frequent token refresh behind a gateway),release()raisesRuntimeError: 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 holdingcontext.lock, plus a single-flightrefresh_lockwith a re-check under the lock. Keeps trio portability (unlike swapping toasyncio.Lock).Verification
Rebased onto current
main. Single commit (authored by @peisuke), touches onlysrc/mcp/client/auth/oauth2.pyandtests/client/test_auth.py(+348/-18).