com php-src: Fix bad run_time_cache with Closure::c all() This also fixes a memory "leak" ( memory is allocated on unbounded arena without li mits) on
each new Closure instantiation. Closur es with same scope now all share the same run _time_cache (as long as it is arena allocat ed):
Zend/tests/closure_060.phpt Zend/zend_ closures.c Zend/zend_compile.h

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); + } } /* }}} */ @@ -138,7 +148,7 @@ ZEND_METHOD(Closure, call) ZEND_METHOD(Closure, bind) { zval *newthis, *zclosure, *scope_arg = NULL; - zend_closure *closure; + zend_closure *closure, *new_closure; zend_class_entry *ce, *called_scope; if (zend_parse_method_parameters(ZEND_NUM_ARGS(), getThis(), "Oo!|z", &zclosure, zend_ce_closure, &newthis, &scope_arg) == FAILURE) { @@ -183,6 +193,15 @@ ZEND_METHOD(Closure, bind) } zend_create_closure(return_value, &closure->func, ce, called_scope, newthis); + new_closure = (zend_closure *) Z_OBJ_P(return_value); + + /* Runtime cache relies on bound scope to be immutable, hence we need a separate rt cache in case scope changed */ + if (ZEND_USER_CODE(closure->func.type) && (closure->func.common.scope != new_closure->func.common.scope || (closure->func.op_array.fn_flags & ZEND_ACC_NO_RT_ARENA))) { + new_closure->func.op_array.run_time_cache = emalloc(new_closure->func.op_array.cache_size); + memset(new_closure->func.op_array.run_time_cache, 0, new_closure->func.op_array.cache_size); + + new_closure->func.op_array.fn_flags |= ZEND_ACC_NO_RT_ARENA; + } } /* }}} */ @@ -297,6 +316,10 @@ static void zend_closure_free_storage(zend_object *object) /* {{{ */ zend_object_std_dtor(&closure->std); if (closure->func.type == ZEND_USER_FUNCTION) { + if (closure->func.op_array.fn_flags & ZEND_ACC_NO_RT_ARENA) { + efree(closure->func.op_array.run_time_cache); + closure->func.op_array.run_time_cache = NULL; + } destroy_op_array(&closure->func.op_array); } @@ -510,7 +533,10 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent zend_hash_init(closure->func.op_array.static_variables, zend_hash_num_elements(static_variables), NULL, ZVAL_PTR_DTOR, 0); zend_hash_apply_with_arguments(static_variables, zval_copy_static_var, 1, closure->func.op_array.static_variables); } - closure->func.op_array.run_time_cache = NULL; + if (UNEXPECTED(!closure->func.op_array.run_time_cache)) { + closure->func.op_array.run_time_cache = func->op_array.run_time_cache = zend_arena_alloc(&CG(arena), func->op_array.cache_size); + memset(func->op_array.run_time_cache, 0, func->op_array.cache_size); + } if (closure->func.op_array.refcount) { (*closure->func.op_array.refcount)++; } diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h index 106d0fc..f1f7195 100644 --- a/Zend/zend_compile.h +++ b/Zend/zend_compile.h @@ -240,6 +240,8 @@ typedef struct _zend_try_catch_element { #define ZEND_ACC_CLOSURE 0x100000 #define ZEND_ACC_GENERATOR 0x800000 +#define ZEND_ACC_NO_RT_ARENA 0x10000 + /* call through user function trampoline. e.g. __call, __callstatic */ #define ZEND_ACC_CALL_VIA_TRAMPOLINE 0x200000