Repository navigation
memory leak when using fopen with a stream wrapper #8548
Description
Description
The following code takes up a lot of memory and keeps increasing in a endless loop:
#!/usr/bin/php
<?php
class Wrapper
{
public function stream_open(string $path, string $mode, int $options): bool
{
return true;
}
}
$filename = tempnam(__DIR__, '');
touch($filename);
$wrapper = new Wrapper();
$counter = 0;
while ($counter < 1_000_000) {
$counter++;
stream_wrapper_register('symfony', Wrapper::class);
fopen('symfony://' . $filename, 'r');
stream_wrapper_unregister('symfony');
}
With a small change, the problem disappears:
#!/usr/bin/php
<?php
class Wrapper
{
public function stream_open(string $path, string $mode, int $options): bool
{
return true;
}
}
$filename = tempnam(__DIR__, '');
touch($filename);
$wrapper = new Wrapper();
$counter = 0;
stream_wrapper_register('symfony', Wrapper::class);
while ($counter < 1_000_000) {
$counter++;
fopen('symfony://' . $filename, 'r');
}
stream_wrapper_unregister('symfony');
The memory leak was described in the following ticket: symfony/symfony#46256
PHP Version
PHP 8.1.5 (cli)
Operating System
Ubuntu 20.04
Reacted by Michael Voříšek
Activity
cmb69 commentedon May 16, 2022
The problem is that
stream_wrapper_register()allocates a resouce, but that is not deleted instream_wrapper_unregister()(but only during shutdown).Note that even when this will be fixed, it's still not a good idea to repeatly register and unregister stream wrappers, because of potential resource ID depletion.
olsavmic commentedon Aug 1, 2022
@iluuu1994 Is there any possibility of backporting/releasing this in 8.0.*/8.1.*?
Thanks!
iluuu1994 commentedon Aug 1, 2022
@olsavmic Since this is a rather theoretical and old leak I decided not to. Are you hitting this issue?
olsavmic commentedon Aug 1, 2022
Yes, pretty heavily due to symfony/http-client (symfony/symfony#46256). We have switched to guzzle client implementation which does not suffer from this issue so it's no longer critical for us but it's still an issue.
I will suggest a singleton-like approach for the stream_wrapper_registrater call in the Symfony repository but I don't see into the internals of streams and linux resource ids. I'd assume that resource id is freed after unregistration but perhaps that's not the case?
iluuu1994 commentedon Aug 1, 2022
Oh, I wasn't aware Symfony actually unregisters the stream wrapper on every call.
@nicolas-grekas Is unregistering really necessary or could Symfony check if the stream wrapper has already been registered?
If not we should probably be able to back-port a5a89cc and c019421 since there aren't any public ABI breaks. @cmb69 Would that be ok for you?
cmb69 commentedon Aug 1, 2022
I'm not hard against back-porting, but I really think this should be fixed in userland; repeatedly registering and unregistering stream wrappers is not a good idea anyway, because of potential resource ID depletion.
nicolas-grekas commentedon Aug 1, 2022
The goal of this code is to not leak anything in the global scope (aka not allow anyone but internal code to fopen() that kind of streams.) I didn't know this had any effect on resource ids. I'll look for changing this.
cmb69 commentedon Aug 1, 2022
All resources (regardless of kind) are internally stored in a hash table (see
get_resources()), and are basically managed with append ($a[] = …) and unset operations. As of PHP 8.1.0, resource ID depletion is actually no longer a concern, but prior to that version, there couldn't be more thanINT_MAXresources, what could cause real issues for long running processes.nicolas-grekas commentedon Aug 1, 2022
Sure, but what's the relationship with repeatedly registering and unregistering stream wrappers?
cmb69 commentedon Aug 1, 2022
stream_wrapper_register()registers a new resource internally:php-src/main/streams/userspace.c
Line 500 in 50bd8ba
cmb69 commentedon Aug 1, 2022
PS: might be worth documenting this.
nicolas-grekas commentedon Aug 1, 2022
Thanks for the hints. This should be fixed for Symfony in symfony/symfony#47143
bug #47143 [HttpClient] Fix memory leak when using StreamWrapper (nic…