SocketHttpServer never closes the client when the HTTP driver returns normally (fd leak, can pin a core at 100%)
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 1.3k
- Forks
- 103
- PR merge metrics
- No merged PRs in 30d
Description
Summary
SocketHttpServer::handleClient() only closes the client on three paths: a failed TLS handshake, a not-yet-started server, or an exception reaching the catch block. When $driver->handleClient() returns normally, the finally block just unsets the driver from the map and the socket is never closed:
try {
$driver->handleClient($client, $socket, $socket);
} finally {
unset($this->drivers[$id]); // <- no $client->close()
}
Nothing releases the descriptor afterwards. Neither ReadableResourceStream::close() nor WritableResourceStream::close() calls fclose() on a socket — each does stream_socket_shutdown(SHUT_RD/SHUT_WR) and drops its own reference, so the fd is freed only once both halves free it and the refcount reaches zero. With no close() at all, the descriptor and its event-loop watcher are retained for the lifetime of the process.
Version
amphp/http-server 3.4.5 (also present in 3.x main as far as I can tell), amphp/byte-stream 2.1.2, PHP 8.4.
Reproduction
Deterministic — no timing dependency. A trivial HttpDriver that returns from handleClient() reproduces the same control flow a real driver takes at end-of-connection:
$noopDriver = new class implements HttpDriver {
public ?Client $seen = null;
public function handleClient(Client $client, ReadableStream $r, WritableStream $w): void {
$this->seen = $client; // return normally, as at end-of-connection
}
public function getPendingRequestCount(): int { return 0; }
public function getApplicationLayerProtocols(): array { return []; }
public function stop(): void {}
};
$server = new SocketHttpServer(new NullLogger(), new ResourceServerSocketFactory(),
new SocketClientFactory(new NullLogger()), httpDriverFactory: $factoryReturning($noopDriver));
$server->expose(SocketAddress\fromString('127.0.0.1:0'));
$server->start($requestHandler, new DefaultErrorHandler());
$client = Socket\connect($server->getServers()[0]->getAddress()->toString());
$client->write("GET / HTTP/1.1\r\nHost: x\r\n\r\n");
\Amp\delay(0.3);
$client->close();
\Amp\delay(0.3);
var_dump($noopDriver->seen->isClosed()); // expected true
Output:
driver received a client: yes
client closed after driver returned: NO
RESULT: LEAKED - SocketHttpServer never closed the client
Impact
Beyond the descriptor leak, whether the leak is merely wasteful or actively burns a core depends on the write queue at the moment of abandonment. In WritableResourceStream's watcher finally:
- write queue empty →
EventLoop::disable()→ idle leak, no CPU cost - write queue non-empty → the watcher is left enabled
A dead socket is permanently "writable" as far as epoll is concerned, so in the second case the watcher retries a failing write forever. Measured at roughly 30k EPIPE writes/second, per leaked descriptor, indefinitely.
In a production deployment this surfaced as two cluster workers pinned at 100% CPU continuously for 19 days, holding 35 and 37 orphaned descriptors between them. The real-world trigger is a peer that RSTs a connection sitting idle in HTTP/1.1 keep-alive — i.e. ordinary internet scanner traffic against a public listener. Roughly 60% of such connections leaked in our testing.
Suggested fix
try {
$driver->handleClient($client, $socket, $socket);
} finally {
+ $client->close();
unset($this->drivers[$id]);
}
With that applied the reproduction above prints client closed after driver returned: yes, and the production symptom disappears: 15 runs of the triggering sequence produce zero orphaned descriptors and 0% idle CPU.
For what it's worth, this is what Amp\Cluster users are most exposed to, and it is also worth noting that at least one downstream project which modelled its own socket server on this method (Flyokai\DataService\DataServer\SocketDataServer::handleClient()) does call $client->close() in the equivalent finally — which is why its non-HTTP listener never exhibited the leak while the HTTP one did.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at SocketHttpServer::handleClient() and run the deterministic reproduction described in the issue, focusing on the normal-return path from HttpDriver::handleClient(). Verify that the client is closed after the driver returns and that repeated runs no longer retain orphaned descriptors or produce the reported busy-writer behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend, networking
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100