php-http / client-common

Common HTTP Client implementations and tools for HTTPlug
http://httplug.io
MIT License
1.02k stars 54 forks source link

Memory leak hotfix #184

Closed thomasvargiu closed 4 years ago

thomasvargiu commented 4 years ago

Hi, there is a big memory leak when using the PluginClient with plugins.

Example code to reproduce the issue:

use Http\Client\Curl\Client;

$samples = 50;
$curlClient = new Client();
$requestFactory = $container->get(\Psr\Http\Message\RequestFactoryInterface::class);
$pluginClient = new \Http\Client\Common\PluginClient(
    $curlClient,
    [
        new \Http\Client\Common\Plugin\RedirectPlugin(),
    ]
);

$request = $requestFactory->createRequest('GET', 'https://google.it');
for ($i = 1; $i <= $samples; $i++) {
    $pluginClient->sendRequest($request);
    echo 'memory: ' . memory_get_usage() . PHP_EOL;
    // gc_collect_cycles(); without this, gc isn't able to free memory
}

This PR fixes it removing useless variable reference.

GrahamCampbell commented 4 years ago

Tests are failing now?

thomasvargiu commented 4 years ago

Yes you're right, what I wrote is breaking the chain. I'm going to look at it better

thomasvargiu commented 4 years ago

So, the problem is on passing the first callable to plugins, it creates a reference loop. The only way to resolve it, is to create a new chain of callables for every plugin request, using the new PluginChain instance as first callable.

dbu commented 4 years ago

Please revert the CS fixes, then rebase.

@GrahamCampbell is the change with the plugin chain good otherwise?

GrahamCampbell commented 4 years ago

Yeh, if you extract just the changes to the files that are needed (I think 3 files?).

GrahamCampbell commented 4 years ago

Sorted out a replacement here: https://github.com/php-http/client-common/pull/194.