Skip to content

Allow Closures to reuse their run_time_cache when opcache is enabled #18556

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions Zend/tests/closures/closure_069.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Closure: Binding + RT cache edge cases
--FILE--
<?php

// cache_size may be zero
$f = function () {};
$f();
$g = $f->bindTo(new class {});

?>
==DONE==
--EXPECT--
==DONE==
44 changes: 33 additions & 11 deletions Zend/zend_closures.c
Original file line number Diff line number Diff line change
Expand Up @@ -733,10 +733,29 @@ static ZEND_NAMED_FUNCTION(zend_closure_internal_handler) /* {{{ */
}
/* }}} */

static zend_class_entry *zend_closure_rt_cache_scope(zend_function *func, void **run_time_cache, bool is_fake, zend_class_entry *default_scope)
{
ZEND_ASSERT(!(func->op_array.fn_flags & ZEND_ACC_HEAP_RT_CACHE));

if (is_fake) {
return func->op_array.scope;
}

/* Zero RT cache is valid for any scope */
if (func->op_array.cache_size == 0) {
return default_scope;
}

ZEND_ASSERT(func->common.fn_flags & ZEND_ACC_CLOSURE);
ZEND_ASSERT(!(func->common.fn_flags & ZEND_ACC_FAKE_CLOSURE));

return CACHED_PTR_EX(run_time_cache - 1);
}

static void zend_create_closure_ex(zval *res, zend_function *func, zend_class_entry *scope, zend_class_entry *called_scope, zval *this_ptr, bool is_fake) /* {{{ */
{
zend_closure *closure;
void *ptr;
void **ptr;

object_init_ex(res, zend_ce_closure);

Expand Down Expand Up @@ -777,19 +796,22 @@ static void zend_create_closure_ex(zval *res, zend_function *func, zend_class_en
/* Runtime cache is scope-dependent, so we cannot reuse it if the scope changed */
ptr = ZEND_MAP_PTR_GET(func->op_array.run_time_cache);
if (!ptr
|| func->common.scope != scope
|| (func->common.fn_flags & ZEND_ACC_HEAP_RT_CACHE)
|| zend_closure_rt_cache_scope(func, ptr, is_fake, scope) != scope
) {
if (!ptr
&& (func->common.fn_flags & ZEND_ACC_CLOSURE)
&& (func->common.scope == scope ||
!(func->common.fn_flags & ZEND_ACC_IMMUTABLE))) {
if (func->op_array.cache_size == 0) {
ptr = zend_arena_alloc(&CG(arena), 0);
ZEND_MAP_PTR_SET(func->op_array.run_time_cache, ptr);
closure->func.op_array.fn_flags &= ~ZEND_ACC_HEAP_RT_CACHE;
} else if (!ptr
&& func->common.fn_flags & ZEND_ACC_CLOSURE) {
ZEND_ASSERT(!(func->common.fn_flags & ZEND_ACC_FAKE_CLOSURE));
/* If a real closure is used for the first time, we create a shared runtime cache
* and remember which scope it is for. */
if (func->common.scope != scope) {
func->common.scope = scope;
}
ptr = zend_arena_alloc(&CG(arena), func->op_array.cache_size);
* and remember which scope it is for. The scope is stored in
* an extra slot before the run_time_cache. */
size_t extra_slot_size = sizeof(void*);
ptr = zend_arena_alloc(&CG(arena), func->op_array.cache_size + extra_slot_size) + extra_slot_size;
CACHE_PTR_EX(ptr - 1, scope);
ZEND_MAP_PTR_SET(func->op_array.run_time_cache, ptr);
closure->func.op_array.fn_flags &= ~ZEND_ACC_HEAP_RT_CACHE;
} else {
Expand Down
126 changes: 126 additions & 0 deletions ext/zend_test/tests/observer_closure_04.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
--TEST--
Observer: Closures keep their runtime cache when not rebinding
--EXTENSIONS--
zend_test
--INI--
zend_test.observer.enabled=1
zend_test.observer.show_output=1
zend_test.observer.observe_all=1
--FILE--
<?php

class Foo {
private static $value = "x";
function static() {
(static function () {
echo Foo::$value, PHP_EOL;
})();
}
function non_static() {
(function () {
echo Foo::$value, PHP_EOL;
})();
}
function rebind() {
((function () {
try {
echo Foo::$value, PHP_EOL;
} catch (Error $e) {
echo $e::class, ": ", $e->getMessage(), PHP_EOL;
}
})->bindTo(null, null))();
}
}

$obj = new Foo();
$obj->static();
$obj->static();
$obj->non_static();
$obj->non_static();
$obj->rebind();
$obj->rebind();

$closure = function () {
echo Foo::$value, PHP_EOL;
};

($closure->bindTo(null, Foo::class))();
($closure->bindTo(null, Foo::class))();

$boundClosure = $closure->bindTo(null, Foo::class);
$boundClosure();
$boundClosure();

?>
--EXPECTF--
<!-- init '%s' -->
<file '%s'>
<!-- init Foo::static() -->
<Foo::static>
<!-- init Foo::{closure:Foo::static():6}() -->
<Foo::{closure:Foo::static():6}>
x
</Foo::{closure:Foo::static():6}>
</Foo::static>
<Foo::static>
<Foo::{closure:Foo::static():6}>
x
</Foo::{closure:Foo::static():6}>
</Foo::static>
<!-- init Foo::non_static() -->
<Foo::non_static>
<!-- init Foo::{closure:Foo::non_static():11}() -->
<Foo::{closure:Foo::non_static():11}>
x
</Foo::{closure:Foo::non_static():11}>
</Foo::non_static>
<Foo::non_static>
<Foo::{closure:Foo::non_static():11}>
x
</Foo::{closure:Foo::non_static():11}>
</Foo::non_static>
<!-- init Foo::rebind() -->
<Foo::rebind>
<!-- init Closure::bindTo() -->
<Closure::bindTo>
</Closure::bindTo>
<!-- init {closure:Foo::rebind():16}() -->
<{closure:Foo::rebind():16}>
Error: <!-- init Error::getMessage() -->
<Error::getMessage>
</Error::getMessage>
Cannot access private property Foo::$value
</{closure:Foo::rebind():16}>
</Foo::rebind>
<Foo::rebind>
<Closure::bindTo>
</Closure::bindTo>
<!-- init {closure:Foo::rebind():16}() -->
<{closure:Foo::rebind():16}>
Error: <Error::getMessage>
</Error::getMessage>
Cannot access private property Foo::$value
</{closure:Foo::rebind():16}>
</Foo::rebind>
<Closure::bindTo>
</Closure::bindTo>
<!-- init Foo::{closure:%s:%d}() -->
<Foo::{closure:%s:%d}>
x
</Foo::{closure:%s:%d}>
<Closure::bindTo>
</Closure::bindTo>
<!-- init Foo::{closure:%s:%d}() -->
<Foo::{closure:%s:%d}>
x
</Foo::{closure:%s:%d}>
<Closure::bindTo>
</Closure::bindTo>
<!-- init Foo::{closure:%s:%d}() -->
<Foo::{closure:%s:%d}>
x
</Foo::{closure:%s:%d}>
<Foo::{closure:%s:%d}>
x
</Foo::{closure:%s:%d}>
</file '%s'>
Loading