[databricks-mcp] Fix streamable-HTTP deadlock by not holding OAuth lock across requests#442
Open
sunishsheth2009 wants to merge 1 commit into
Open
Conversation
…DatabricksOAuthClientProvider
`DatabricksOAuthClientProvider` extends `mcp.client.auth.OAuthClientProvider`,
whose `async_auth_flow` does:
async with self.context.lock:
...
response = yield request # lock held while httpx awaits the response
...
i.e. the lock is held for the entire HTTP request lifetime, not just the
moments around the yield.
`mcp.client.streamable_http.streamablehttp_client` spawns two concurrent
tasks against the same auth provider:
* a long-lived GET (`handle_get_stream`) that opens an SSE channel for
server-pushed events,
* per-JSON-RPC POSTs (`_handle_post_request`) for `tools/list`,
`tools/call`, ...
For most Databricks-fronted MCP servers reached through the AI Gateway
UC-Connection proxy (`/api/2.0/mcp/external/<connection>`) — Atlassian,
Jira, Confluence, ... — the GET keeps the SSE channel open indefinitely
waiting on server events. The auth lock is acquired first by that GET
task and is never released, so every POST queues behind it and eventually
surfaces as:
mcp.shared.exceptions.McpError: Timed out while waiting for response
to ClientRequest. Waited 20.0 seconds.
even though the upstream MCP server is healthy (a bare `httpx` call —
no auth provider, no concurrent GET — returns the same response in <1s).
The Databricks scenario doesn't need any of the OAuth-dance machinery
the lock guards: there is no client registration, no PKCE, no callback
handlers, no refresh-token flow — the credential is whatever
`WorkspaceClient.config.authenticate()` already produced.
This change overrides `async_auth_flow` in `DatabricksOAuthClientProvider`
to stamp the Authorization header and yield, without acquiring the
parent's lock. The override keeps the same constructor signature and
the `databricks_token_storage` attribute, so existing callers and the
existing token-storage tests continue to work unchanged.
Added regression test `test_async_auth_flow_does_not_serialize_concurrent_requests`
which interleaves two `async_auth_flow` generators (mirroring the GET +
POST pattern of the streamable-HTTP transport) and asserts neither blocks
on the other.
Co-authored-by: Isaac
77f452f to
4376c2b
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
DatabricksOAuthClientProviderinherits frommcp.client.auth.OAuthClientProvider, whoseasync_auth_flowholdsself.context.lockacross theyield requestthat suspends until the HTTP response arrives. Combined withmcp.client.streamable_http.streamablehttp_client's pattern of running a long-lived GET (handle_get_stream) alongside per-JSON-RPC POSTs against the same auth provider, this deadlocks every POST whenever the GET's SSE channel stays open — which is the common case for UC-Connection-backed MCP servers (Atlassian, Jira, Confluence).The user-visible symptom is:
…on the first
tools/listcall after__aenter__, even though the same request issued via a barehttpxcall (no auth provider, no concurrent GET) returns in <1 second.Root cause
Both
handle_get_streamand_handle_post_requestroute through the sameDatabricksOAuthClientProviderinstance. The GET entersasync_auth_flow, acquires the lock, yields the request, and httpx then awaits the response — which never arrives for an open SSE channel. The lock stays held; every POST blocks onasync with self.context.lock:until the session-levelanyio.fail_after(client_session_timeout_seconds)fires.Fix
Override
async_auth_flowinDatabricksOAuthClientProviderto stamp the Authorization header fromWorkspaceClient.config.authenticate()and yield, without acquiring the parent's lock. The full OAuth dance the lock guards (client registration, PKCE, callback handlers, refresh-token flow) is unused in the Databricks scenario — the credential is fully managed byWorkspaceClient.Constructor signature and the
databricks_token_storageattribute are preserved; existing callers and the existing token-storage tests are unchanged.Test
Added
test_async_auth_flow_does_not_serialize_concurrent_requests: opens anasync_auth_flowgenerator for a "GET" without closing it (mirrors the open SSE channel), then opens a secondasync_auth_flowgenerator for a "POST" and asserts it advances within a 1-second timeout. With the old behavior the POST hangs on the lock; with the override it completes immediately.Also added
test_async_auth_flow_stamps_bearer_tokento pin the basic behavior (Authorization header is added, generator completes after one yield).Repro context
Discovered while running an OpenAI Agents SDK agent against an external UC-Connection MCP (Atlassian) through
/api/2.0/mcp/external/<connection>. Every first-turntools/listfromagents.mcp.MCPServerStreamableHttp(which inherits the same MCP client transport) failed withMcpError: Timed out … 20.0 seconds. A side-by-side directhttpxPOST to the same URL (with the same OBO bearer token, noOAuthClientProviderconfigured) succeeded in <1s and returned all tools, confirming the latency was introduced by the auth provider rather than the proxy or upstream server.Test plan
databricks_mcp/tests/unit_tests/test_oauth_provider.py—test_async_auth_flow_stamps_bearer_tokendatabricks_mcp/tests/unit_tests/test_oauth_provider.py—test_async_auth_flow_does_not_serialize_concurrent_requeststest_oauth_providerandtest_authenticate_raises_exception(token-storage path) still pass — that code path is unchanged.This pull request and its description were written by Isaac.