Skip to content

Commit bf96980

Browse files
arnaud-lbarnaud-lb
committed
Merge branch 'PHP-8.3'
* PHP-8.3: [ci skip] NEWS for GH-15330 [ci skip] NEWS for GH-15330 Do not scan generator frames more than once (#15330)
2 parents 5b482b7 + 0e9e591 commit bf96980

10 files changed

+357
-35
lines changed

‎Zend/tests/gh15330-001.phpt

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
GH-15330 001: Do not scan generator frames more than once
3+
--FILE--
4+
<?php
5+
6+
class It implements \IteratorAggregate
7+
{
8+
public function getIterator(): \Generator
9+
{
10+
yield 'foo';
11+
Fiber::suspend();
12+
var_dump("not executed");
13+
}
14+
}
15+
16+
function f() {
17+
var_dump(yield from new It());
18+
}
19+
20+
$iterable = f();
21+
22+
$fiber = new Fiber(function () use ($iterable) {
23+
var_dump($iterable->current());
24+
$iterable->next();
25+
var_dump("not executed");
26+
});
27+
28+
$ref = $fiber;
29+
30+
$fiber->start();
31+
32+
gc_collect_cycles();
33+
34+
?>
35+
==DONE==
36+
--EXPECT--
37+
string(3) "foo"
38+
==DONE==

‎Zend/tests/gh15330-002.phpt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
GH-15330 002: Do not scan generator frames more than once
3+
--FILE--
4+
<?php
5+
6+
function g() {
7+
yield 'foo';
8+
Fiber::suspend();
9+
}
10+
11+
function f() {
12+
var_dump(yield from g());
13+
}
14+
15+
$iterable = f();
16+
17+
$fiber = new Fiber(function () use ($iterable) {
18+
var_dump($iterable->current());
19+
$iterable->next();
20+
var_dump("not executed");
21+
});
22+
23+
$ref = $fiber;
24+
25+
$fiber->start();
26+
27+
gc_collect_cycles();
28+
29+
?>
30+
==DONE==
31+
--EXPECT--
32+
string(3) "foo"
33+
==DONE==

‎Zend/tests/gh15330-003.phpt

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
--TEST--
2+
GH-15330 003: Do not scan generator frames more than once
3+
--FILE--
4+
<?php
5+
6+
class It implements \IteratorAggregate
7+
{
8+
public function getIterator(): \Generator
9+
{
10+
yield 'foo';
11+
Fiber::suspend();
12+
var_dump("not executed");
13+
}
14+
}
15+
16+
class Canary {
17+
public function __construct(public mixed $value) {}
18+
public function __destruct() {
19+
var_dump(__METHOD__);
20+
}
21+
}
22+
23+
function f($canary) {
24+
var_dump(yield from new It());
25+
}
26+
27+
$canary = new Canary(null);
28+
29+
$iterable = f($canary);
30+
31+
$fiber = new Fiber(function () use ($iterable, $canary) {
32+
var_dump($canary, $iterable->current());
33+
$iterable->next();
34+
var_dump("not executed");
35+
});
36+
37+
$canary->value = $fiber;
38+
39+
$fiber->start();
40+
41+
$iterable->current();
42+
43+
$fiber = $iterable = $canary = null;
44+
45+
gc_collect_cycles();
46+
47+
?>
48+
==DONE==
49+
--EXPECTF--
50+
object(Canary)#%d (1) {
51+
["value"]=>
52+
object(Fiber)#%d (0) {
53+
}
54+
}
55+
string(3) "foo"
56+
string(18) "Canary::__destruct"
57+
==DONE==

‎Zend/tests/gh15330-004.phpt

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
--TEST--
2+
GH-15330 004: Do not scan generator frames more than once
3+
--FILE--
4+
<?php
5+
6+
class Canary {
7+
public function __construct(public mixed $value) {}
8+
public function __destruct() {
9+
var_dump(__METHOD__);
10+
}
11+
}
12+
13+
function g() {
14+
yield 'foo';
15+
Fiber::suspend();
16+
}
17+
18+
function f($canary) {
19+
var_dump(yield from g());
20+
}
21+
22+
$canary = new Canary(null);
23+
24+
$iterable = f($canary);
25+
26+
$fiber = new Fiber(function () use ($iterable, $canary) {
27+
var_dump($canary, $iterable->current());
28+
$iterable->next();
29+
var_dump("not executed");
30+
});
31+
32+
$canary->value = $fiber;
33+
34+
$fiber->start();
35+
36+
$iterable->current();
37+
38+
$fiber = $iterable = $canary = null;
39+
40+
gc_collect_cycles();
41+
42+
?>
43+
==DONE==
44+
--EXPECTF--
45+
object(Canary)#%d (1) {
46+
["value"]=>
47+
object(Fiber)#%d (0) {
48+
}
49+
}
50+
string(3) "foo"
51+
string(18) "Canary::__destruct"
52+
==DONE==

‎Zend/tests/gh15330-005.phpt

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
--TEST--
2+
GH-15330 005: Do not scan generator frames more than once
3+
--FILE--
4+
<?php
5+
6+
class Canary {
7+
public function __construct(public mixed $value) {}
8+
public function __destruct() {
9+
var_dump(__METHOD__);
10+
}
11+
}
12+
13+
function g() {
14+
yield 'foo';
15+
Fiber::suspend();
16+
}
17+
18+
function f($canary) {
19+
var_dump(yield from g());
20+
}
21+
22+
$canary = new Canary(null);
23+
24+
$iterable = f($canary);
25+
26+
$fiber = new Fiber(function () use ($iterable, $canary) {
27+
var_dump($canary, $iterable->current());
28+
$f = $iterable->next(...);
29+
$f();
30+
var_dump("not executed");
31+
});
32+
33+
$canary->value = $fiber;
34+
35+
$fiber->start();
36+
37+
$iterable->current();
38+
39+
$fiber = $iterable = $canary = null;
40+
41+
gc_collect_cycles();
42+
43+
?>
44+
==DONE==
45+
--EXPECTF--
46+
object(Canary)#%d (1) {
47+
["value"]=>
48+
object(Fiber)#%d (0) {
49+
}
50+
}
51+
string(3) "foo"
52+
string(18) "Canary::__destruct"
53+
==DONE==

‎Zend/tests/gh15330-006.phpt

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
--TEST--
2+
GH-15330 006: Do not scan generator frames more than once
3+
--FILE--
4+
<?php
5+
6+
class Canary {
7+
public function __construct(public mixed $value) {}
8+
public function __destruct() {
9+
var_dump(__METHOD__);
10+
}
11+
}
12+
13+
function h() {
14+
yield 'foo';
15+
Fiber::suspend();
16+
}
17+
18+
function g() {
19+
yield from h();
20+
}
21+
22+
function f($canary) {
23+
var_dump(yield from g());
24+
}
25+
26+
$canary = new Canary(null);
27+
28+
$iterable = f($canary);
29+
30+
$fiber = new Fiber(function () use ($iterable, $canary) {
31+
var_dump($canary, $iterable->current());
32+
$iterable->next();
33+
var_dump("not executed");
34+
});
35+
36+
$canary->value = $fiber;
37+
38+
$fiber->start();
39+
40+
$iterable->current();
41+
42+
$fiber = $iterable = $canary = null;
43+
44+
gc_collect_cycles();
45+
46+
?>
47+
==DONE==
48+
--EXPECTF--
49+
object(Canary)#%d (1) {
50+
["value"]=>
51+
object(Fiber)#%d (0) {
52+
}
53+
}
54+
string(3) "foo"
55+
string(18) "Canary::__destruct"
56+
==DONE==

‎Zend/zend_execute.c

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4753,7 +4753,20 @@ ZEND_API ZEND_ATTRIBUTE_DEPRECATED HashTable *zend_unfinished_execution_gc(zend_
47534753

47544754
ZEND_API HashTable *zend_unfinished_execution_gc_ex(zend_execute_data *execute_data, zend_execute_data *call, zend_get_gc_buffer *gc_buffer, bool suspended_by_yield)
47554755
{
4756-
if (!EX(func) || !ZEND_USER_CODE(EX(func)->common.type)) {
4756+
if (!EX(func)) {
4757+
return NULL;
4758+
}
4759+
4760+
if (EX_CALL_INFO() & ZEND_CALL_RELEASE_THIS) {
4761+
zend_get_gc_buffer_add_obj(gc_buffer, Z_OBJ(execute_data->This));
4762+
}
4763+
4764+
if (EX_CALL_INFO() & ZEND_CALL_CLOSURE) {
4765+
zend_get_gc_buffer_add_obj(gc_buffer, ZEND_CLOSURE_OBJECT(EX(func)));
4766+
}
4767+
4768+
if (!ZEND_USER_CODE(EX(func)->common.type)) {
4769+
ZEND_ASSERT(!(EX_CALL_INFO() & (ZEND_CALL_HAS_SYMBOL_TABLE|ZEND_CALL_FREE_EXTRA_ARGS|ZEND_CALL_HAS_EXTRA_NAMED_PARAMS)));
47574770
return NULL;
47584771
}
47594772

@@ -4774,12 +4787,6 @@ ZEND_API HashTable *zend_unfinished_execution_gc_ex(zend_execute_data *execute_d
47744787
}
47754788
}
47764789

4777-
if (EX_CALL_INFO() & ZEND_CALL_RELEASE_THIS) {
4778-
zend_get_gc_buffer_add_obj(gc_buffer, Z_OBJ(execute_data->This));
4779-
}
4780-
if (EX_CALL_INFO() & ZEND_CALL_CLOSURE) {
4781-
zend_get_gc_buffer_add_obj(gc_buffer, ZEND_CLOSURE_OBJECT(EX(func)));
4782-
}
47834790
if (EX_CALL_INFO() & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) {
47844791
zval extra_named_params;
47854792
ZVAL_ARR(&extra_named_params, EX(extra_named_params));

‎Zend/zend_fibers.c

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include "zend.h"
2121
#include "zend_API.h"
22+
#include "zend_gc.h"
2223
#include "zend_ini.h"
2324
#include "zend_variables.h"
2425
#include "zend_vm.h"
@@ -28,6 +29,7 @@
2829
#include "zend_mmap.h"
2930
#include "zend_compile.h"
3031
#include "zend_closures.h"
32+
#include "zend_generators.h"
3133

3234
#include "zend_fibers.h"
3335
#include "zend_fibers_arginfo.h"
@@ -824,7 +826,25 @@ static HashTable *zend_fiber_object_gc(zend_object *object, zval **table, int *n
824826
HashTable *lastSymTable = NULL;
825827
zend_execute_data *ex = fiber->execute_data;
826828
for (; ex; ex = ex->prev_execute_data) {
827-
HashTable *symTable = zend_unfinished_execution_gc_ex(ex, ex->func && ZEND_USER_CODE(ex->func->type) ? ex->call : NULL, buf, false);
829+
HashTable *symTable;
830+
if (ZEND_CALL_INFO(ex) & ZEND_CALL_GENERATOR) {
831+
/* The generator object is stored in ex->return_value */
832+
zend_generator *generator = (zend_generator*)ex->return_value;
833+
/* There are two cases to consider:
834+
* - If the generator is currently running, the Generator's GC
835+
* handler will ignore it because it is not collectable. However,
836+
* in this context the generator is suspended in Fiber::suspend()
837+
* and may be collectable, so we can inspect it.
838+
* - If the generator is not running, the Generator's GC handler
839+
* will inspect it. In this case we have to skip the frame.
840+
*/
841+
if (!(generator->flags & ZEND_GENERATOR_CURRENTLY_RUNNING)) {
842+
continue;
843+
}
844+
symTable = zend_generator_frame_gc(buf, generator);
845+
} else {
846+
symTable = zend_unfinished_execution_gc_ex(ex, ex->func && ZEND_USER_CODE(ex->func->type) ? ex->call : NULL, buf, false);
847+
}
828848
if (symTable) {
829849
if (lastSymTable) {
830850
zval *val;

0 commit comments

Comments
 (0)