amphp / http-client

An advanced async HTTP client library for PHP, enabling efficient, non-blocking, and concurrent requests and responses.
https://amphp.org/http-client
MIT License
707 stars 66 forks source link

Memory issue in HTTP 2 connection #329

Closed FlorianVen closed 1 year ago

FlorianVen commented 1 year ago

Related to #324, while the reproducer in that issue was indeed fixed, we still experienced the issue on production.

I have narrowed the issue down to HTTP 2 connections, since httpbin.org seems to not support HTTP 2, the reproducer in #324 was fixed. Note that due to some internal reasons, we are not able to re-use the HttpClient for long and as such must re-create it periodically which causes the problem.

New reproducer uses an alternative httpbin that supports HTTP 2:

<?php

declare(strict_types=1);

use Amp\Http\Client\HttpClientBuilder;
use Amp\Http\Client\Request;
use function Amp\async;

include __DIR__ . '/../vendor/autoload.php';

function reportMemory(): void
{
    gc_collect_cycles();
    gc_mem_caches();
    echo memory_get_usage() . PHP_EOL;
}

reportMemory();
for ($i = 0; $i < 10; ++$i) {
    async(static function () {
        $client = (new HttpClientBuilder())->build();
        $request = new Request('https://www.nghttp2.org/httpbin/');
        $request->setProtocolVersions(['2']);
        $client->request($request)->getBody()->buffer();
    })->await();

    reportMemory();
}

Output:

1385408
37151456
37225792
37304112
37381120
37458128
...

I narrowed the problem down to the HttpStream::$releaseCallback not being called, because it is set to null in its request(...) method. This in turn prevents the connection from reaching an idle state and prevents clean up. I'll submit a pull request shortly.

kelunik commented 1 year ago

Fixed, in https://github.com/amphp/http-client/commit/d92c290fd30c21e7a93f0af0da38c55db1ee42aa, thanks!