guzzle / promises

Promises/A+ library for PHP with synchronous support
MIT License
7.61k stars 116 forks source link

Memory leak when using \GuzzleHttp\Pool with empty array of requests #143

Closed xdanik closed 3 years ago

xdanik commented 3 years ago

PHP version: 7.4.12 Description

Creating \GuzzleHttp\Pool with $requests being empty array/iterator, and calling $pool->promise() causes memory leak as the promise is never freed from memory. Its caused by GuzzleHttp\Promise\EachPromise->createPromise() that registers $clearFn in order to remove references after the promise is resolved. But $clearFn is never called in case of empty iterator / no promises to be handled.

How to reproduce

$client = new \GuzzleHttp\Client();
while (true) {
    $requests = [];
    /* Uncomment me to fix the issue
    $requests[] = static function () {
        return null;
    };
    */
    $pool = new \GuzzleHttp\Pool($client, $requests);
    $promise = $pool->promise();
    $promise->wait(false);

    unset($requests, $pool, $promise);
    echo memory_get_peak_usage() . PHP_EOL;
}

Possible Solution Call $clearFn immediately if $this->pending is empty inside GuzzleHttp\Promise\Eachpromise->createPromise() 🤔.

GrahamCampbell commented 3 years ago

What version of this library are you using please? I think we may have already fixed this?

xdanik commented 3 years ago

I am using guzzle 6.5.5 + promises 1.4.1, but I have just tested that it also happens on guzzle 7.3.0 + promises 1.5.0.

GrahamCampbell commented 3 years ago

Thanks. Are you able to submit a PR to fix this please.


Unrelated, but note that Guzzle 6 is EOL.

xdanik commented 3 years ago

@GrahamCampbell Here you go 😉.