Skip to content

Conversation

danog
Copy link
Contributor

@danog danog commented Jun 25, 2025

Fixes #395, the root cause is likely some leaked reference somewhere and this is more of a hotfix, but it does the job.

@trowski
Copy link
Member

trowski commented Jul 14, 2025

Commented on the source issue that I'm not able to reproduce this issue this is fixing. I can envision scenarios where the connection pool is destroyed before a request has completed, so I'm not eager to abruptly close the connections.

@kelunik
Copy link
Member

kelunik commented Jul 23, 2025

I have similar thoughts, so I don’t think we should accept this change.

@danog
Copy link
Contributor Author

danog commented Jul 23, 2025

The fully reproduces the leaks:

<?php

require 'vendor/autoload.php';

use Amp\Http\Client\Request;
use Amp\Http\Client\HttpClientBuilder;
use Amp\Http\HttpStatus;
use Amp\Http\Server\DefaultErrorHandler;
use Amp\Http\Server\Request as ServerRequest;
use Amp\Http\Server\RequestHandler;
use Amp\Http\Server\Response;
use Amp\Http\Server\SocketHttpServer;
use Amp\Log\ConsoleFormatter;
use Amp\Log\StreamHandler;
use Monolog\Logger;
use Monolog\Processor\PsrLogMessageProcessor;
use Psr\Log\NullLogger;

use function Amp\async;
use function Amp\ByteStream\getStdout;
use function Amp\Future\await;

$requestHandler = new class() implements RequestHandler {
    public function handleRequest(ServerRequest $request) : Response
    {
        return new Response(
            status: HttpStatus::OK,
            headers: ['Content-Type' => 'text/plain'],
            body: 'Hello, world!',
        );
    }
};
$errorHandler = new DefaultErrorHandler();

$server = SocketHttpServer::createForDirectAccess(new NullLogger);
$server->expose('127.0.0.1:1337');
$server->start($requestHandler, $errorHandler);

ini_set('memory_limit', '8M');

final class Test {

	public static function t(): void {

		$ips = [];
		$counter = 0;
		for ($x = 0; $x < 1000000; $x++) {
			$ip = long2ip($x);
			$ips[$ip] = $ip;

			if ($x % 1000 === 0) {
				echo "counter: $counter\n";
				self::resolveIps($ips);
				$ips = [];
			}
		}
	}

	private static function resolveIps(array $ips, int $threads = 10): void {
		$client = (new HttpClientBuilder())
			->retry(0)
			->build()
		;

		$futures = [];
		foreach ($ips as $ip => &$item) {
			if (str_starts_with($ip, '172.')) {
				$item = [
					'country' => '',
					'city' => '',
				];
				continue;
			}
			$futures[] = async(function () use (&$client, $ip, &$item) {
                $r = new Request('http://127.0.0.1:1337/?ip=' . $ip);
                $r->setHeader('Connection', 'keep-alive');
				$response = $client->request($r);
				$responseString = $response->getBody()->buffer();
			});

			if (count($futures) % $threads === 0) {
				await($futures);
				$futures = [];
			}

		}unset($item);

		if (count($futures) > 0) {
			await($futures);
			unset($futures);
		}
	}
}


Test::t();

@danog
Copy link
Contributor Author

danog commented Aug 5, 2025

Ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Connections leak when destroying client
3 participants