Skip to content

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

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

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

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.

1 participant