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
706 stars 66 forks source link

Memory leak #324

Closed FlorianVen closed 1 year ago

FlorianVen commented 1 year ago

We seem to be encountering a memory leak on v5.0.0-beta.11, reproducer here: https://github.com/FlorianVen/amphp-http-client-mem-leak-repro

Relevant code:

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

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

$request = static function () {
    $client = (new HttpClientBuilder())->build();

    $request = new Request('https://httpbin.org/get');

    $response = $client->request($request);
    $response->getBody()->buffer();
};

for ($i = 0; $i < 3; $i++) {
    async($request(...))->await();

    gc_collect_cycles();
    gc_mem_caches();

    echo memory_get_usage() . PHP_EOL;
}

I would have expected the reported memory usage to stay equal, instead it increases with each iteration (example down below).

The best clue I managed to find was that for each request performed, two callbacks remain in the EventLoop callbacks queue never seem to disappear ("k" and "l" in this screenshot after the first request): image

I have tried finding the source of these callbacks but did not manage to find them.

PHP information + example run:

❯ php -v
PHP 8.1.18 (cli) (built: Apr 14 2023 04:39:44) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.18, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.18, Copyright (c), by Zend Technologies

❯ php -m
[PHP Modules]
apcu
bcmath
calendar
Core
ctype
curl
date
dom
exif
FFI
fileinfo
filter
ftp
gd
gettext
hash
iconv
igbinary
intl
json
libxml
mbstring
mysqli
mysqlnd
openssl
pcntl
pcre
PDO
pdo_mysql
pdo_pgsql
pgsql
Phar
posix
readline
redis
Reflection
session
shmop
SimpleXML
soap
sockets
sodium
SPL
standard
sysvmsg
sysvsem
sysvshm
tokenizer
xml
xmlreader
xmlwriter
xsl
Zend OPcache
zip
zlib

[Zend Modules]
Zend OPcache

❯ php src/repro.php 
35915024
35977616
36040744

Please let me know what you think or if you need anything else.

kelunik commented 1 year ago

@FlorianVen Thanks for reporting and providing a reproduction case. Could you please try running it with https://github.com/revoltphp/event-loop/pull/75/files applied to see whether that fixes the leak?

FlorianVen commented 1 year ago

@kelunik Thanks for the quick reply, I've just applied the changes to the reproducer and they unfortunately do not fix the leak.

FYI: I'll be away the coming week, @Liiva will take over for me.

trowski commented 1 year ago

Could you please try https://github.com/revoltphp/event-loop/pull/78 instead? That PR takes a different approach to fixing the same leak in non-resumed fibers.

Liiva commented 1 year ago

@trowski I applied the changes and no luck unfortunately, still seeing a memory usage increase each iteration.

kelunik commented 1 year ago

@FlorianVen Please retry. Note that you should usually reuse the http client instance so the connections are reused.

kelunik commented 1 year ago

@FlorianVen Please retry. Note that you should usually reuse the http client instance so the connections are reused.

FlorianVen commented 1 year ago

Issue seems fixed, thanks a lot!

xpader commented 1 year ago

Looks like memory leak still exists in 4.x version, after switch to FFI HPackNgHttp2 to process http2 connection, process running after some days, php memory_get_usage() report about 100+MB memory usage, but in top, ps, php process memory up to 6GB, what??

This is the problems after I switch to FFI: https://github.com/amphp/hpack/issues/12

We have a lots of http connection job in whole day, beforer switch to ffi, memory never grow to GB.