Re: com php-src: Fix bad run_time_cache with Closure::call() This also fixes a memory "leak" (memory is allocated on unbounded arena without limits)
on each new Closure instantiation. Closures with same scope now all share the same run_time_cache (as long as it is arena allocated):
Zend/tests/closure_060.phpt Zend/zend_closures.c Zend/zend_compile.h
| From: | Nikita Popov | Date: | Sun, 21 Jun 2015 14:55:23 +0000 |
| Subject: | Re: com php-src: Fix bad run_time_cache with Closure::call() This also fixes a memory "leak" (memory is allocated on unbounded arena without limits) on each new Closure instantiation. Closures with same scope now all share the same run_time_cache (as long as it is arena allocated): Zend/tests/closure_060.phpt Zend/zend_closures.c Zend/zend_compile.h |
||
| References: | 1 | Groups: | php.cvs |
| Request: | Send a blank email to php-cvs+get-87420@lists.php.net to get a copy of this message | ||
On Sun, Jun 21, 2015 at 4:39 PM, Bob Weinand <bwoebi@php.net> wrote:
> Commit: 3c288b12b45763b3215ee0463e51f8be9d643425
> Author: Bob Weinand <bobwei9@hotmail.com> Sun, 21 Jun 2015
> 16:39:28 +0200
> Parents: f3df3df8737ace9f4431416fdd0d312cb0ee9cfd
> Branches: master
>
> Link:
>
> http://git.php.net/?p=php-src.git;a=commitdiff;h=3c288b12b45763b3215ee0463e51f8be9d643425
>
> Log:
> Fix bad run_time_cache with Closure::call()
> This also fixes a memory "leak" (memory is allocated on unbounded arena
> without limits) on each new Closure instantiation.
> Closures with same scope now all share the same run_time_cache (as long as
> it is arena allocated)
>
> Changed paths:
> A Zend/tests/closure_060.phpt
> M Zend/zend_closures.c
> M Zend/zend_compile.h
>
>
> Diff:
> diff --git a/Zend/tests/closure_060.phpt b/Zend/tests/closure_060.phpt
> new file mode 100644
> index 0000000..f03160d
> --- /dev/null
> +++ b/Zend/tests/closure_060.phpt
> @@ -0,0 +1,25 @@
> +--TEST--
> +runtime cache must be invalidated for Closure::call()
> +--FILE--
> +<?php
> +
> +class A {
> + private static $priv = 7;
> +
> + static function get() {
> + return function() {
> + var_dump(isset(A::$priv));
> + };
> + }
> +}
> +
> +$closure = A::get();
> +$closure(); // init rt_cache
> +$closure->call(new class(){}, null);
> +$closure();
> +
> +?>
> +--EXPECT--
> +bool(true)
> +bool(false)
> +bool(true)
> diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c
> index b28810a..4e642ff 100644
> --- a/Zend/zend_closures.c
> +++ b/Zend/zend_closures.c
> @@ -127,9 +127,19 @@ ZEND_METHOD(Closure, call)
> my_function.common.scope = Z_OBJCE_P(newthis);
> fci_cache.function_handler = &my_function;
>
> + /* Runtime cache relies on bound scope to be immutable, hence we
> need a separate rt cache in case scope changed */
> + if (ZEND_USER_CODE(my_function.type) && closure->func.common.scope
> != Z_OBJCE_P(newthis)) {
> + my_function.op_array.run_time_cache =
> emalloc(my_function.op_array.cache_size);
> + memset(my_function.op_array.run_time_cache, 0,
> my_function.op_array.cache_size);
> + }
> +
> if (zend_call_function(&fci, &fci_cache) == SUCCESS &&
> Z_TYPE(closure_result) != IS_UNDEF) {
> ZVAL_COPY_VALUE(return_value, &closure_result);
> }
> +
> + if (ZEND_USER_CODE(my_function.type) && closure->func.common.scope
> != Z_OBJCE_P(newthis)) {
> + efree(my_function.op_array.run_time_cache);
> + }
> }
> /* }}} */
>
This way of freeing is unsafe if you have multiple nested ::call()s. E.g.
this crashes:
<?php
class A {}
class B {}
$closure = function() use(&$closure) {
$closure->call(new class{}, A::class);
};
$closure->call(new class{}, B::class);
Nikita