Skip to content

Replacement memory leak hotfix #194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 1, 2020
Merged

Replacement memory leak hotfix #194

merged 4 commits into from
Jul 1, 2020

Conversation

GrahamCampbell
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets fixes #184
License MIT

Original description:

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 GrahamCampbell changed the title Replacement Memory leak hotfix Replacement memory leak hotfix Jul 1, 2020
@GrahamCampbell GrahamCampbell marked this pull request as ready for review July 1, 2020 14:21
@GrahamCampbell GrahamCampbell mentioned this pull request Jul 1, 2020
Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please add a note to the changelog? this could also help in case there is some unexpected side effect, to help people figure out what to look at if they see a problem after upgrading.

@GrahamCampbell
Copy link
Contributor Author

I'll send a change log PR to add everything at the end if that's ok? This is not the only change worth noting since the last release. :)

@dbu
Copy link
Contributor

dbu commented Jul 1, 2020

i usually try to keep the changelog updated constantly, as otherwise these things get forgotten. but with all the parallel things you do that would mean conflicts and rebases after every merge, and i trust you to not forget it at the end :-)

@dbu dbu merged commit 6aaedfa into php-http:master Jul 1, 2020
@GrahamCampbell GrahamCampbell deleted the memory-leak branch July 1, 2020 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants