Skip to content

eval is leaking memory #8078

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

Open
mvorisek opened this issue Feb 11, 2022 · 5 comments
Open

eval is leaking memory #8078

mvorisek opened this issue Feb 11, 2022 · 5 comments

Comments

@mvorisek
Copy link
Contributor

mvorisek commented Feb 11, 2022

Description

The following code:

<?php

$m = memory_get_usage();
for ($i = 0; $i < 1000; $i++) {
    $fn = eval('return function ($foo) {};');
    //$fn = function ($foo) {};
    var_dump(memory_get_usage() - $m);
}

https://3v4l.org/LF4Ri
https://3v4l.org/1DeC6 - /wo eval, not leaking
https://3v4l.org/NtIrU - eval /wo anonymous function is not leaking

Resulted in this output:

int(640)
int(672)
int(672)
int(672)
...
int(197280)

But I expected this output instead:

int(640)
int(672)
int(672)
int(672)
...
int(672)

PHP Version

8.1.x

Operating System

any

@bwoebi
Copy link
Member

bwoebi commented Feb 11, 2022

This is by design - for now at least. This is not specific to eval, but to any non-top level op_array (i.e. all functions).

Generally, code, once compiled - unless it is top level code - is persisted for the request.

For example, this also leaks:

php -r 'for ($i = 0; $i < 5; ++$i) { var_dump(memory_get_usage()); include "a.php"; }'
int(430072)
int(430888)
int(431256)
int(431624)
int(431992)

with a.php being:

<?php

if (false) {
	class A {
	}
}

But we do currently not have any mechanisms to track unresolved (or Closure) op_arrays (and classes) by their parent op_array to free them on destruction (in this case, triggered by: eval or inclusion returning).

@nikic
Copy link
Member

nikic commented Feb 11, 2022

@bwoebi As of PHP 8.1 we do actually. We track these via dynamic_func_defs and release them once unused. The only reason there is still a small leak here, is that the op_array structure itself is still arena-allocated.

Edit: This doesn't apply to classes -- classes are much harder to fix due to preloading, which makes everything very complicated.

@mvorisek
Copy link
Contributor Author

is the freeing of the arena-allocated op_array structure for the closure easily fixable?

@cmb69
Copy link
Member

cmb69 commented Feb 17, 2022

is the freeing of the arena-allocated op_array structure for the closure easily fixable?

Arena allocations are similar to SQL transactions with checkpoints. You can destroy the whole arena or roll back to a certain checkpoint, but you can't free certain parts of the arena. So I don't think this is (easily) fixable.

@nikic
Copy link
Member

nikic commented Feb 17, 2022

It requires moving away from arena allocation. I believe it's not entirely simple because we have two different ways to share op_arrays, one of them sharing only the resources it points to while having a separately allocated op_array structure, and another where the op_array itself is also shared. This makes it a bit tricky to determine whether or not an op_array needs to be freed when it is released, or only the refcount decremented.

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

4 participants