fix: #3585928 Session lock delays elicitation and sampling replies until the lease expires

Closes #3585928 (closed)

Problem

McpServerController::handle() holds the mcp_session: lock until a streamed response ends. While a tool waits in ClientGateway::elicit(), sample() or listRoots(), the SSE loop polls the session for the client's answer, and the client sends that answer as another POST on the same session. That POST waits in LockBackendInterface::wait() until the 30-second lease expires, so every round trip costs about 30 seconds, and after the expiry both requests write the session at once. #3585940 (closed) reported the same thing and is closed as a duplicate.

Resolution

Release the lock while the stream waits, and take it again around each session write the SSE loop makes.

SessionLockingStreamableHttpTransport extends the SDK transport and wraps the three hooks where the loop reads and rewrites the session: getOutgoingMessages() (consumes the queue), checkForResponse() (consumes a stored answer) and handleFiberYield() (queues the next request or notification). When the stream starts, the controller switches the transport to per-write locking and releases its own hold. The answer POST takes the lock as usual, stores the answer and releases it, so it gets in between polls. The hooks have the same signatures in mcp/sdk 0.7.1 and 0.8.x.

Bypassing the lock for answer-only POSTs is not safe while the SDK saves the session as one blob (php-sdk#275): an unlocked answer write would race the loop's queue rewrite every 100 ms, losing either the answer (the tool then times out after 120 s) or the queue.

If a transaction is open when the stream starts, the controller keeps the old behavior and holds the lock to the end, because the tool's rollback would undo the release (#3585923 (closed)).

Tests

McpServerFunctionalTest::assertElicitationAnswerResumesToolPromptly() calls a fixture tool that elicits, reads the elicitation from the open event stream, answers it with a second POST, and asserts the tool's result arrives within 10 seconds. On 2.x it fails at 30.2 s; with this change it passes in about 4 s. The streamed request turns off the HTML debug output, which reads the whole body and would block on an open stream.

Ran the module's tests (61 pass), phpcs (Drupal, DrupalPractice) and phpstan.

Remaining

  • Two concurrent tool calls on one session that both elicit can pick up each other's answers: the SDK loop checks every pending request on the session, not only its own. This was already possible after the lease expired; it now happens without the 30-second wait.
  • A tool that elicits inside an open transaction still waits for the lease. Eliciting inside a transaction cannot work anyway: on MySQL the stream never sees the committed answer, and on SQLite the answer cannot be written.
  • While a stream waits, it takes and releases the lock about twice per 100 ms poll.

Closes #3585928 (closed)

Merge request reports

Loading
Loading