From 8669d4cb4c39e59abf6c06596d48c4ada757a0d5 Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Thu, 14 May 2026 17:34:00 +0200 Subject: [PATCH 1/4] Fix idle HTTP/1 connection GC leak --- src/Connection/Http1Connection.php | 15 ++++++++++---- test/Connection/Http1ConnectionTest.php | 27 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/Connection/Http1Connection.php b/src/Connection/Http1Connection.php index 19a0b76d..c233b0b3 100644 --- a/src/Connection/Http1Connection.php +++ b/src/Connection/Http1Connection.php @@ -468,7 +468,11 @@ private function readResponse( $this->priorTimeout = $priorTimeout ?? $this->priorTimeout; if ($requestTimeout > 0 && $parser->getState() !== Http1Parser::BODY_IDENTITY_EOF) { - $this->timeoutWatcher = EventLoop::delay($requestTimeout, $this->close(...)); + $connectionRef = \WeakReference::create($this); + $this->timeoutWatcher = EventLoop::delay( + $requestTimeout, + static fn () => $connectionRef->get()?->close(), + ); EventLoop::unreference($this->timeoutWatcher); $this->watchIdleConnection(); } else { @@ -736,16 +740,19 @@ private function watchIdleConnection(): void $this->socket->unreference(); } - $this->idleRead = async(function (): ?string { + $socket = $this->socket; + $connectionRef = \WeakReference::create($this); + + $this->idleRead = async(static function () use ($socket, $connectionRef): ?string { $chunk = null; try { - $chunk = $this->socket?->read(); + $chunk = $socket?->read(); } catch (\Throwable) { // Close connection below. } if ($chunk === null) { - $this->close(); + $connectionRef->get()?->close(); } return $chunk; diff --git a/test/Connection/Http1ConnectionTest.php b/test/Connection/Http1ConnectionTest.php index 278e3eeb..1ffd4b36 100644 --- a/test/Connection/Http1ConnectionTest.php +++ b/test/Connection/Http1ConnectionTest.php @@ -80,6 +80,33 @@ public function testConnectionNotBusyWithoutRequestGarbageCollected(): void self::assertNotNull($connection->getStream($secondRequest)); } + public function testIdleKeepAliveConnectionCanBeGarbageCollected(): void + { + [$server, $client] = Socket\createSocketPair(); + + $connection = new Http1Connection($client, 0, null, 5); + $connectionRef = \WeakReference::create($connection); + + $request = new Request('http://localhost'); + events()->requestStart($request); + + $stream = $connection->getStream($request); + $server->write("HTTP/1.1 204 No Content\r\nConnection: keep-alive\r\nContent-Length: 0\r\n\r\n"); + + $response = $stream->request($request, new NullCancellation); + $response->getBody()->buffer(); + + unset($client, $connection, $request, $response, $stream); + + do { + delay(0); + } while (gc_collect_cycles()); + + self::assertNull($connectionRef->get()); + + $server->close(); + } + public function test100Continue(): void { [$server, $client] = Socket\createSocketPair(); From 0d309ee09657c4261847f7ed070be862ba95d37b Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Thu, 14 May 2026 17:40:59 +0200 Subject: [PATCH 2/4] fix --- test/Connection/Http1ConnectionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Connection/Http1ConnectionTest.php b/test/Connection/Http1ConnectionTest.php index 1ffd4b36..c6fb721b 100644 --- a/test/Connection/Http1ConnectionTest.php +++ b/test/Connection/Http1ConnectionTest.php @@ -100,7 +100,7 @@ public function testIdleKeepAliveConnectionCanBeGarbageCollected(): void do { delay(0); - } while (gc_collect_cycles()); + } while (\gc_collect_cycles()); self::assertNull($connectionRef->get()); From bc680b87eda879c0f00ff53e9911ef2cc5160911 Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Thu, 14 May 2026 17:46:22 +0200 Subject: [PATCH 3/4] Remove failed connection waiters --- src/Connection/ConnectionLimitingPool.php | 7 +++-- .../Connection/ConnectionLimitingPoolTest.php | 29 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/Connection/ConnectionLimitingPool.php b/src/Connection/ConnectionLimitingPool.php index 52c8f8ea..1bc38ea7 100644 --- a/src/Connection/ConnectionLimitingPool.php +++ b/src/Connection/ConnectionLimitingPool.php @@ -187,8 +187,9 @@ private function getStreamFor(string $uri, Request $request, Cancellation $cance $deferred = new DeferredFuture; $futureFromDeferred = $deferred->getFuture(); + $deferredId = \spl_object_id($deferred); - $this->waiting[$uri][\spl_object_id($deferred)] = $deferred; + $this->waiting[$uri][$deferredId] = $deferred; if ($this->isAdditionalConnectionAllowed($uri)) { break; @@ -251,10 +252,10 @@ private function getStreamFor(string $uri, Request $request, Cancellation $cance } catch (CompositeException $exception) { [$exception] = $exception->getReasons(); // The first reason is why the connection failed. throw $exception; + } finally { + $this->removeWaiting($uri, $deferredId); // DeferredFuture no longer needed for this request. } - $this->removeWaiting($uri, \spl_object_id($deferred)); // DeferredFuture no longer needed for this request. - \assert($connection instanceof Connection); $stream = $this->getStreamFromConnection($connection, $request); diff --git a/test/Connection/ConnectionLimitingPoolTest.php b/test/Connection/ConnectionLimitingPoolTest.php index e8570fb1..570d95b4 100644 --- a/test/Connection/ConnectionLimitingPoolTest.php +++ b/test/Connection/ConnectionLimitingPoolTest.php @@ -5,6 +5,7 @@ use Amp\ByteStream\ReadableBuffer; use Amp\Future; use Amp\Http\Client\HttpClientBuilder; +use Amp\Http\Client\SocketException; use Amp\Http\Client\Request; use Amp\Http\Client\Response; use Amp\Http\Client\Trailers; @@ -195,6 +196,34 @@ public function testConnectionNotClosedWhileInUse(): void } } + public function testWaitingRequestRemovedIfConnectionAttemptFails(): void + { + $factory = $this->createMock(ConnectionFactory::class); + $factory->expects(self::once()) + ->method('create') + ->willThrowException(new SocketException('Connection failed')); + + $pool = ConnectionLimitingPool::byAuthority(1, $factory); + + $client = (new HttpClientBuilder) + ->retry(0) + ->usingPool($pool) + ->build(); + + try { + $client->request(new Request('http://localhost')); + self::fail('Connection attempt should have failed'); + } catch (SocketException) { + // Expected. + } + + delay(0); + + $property = new \ReflectionProperty($pool, 'waiting'); + + self::assertSame([], $property->getValue($pool)); + } + private function createMockConnection(Request $request): Connection&MockObject { $response = new Response('1.1', 200, null, [], new ReadableBuffer, $request, Future::complete(new Trailers([]))); From 3f3b0a7fb0775bab90421dcdcebfdec294bfc587 Mon Sep 17 00:00:00 2001 From: Daniil Gentili Date: Thu, 14 May 2026 18:01:20 +0200 Subject: [PATCH 4/4] cs-fix --- test/Connection/ConnectionLimitingPoolTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Connection/ConnectionLimitingPoolTest.php b/test/Connection/ConnectionLimitingPoolTest.php index 570d95b4..9a119e79 100644 --- a/test/Connection/ConnectionLimitingPoolTest.php +++ b/test/Connection/ConnectionLimitingPoolTest.php @@ -5,9 +5,9 @@ use Amp\ByteStream\ReadableBuffer; use Amp\Future; use Amp\Http\Client\HttpClientBuilder; -use Amp\Http\Client\SocketException; use Amp\Http\Client\Request; use Amp\Http\Client\Response; +use Amp\Http\Client\SocketException; use Amp\Http\Client\Trailers; use Amp\PHPUnit\AsyncTestCase; use Amp\Socket\InternetAddress;