Skip to content
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

Potential memory leak in addslashes/stripslashes #12508

Closed
oleg-andreyev opened this issue Oct 24, 2023 · 5 comments
Closed

Potential memory leak in addslashes/stripslashes #12508

oleg-andreyev opened this issue Oct 24, 2023 · 5 comments

Comments

@oleg-andreyev
Copy link

oleg-andreyev commented Oct 24, 2023

Description

The following code:

No leak

<?php

function encode() {
    return addslashes('stdClass');
}

function decode(string $str) {
    return stripslashes($str);
}

for ($ctr = 0; $ctr < 1_000_000; $ctr++) {
    $php = encode();
    decode($php);
}

Resulted in this output:
https://gist.github.com/oleg-andreyev/0cb275e5945061f00560f0fe07a7e068

Potential memory leak

function encode() {
    return addslashes('\stdClass');
}

function decode(string $str) {
    return stripslashes($str);
}

for ($ctr = 0; $ctr < 1_000_000; $ctr++) {
    $php = encode();
    decode($php);
}

Resulted in this output:
https://gist.github.com/oleg-andreyev/bd85f9441e1b12e603a8592553d6ec69

image

As you can see (first example):
encode allocated 0 and free 0
decode allocated 38.1Mb and free 38.1Mb

second example:
encode allocated 53.4Mb and free 0
decode allocated 38.1Mb and free 38.1Mb

PHP Version

8.1.22

Operating System

Alpine Linux v3.18


UPD
same but with extra flag SPX_BUILTINS=1
image
image

it looks like both addslashes/stripslashes are not freeing memory.

@oleg-andreyev oleg-andreyev changed the title Potential memory leak in addslashes Potential memory leak in addslashes/stripslashes Oct 24, 2023
@nielsdos
Copy link
Member

For your first code snippet there are no allocations because addslashes and stripslashes will only allocate if the string needs tl change. If it realizes the string stays the same then the refcount of the string is just increased and nothing is allocated.

For the second snippet I'm not sure why SPX thinks there is a leak. I don't see one in ASAN or in the ZMM leak detector. Using memory_get_usage also shows no increase. I think therefore the (third party) SPX extension is misreporting or doing something wrong. Possibly getting confused with interned strings?

@oleg-andreyev
Copy link
Author

Real case is no longer relevant as found that Symfony is collecting some "trace" information (even when --no-debug passed)

@NoiseByNorthwest
Copy link

SPX does not directly highlight memory leaks, it only collects memory related metrics at function start/end which can help investigating various memory issues like leaks.

Regarding your second example:

encode allocated 53.4Mb and free 0

As expected, encode allocates on its own 1M strings and return them to the parent scope.

decode allocated 38.1Mb and free 38.1Mb

This is also expected, decode allocates what is needed to create 1M strings that reach refcount=0 in this scope so they are freed right before leaving the function.

And finally test.php is correctly reported as the function which frees the 53.4MB held by $php variable.

@oleg-andreyev
Copy link
Author

@NoiseByNorthwest latest screenshot was made with following code:

<?php

for ($ctr = 0; $ctr < 1_000_000; $ctr++) {
    stripslashes(addslashes('\stdClass'));
}

no $php variable anymore.

SPX_ENABLED=1 SPX_FP_LIVE=1 SPX_BUILTINS=1 SPX_METRICS=wt,zm,zmab,zmfb php test.php
image

shouldn't zmfb match zmab?

@NoiseByNorthwest
Copy link

NoiseByNorthwest commented Oct 26, 2023

Even without $php there is still a zval returned by both *slashes functions wich eventually reaches its refcount=0 in test.php. So test.php has, as expected, freed 91.6MB on its own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants