Skip to content

Commit f551a71

Browse files
arnaud-lbarnaud-lb
committed
Merge branch 'PHP-8.2' into PHP-8.3
* PHP-8.2: [ci skip] NEWS for GH-15330 Do not scan generator frames more than once (#15330)
2 parents 0f487e7 + 4db7814 commit f551a71

10 files changed

+357
-35
lines changed

‎Zend/tests/gh15330-001.phpt

+38
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

+33
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

+57
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

+52
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

+53
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

+56
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

+14-7
Original file line numberDiff line numberDiff line change
@@ -4564,7 +4564,20 @@ ZEND_API ZEND_ATTRIBUTE_DEPRECATED HashTable *zend_unfinished_execution_gc(zend_
45644564

45654565
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)
45664566
{
4567-
if (!EX(func) || !ZEND_USER_CODE(EX(func)->common.type)) {
4567+
if (!EX(func)) {
4568+
return NULL;
4569+
}
4570+
4571+
if (EX_CALL_INFO() & ZEND_CALL_RELEASE_THIS) {
4572+
zend_get_gc_buffer_add_obj(gc_buffer, Z_OBJ(execute_data->This));
4573+
}
4574+
4575+
if (EX_CALL_INFO() & ZEND_CALL_CLOSURE) {
4576+
zend_get_gc_buffer_add_obj(gc_buffer, ZEND_CLOSURE_OBJECT(EX(func)));
4577+
}
4578+
4579+
if (!ZEND_USER_CODE(EX(func)->common.type)) {
4580+
ZEND_ASSERT(!(EX_CALL_INFO() & (ZEND_CALL_HAS_SYMBOL_TABLE|ZEND_CALL_FREE_EXTRA_ARGS|ZEND_CALL_HAS_EXTRA_NAMED_PARAMS)));
45684581
return NULL;
45694582
}
45704583

@@ -4585,12 +4598,6 @@ ZEND_API HashTable *zend_unfinished_execution_gc_ex(zend_execute_data *execute_d
45854598
}
45864599
}
45874600

4588-
if (EX_CALL_INFO() & ZEND_CALL_RELEASE_THIS) {
4589-
zend_get_gc_buffer_add_obj(gc_buffer, Z_OBJ(execute_data->This));
4590-
}
4591-
if (EX_CALL_INFO() & ZEND_CALL_CLOSURE) {
4592-
zend_get_gc_buffer_add_obj(gc_buffer, ZEND_CLOSURE_OBJECT(EX(func)));
4593-
}
45944601
if (EX_CALL_INFO() & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) {
45954602
zval extra_named_params;
45964603
ZVAL_ARR(&extra_named_params, EX(extra_named_params));

‎Zend/zend_fibers.c

+21-1
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_vm.h"
2425
#include "zend_exceptions.h"
@@ -27,6 +28,7 @@
2728
#include "zend_mmap.h"
2829
#include "zend_compile.h"
2930
#include "zend_closures.h"
31+
#include "zend_generators.h"
3032

3133
#include "zend_fibers.h"
3234
#include "zend_fibers_arginfo.h"
@@ -763,7 +765,25 @@ static HashTable *zend_fiber_object_gc(zend_object *object, zval **table, int *n
763765
HashTable *lastSymTable = NULL;
764766
zend_execute_data *ex = fiber->execute_data;
765767
for (; ex; ex = ex->prev_execute_data) {
766-
HashTable *symTable = zend_unfinished_execution_gc_ex(ex, ex->func && ZEND_USER_CODE(ex->func->type) ? ex->call : NULL, buf, false);
768+
HashTable *symTable;
769+
if (ZEND_CALL_INFO(ex) & ZEND_CALL_GENERATOR) {
770+
/* The generator object is stored in ex->return_value */
771+
zend_generator *generator = (zend_generator*)ex->return_value;
772+
/* There are two cases to consider:
773+
* - If the generator is currently running, the Generator's GC
774+
* handler will ignore it because it is not collectable. However,
775+
* in this context the generator is suspended in Fiber::suspend()
776+
* and may be collectable, so we can inspect it.
777+
* - If the generator is not running, the Generator's GC handler
778+
* will inspect it. In this case we have to skip the frame.
779+
*/
780+
if (!(generator->flags & ZEND_GENERATOR_CURRENTLY_RUNNING)) {
781+
continue;
782+
}
783+
symTable = zend_generator_frame_gc(buf, generator);
784+
} else {
785+
symTable = zend_unfinished_execution_gc_ex(ex, ex->func && ZEND_USER_CODE(ex->func->type) ? ex->call : NULL, buf, false);
786+
}
767787
if (symTable) {
768788
if (lastSymTable) {
769789
zval *val;

0 commit comments

Comments
 (0)