Skip to content

Commit 6b0f14f

Browse files
committed
Fixed bug #75474
For fake closures, we need to share static variables with the original function, not work on a separate copy. Calling a function through Closure::fromCallable() should have the same behavior as calling it directly.
1 parent 5d160e3 commit 6b0f14f

File tree

3 files changed

+97
-7
lines changed

3 files changed

+97
-7
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ PHP NEWS
55
- Core:
66
. Fixed inclusion order for phpize builds on Windows. (cmb)
77
. Added missing hashtable insertion APIs for arr/obj/ref. (Sara)
8+
. Fixed bug #75474 (function scope static variables are not bound to a unique
9+
function). (Nikita)
810

911
- FTP:
1012
. Convert resource<ftp> to object \FTPConnection. (Sara)

Zend/tests/bug75474.phpt

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
--TEST--
2+
Bug #75474: function scope static variables are not bound to a unique function
3+
--FILE--
4+
<?php
5+
6+
function bar($k, $v) {
7+
static $foo = [];
8+
$foo[$k] = $v;
9+
return $foo;
10+
}
11+
12+
var_dump(bar(0, 0));
13+
var_dump(Closure::fromCallable("bar")(1, 1));
14+
var_dump(bar(2, 2));
15+
var_dump(Closure::fromCallable("bar")(3, 3));
16+
$RF = new ReflectionFunction("bar");
17+
var_dump($RF->getClosure()(4, 4));
18+
var_dump(bar(5, 5));
19+
20+
?>
21+
--EXPECT--
22+
array(1) {
23+
[0]=>
24+
int(0)
25+
}
26+
array(2) {
27+
[0]=>
28+
int(0)
29+
[1]=>
30+
int(1)
31+
}
32+
array(3) {
33+
[0]=>
34+
int(0)
35+
[1]=>
36+
int(1)
37+
[2]=>
38+
int(2)
39+
}
40+
array(4) {
41+
[0]=>
42+
int(0)
43+
[1]=>
44+
int(1)
45+
[2]=>
46+
int(2)
47+
[3]=>
48+
int(3)
49+
}
50+
array(5) {
51+
[0]=>
52+
int(0)
53+
[1]=>
54+
int(1)
55+
[2]=>
56+
int(2)
57+
[3]=>
58+
int(3)
59+
[4]=>
60+
int(4)
61+
}
62+
array(6) {
63+
[0]=>
64+
int(0)
65+
[1]=>
66+
int(1)
67+
[2]=>
68+
int(2)
69+
[3]=>
70+
int(3)
71+
[4]=>
72+
int(4)
73+
[5]=>
74+
int(5)
75+
}

Zend/zend_closures.c

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,11 @@ static void zend_closure_free_storage(zend_object *object) /* {{{ */
486486
zend_object_std_dtor(&closure->std);
487487

488488
if (closure->func.type == ZEND_USER_FUNCTION) {
489+
/* We shared static_variables with the original function.
490+
* Unshare now so we don't try to destroy them. */
491+
if (closure->func.op_array.fn_flags & ZEND_ACC_FAKE_CLOSURE) {
492+
ZEND_MAP_PTR_INIT(closure->func.op_array.static_variables_ptr, NULL);
493+
}
489494
destroy_op_array(&closure->func.op_array);
490495
}
491496

@@ -660,7 +665,7 @@ static ZEND_NAMED_FUNCTION(zend_closure_internal_handler) /* {{{ */
660665
}
661666
/* }}} */
662667

663-
ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_entry *scope, zend_class_entry *called_scope, zval *this_ptr) /* {{{ */
668+
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) /* {{{ */
664669
{
665670
zend_closure *closure;
666671

@@ -679,12 +684,15 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent
679684
closure->func.common.fn_flags |= ZEND_ACC_CLOSURE;
680685
closure->func.common.fn_flags &= ~ZEND_ACC_IMMUTABLE;
681686

682-
if (closure->func.op_array.static_variables) {
683-
closure->func.op_array.static_variables =
684-
zend_array_dup(closure->func.op_array.static_variables);
687+
/* For fake closures, we want to reuse the static variables of the original function. */
688+
if (!is_fake) {
689+
if (closure->func.op_array.static_variables) {
690+
closure->func.op_array.static_variables =
691+
zend_array_dup(closure->func.op_array.static_variables);
692+
}
693+
ZEND_MAP_PTR_INIT(closure->func.op_array.static_variables_ptr,
694+
&closure->func.op_array.static_variables);
685695
}
686-
ZEND_MAP_PTR_INIT(closure->func.op_array.static_variables_ptr,
687-
&closure->func.op_array.static_variables);
688696

689697
/* Runtime cache is scope-dependent, so we cannot reuse it if the scope changed */
690698
if (!ZEND_MAP_PTR_GET(closure->func.op_array.run_time_cache)
@@ -754,11 +762,16 @@ ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_ent
754762
}
755763
/* }}} */
756764

765+
ZEND_API void zend_create_closure(zval *res, zend_function *func, zend_class_entry *scope, zend_class_entry *called_scope, zval *this_ptr)
766+
{
767+
zend_create_closure_ex(res, func, scope, called_scope, this_ptr, /* is_fake */ false);
768+
}
769+
757770
ZEND_API void zend_create_fake_closure(zval *res, zend_function *func, zend_class_entry *scope, zend_class_entry *called_scope, zval *this_ptr) /* {{{ */
758771
{
759772
zend_closure *closure;
760773

761-
zend_create_closure(res, func, scope, called_scope, this_ptr);
774+
zend_create_closure_ex(res, func, scope, called_scope, this_ptr, /* is_fake */ true);
762775

763776
closure = (zend_closure *)Z_OBJ_P(res);
764777
closure->func.common.fn_flags |= ZEND_ACC_FAKE_CLOSURE;

0 commit comments

Comments
 (0)