Skip to content

Commit da06f4a

Browse files
committed
[Server] Stop the handshake-era SSE loop once the client disconnected
1 parent 1515509 commit da06f4a

3 files changed

Lines changed: 77 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ All notable changes to `mcp/sdk` will be documented in this file.
5959
* Fix a client `onProgress` callback receiving progress for an earlier request that timed out: progress now only reaches the callback of the request whose `progressToken` it carries.
6060
* Fix `Client::isConnected()` staying true after the stdio server process exited, which left later calls failing against the dead process.
6161
* Fix a client request that timed out (elicitation, sampling) staying in the session's pending requests: `Protocol::checkResponse()` now expires it, answering with the `Request timed out` error, so the transports no longer implement the timeout themselves.
62+
* Fix a handshake-era Streamable HTTP stream pinning its PHP worker until the elicitation (or other client request) times out after the client disconnected: the idle stream now sends a keep-alive comment and stops once the connection is aborted.
6263

6364
0.8.1
6465
-----

‎src/Server/Transport/StreamableHttpTransport.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,8 @@ protected function createStreamedResponse(): ResponseInterface
293293
flush();
294294
}
295295

296+
$lastKeepAlive = null;
297+
296298
while ($fiber->isSuspended()) {
297299
$this->flushOutgoingMessages($this->sessionId);
298300

@@ -319,6 +321,24 @@ protected function createStreamedResponse(): ResponseInterface
319321
}
320322

321323
if (!$resumed) {
324+
// PHP only notices a dropped peer when it writes, so the
325+
// idle stream writes, or a vanished client would pin this
326+
// worker until the pending request times out. Paced like
327+
// the stateless keep-alive, not at every tick.
328+
$now = microtime(true);
329+
if (null === $lastKeepAlive || $now - $lastKeepAlive >= 0.25) {
330+
$lastKeepAlive = $now;
331+
echo ": keep-alive\n\n";
332+
@ob_flush();
333+
flush();
334+
}
335+
336+
if ($this->isConnectionAborted()) {
337+
$this->logger->info('SSE: Client disconnected, dropping the suspended request.');
338+
339+
return;
340+
}
341+
322342
usleep(100000);
323343
} // Prevent tight loop
324344
}
@@ -344,6 +364,11 @@ protected function createStreamedResponse(): ResponseInterface
344364
return $response;
345365
}
346366

367+
protected function isConnectionAborted(): bool
368+
{
369+
return 1 === connection_aborted();
370+
}
371+
347372
/**
348373
* @param McpFiber $fiber
349374
*/

‎tests/Unit/Server/Transport/StreamableHttpTransportTest.php‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,57 @@ public function testPollingLoopResumesFiberWithTimeoutErrorFromResponseFinder():
448448
$this->assertInstanceOf(Error::class, $received);
449449
}
450450

451+
#[TestDox('the polling loop stops once the client has disconnected')]
452+
public function testPollingLoopStopsOnceTheClientHasDisconnected(): void
453+
{
454+
$request = $this->factory
455+
->createServerRequest('POST', 'http://localhost/')
456+
->withHeader('Host', 'localhost')
457+
->withBody($this->factory->createStream('{"jsonrpc":"2.0","id":1,"method":"ping"}'));
458+
459+
$transport = new class($request, $this->factory, $this->factory) extends StreamableHttpTransport {
460+
protected function isConnectionAborted(): bool
461+
{
462+
return true;
463+
}
464+
};
465+
466+
$received = null;
467+
$fiber = new \Fiber(static function () use (&$received) {
468+
$received = \Fiber::suspend();
469+
470+
return null;
471+
});
472+
$fiber->start();
473+
474+
$transport->onMessage(static function (TransportInterface $transport) use ($fiber): void {
475+
$transport->attachFiberToSession($fiber, Uuid::v4());
476+
});
477+
$transport->setOutgoingMessagesProvider(static fn (): array => []);
478+
$transport->setResponseFinder(static fn () => null);
479+
$transport->setPendingRequestsProvider(static fn (): array => [
480+
['request_id' => 1, 'timestamp' => time(), 'timeout' => 120],
481+
]);
482+
483+
$response = $transport->listen();
484+
485+
$output = '';
486+
ob_start(static function (string $chunk) use (&$output): string {
487+
$output .= $chunk;
488+
489+
return '';
490+
});
491+
try {
492+
$response->getBody()->getContents();
493+
} finally {
494+
ob_end_flush();
495+
}
496+
497+
$this->assertSame(": keep-alive\n\n", $output);
498+
$this->assertTrue($fiber->isSuspended(), 'The abandoned call must not be resumed.');
499+
$this->assertNull($received);
500+
}
501+
451502
/**
452503
* @return iterable<string, array{bool}>
453504
*/

0 commit comments

Comments
 (0)