From d618328cb0788da35aecfb7912ed0b3ab8244e8f Mon Sep 17 00:00:00 2001 From: David Grudl Date: Wed, 4 Nov 2026 16:23:13 +0100 Subject: [PATCH] [Server] Fix stdio server ending by itself on an idle Windows pipe On Windows, PHP waits for data on a pipe in a loop of 3,200,000 usleep(10) calls, also in non-blocking mode. One call takes about 15 ms there, so fgets() on an idle pipe blocks for about 13.5 hours and then reports a false EOF, which ends listen(). Reading only after stream_select() reports input stays out of that loop. It needs PHP 8.5, where stream_select() stopped reporting idle pipes as readable (php/php-src#16889). stream_select() throws a ValueError on a stream it cannot select, like php://memory, instead of returning false. --- CHANGELOG.md | 2 + src/Server/Transport/StdioTransport.php | 17 ++++--- .../Server/Transport/StdioTransportTest.php | 45 +++++++++++++++++++ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e70a3223..90f0c765 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ All notable changes to `mcp/sdk` will be documented in this file. * [BC Break] Answer `server/discover` with `DiscoverHandler`, which a handler added through `Builder::addRequestHandler()` can replace, and dispatch `RequestEvent`, `ResponseEvent` and `ErrorEvent` for it. A `StatelessProtocol` built without the `Builder` needs `DiscoverHandler` and a `MessageFactory` with `DiscoverRequest` to answer it. * Add a `binder` argument to `Builder::setRequestState()` binding the modern era's `requestState` to the request it was minted for, as the 2026-07-28 specification recommends: the binder's value is covered by the state's MAC, and a state echoed on a request that binds to another value is refused like a forged one. `OriginatingRequestBinder` binds the authenticated principal, the method, the tool, prompt or resource, and a digest of the arguments; a token without a subject fails closed, and a `subject` resolver identifies users by another claim. `RequestStateBinderInterface` takes custom bindings. Without a binder, nothing changes. * Rotate the `requestState` signing key without failing the calls in flight: `Builder::setRequestState()` and `RequestStateCodec` take a list of keys, the first signing and every key verifying. The wire format is unchanged, and a single key works as before. +* Fix a stdio server on Windows ending by itself after about 13.5 hours without input, when `fgets()` on the idle pipe reported a false EOF: `StdioTransport` now reads only once `stream_select()` reports input, which also keeps the run loop going while idle. Requires PHP 8.5, as `stream_select()` on Windows reports an idle pipe as readable before that. +* Fix `StdioTransport` throwing a `ValueError` on an input that cannot be selected, like `php://memory`. ### Client diff --git a/src/Server/Transport/StdioTransport.php b/src/Server/Transport/StdioTransport.php index 54ecd2cb..1998e6d8 100644 --- a/src/Server/Transport/StdioTransport.php +++ b/src/Server/Transport/StdioTransport.php @@ -162,10 +162,13 @@ public function listen(): int protected function processInput(): void { + // fgets() on an idle Windows pipe blocks even in non-blocking mode, then reports a false EOF. + if (!$this->waitForInput()) { + return; + } + $line = fgets($this->input, $this->maxLineBytes); if (false === $line) { - $this->waitForInput(); - return; } @@ -202,14 +205,16 @@ protected function processInput(): void * Idles until input arrives, for at most 50ms, so the run loop still gets * to suspended fibers, open streams and queued messages in between. */ - private function waitForInput(): void + private function waitForInput(): bool { $read = [$this->input]; $write = $except = null; - // Streams that cannot be selected, like php://memory, fall back to sleeping. - if (false === @stream_select($read, $write, $except, 0, 50000)) { - usleep(50000); + // A stream that cannot be selected, like php://memory, warns and throws, so it is just read. + try { + return 0 !== @stream_select($read, $write, $except, 0, 50000); + } catch (\ValueError) { + return true; } } diff --git a/tests/Unit/Server/Transport/StdioTransportTest.php b/tests/Unit/Server/Transport/StdioTransportTest.php index faa8a0df..b955bc07 100644 --- a/tests/Unit/Server/Transport/StdioTransportTest.php +++ b/tests/Unit/Server/Transport/StdioTransportTest.php @@ -89,6 +89,51 @@ public function testIdleInputWakesOnArrival(): void $this->assertLessThan(45, ($arrivedAt - $answeredAt) / 1e6, 'the idle wait must end when input arrives'); } + #[TestDox('an idle pipe does not hold up the run loop')] + public function testIdlePipeDoesNotHoldUpTheRunLoop(): void + { + if (!\function_exists('proc_open')) { + $this->markTestSkipped('proc_open() is needed to feed the transport from a real pipe.'); + } + + if ('Windows' === \PHP_OS_FAMILY && \PHP_VERSION_ID < 80500) { + $this->markTestSkipped('Before PHP 8.5, stream_select() on Windows reports an idle pipe as readable.'); + } + + $process = proc_open([\PHP_BINARY, '-r', 'sleep(3);'], [1 => ['pipe', 'w']], $pipes); + $this->assertIsResource($process); + stream_set_blocking($pipes[1], false); + + $transport = new StdioTransport(input: $pipes[1], output: $this->stream('')); + $start = hrtime(true); + (new \ReflectionMethod($transport, 'processInput'))->invoke($transport); + $elapsed = (hrtime(true) - $start) / 1e6; + + proc_terminate($process); + proc_close($process); + + $this->assertLessThan(1000, $elapsed, 'a tick must not wait for input longer than the idle wait'); + } + + #[TestDox('an input that cannot be selected is still read')] + public function testUnselectableInputIsRead(): void + { + $input = fopen('php://memory', 'r+'); + $this->assertNotFalse($input); + fwrite($input, '{"valid":1}'."\n"); + rewind($input); + + $messages = []; + $transport = new StdioTransport(input: $input, output: $this->stream('')); + $transport->onMessage(static function ($transport, string $payload) use (&$messages): void { + $messages[] = $payload; + }); + + $this->pumpToEof($transport); + + $this->assertSame(['{"valid":1}'], $messages); + } + #[TestDox('the line byte cap must be a positive number of bytes')] public function testRejectsNonPositiveCap(): void {