Skip to content

fix(oauth): avoid lock contention for long-running requests - #3243

Closed
ctaylor86 wants to merge 3 commits into
modelcontextprotocol:v1.xfrom
ctaylor86:fix/oauth-lock-scope-v1
Closed

fix(oauth): avoid lock contention for long-running requests#3243
ctaylor86 wants to merge 3 commits into
modelcontextprotocol:v1.xfrom
ctaylor86:fix/oauth-lock-scope-v1

Conversation

@ctaylor86

Copy link
Copy Markdown

Summary

  • move normal OAuth-authenticated resource requests outside the shared provider-state lock
  • add a dedicated refresh lock so concurrent refreshes remain single-flight
  • retry stale 401 responses with a token refreshed by another request, without holding the state lock
  • add regression coverage for long-running GET/SSE concurrency, refresh single-flight, failed refresh cleanup, and stale-token retries

Problem

OAuthClientProvider.async_auth_flow() currently holds context.lock across response = yield request. A Streamable HTTP or SSE GET can remain open for the lifetime of the MCP session, so every concurrent POST auth flow waits forever on that lock. This affects clients that reuse persisted OAuth credentials and can also surface as a lock-owner cleanup error when the transport is cancelled.

This backports the lock-scope direction from #2660 / #2858 to the maintained v1.x branch. OAuth 401/403 state transitions remain serialized, but normal resource requests and stale-token retries yield outside the lock. Refresh uses a separate lock and re-checks token validity after acquiring it to avoid duplicate refreshes.

Fixes #1326.
Relates to #2847 and #2858.

Validation

  • uv run --frozen pytest -n 0 -q: 1162 passed, 95 skipped, 1 xfailed
  • uv run --frozen ruff check .: passed
  • uv run --frozen ruff format --check .: passed
  • uv run --frozen pyright: passed
  • downstream Hermes MCP suite: 423 passed
  • live OAuth Streamable HTTP interoperability: connected in under 1 second and listed 171 tools; two existing downstream MCP servers also remained healthy (24 and 7 tools)

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the contribution. This repository only keeps pull requests open when they're linked to an issue that a maintainer has assigned to the author — CONTRIBUTING.md explains why and how we work. This PR has been closed for now because you aren't currently assigned to #1326.

If a maintainer would like this change as a PR from you, they'll assign you to #1326 and this PR will reopen automatically — there's nothing more you need to do. (If you opened the issue, this PR already shows up on its timeline.)

There's no need to open a new PR — this one will be reopened. While it's closed, please push any updates as new commits rather than force-pushing, since GitHub can't reopen a PR whose branch has been rewritten.

Maintainers: reopening this PR, removing the missing-issue-link label, or adding bypass-issue-check bypasses the check.

@github-actions github-actions Bot added the missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) label Aug 17, 2026
@github-actions github-actions Bot closed this Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant