verdier commented on PR #43007:
URL: https://github.com/apache/superset/pull/43007#issuecomment-5250763884
Found a defect in my own semaphore while spiking something unrelated, so
flagging it before anyone else hits it — it is fixed in `70c258f`.
Tool search is enabled by default, and its `call_tool` proxy reaches the
target through `ctx.fastmcp.call_tool(name, arguments)`. `FastMCP.call_tool()`
has `run_middleware=True` by default, so **the middleware chain runs twice for
one client call**. My first version took a slot on each pass, which means every
call held one slot while waiting for a second: a guaranteed deadlock at the
limit — precisely the failure the limit exists to prevent, and in the default
configuration at that.
My harness never caught it because the POC config sets
`MCP_TOOL_SEARCH_CONFIG = {"enabled": False}` to exercise the full tool
catalogue. Worth knowing for anyone else measuring MCP concurrency: with tool
search on, you are measuring two middleware passes per call.
Admission is now re-entrant — a nested pass runs inside the slot the outer
pass took — with a test that drives proxy-then-target at a limit of one and
times out without the fix. Eight tests on the limiter now, full
`tests/unit_tests` still green.
Unrelated to this PR but found on the way, and filed separately as #43055:
the per-tool `mcp_auth_hook` decorator is doing a middleware's job, and the
startup assertion plus `ALLOWED_UNPROTECTED` allowlist exist only because it is
skippable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
