diff --git a/Zend/tests/gh15330-001.phpt b/Zend/tests/gh15330-001.phpt new file mode 100644 index 00000000000..dafc31d8b8e --- /dev/null +++ b/Zend/tests/gh15330-001.phpt @@ -0,0 +1,38 @@ +--TEST-- +GH-15330 001: Do not scan generator frames more than once +--FILE-- +current()); + $iterable->next(); + var_dump("not executed"); +}); + +$ref = $fiber; + +$fiber->start(); + +gc_collect_cycles(); + +?> +==DONE== +--EXPECT-- +string(3) "foo" +==DONE== diff --git a/Zend/tests/gh15330-002.phpt b/Zend/tests/gh15330-002.phpt new file mode 100644 index 00000000000..cb2fdc560c5 --- /dev/null +++ b/Zend/tests/gh15330-002.phpt @@ -0,0 +1,33 @@ +--TEST-- +GH-15330 002: Do not scan generator frames more than once +--FILE-- +current()); + $iterable->next(); + var_dump("not executed"); +}); + +$ref = $fiber; + +$fiber->start(); + +gc_collect_cycles(); + +?> +==DONE== +--EXPECT-- +string(3) "foo" +==DONE== diff --git a/Zend/tests/gh15330-003.phpt b/Zend/tests/gh15330-003.phpt new file mode 100644 index 00000000000..f4208c54455 --- /dev/null +++ b/Zend/tests/gh15330-003.phpt @@ -0,0 +1,57 @@ +--TEST-- +GH-15330 003: Do not scan generator frames more than once +--FILE-- +current()); + $iterable->next(); + var_dump("not executed"); +}); + +$canary->value = $fiber; + +$fiber->start(); + +$iterable->current(); + +$fiber = $iterable = $canary = null; + +gc_collect_cycles(); + +?> +==DONE== +--EXPECTF-- +object(Canary)#%d (1) { + ["value"]=> + object(Fiber)#%d (0) { + } +} +string(3) "foo" +string(18) "Canary::__destruct" +==DONE== diff --git a/Zend/tests/gh15330-004.phpt b/Zend/tests/gh15330-004.phpt new file mode 100644 index 00000000000..9e971da7f98 --- /dev/null +++ b/Zend/tests/gh15330-004.phpt @@ -0,0 +1,52 @@ +--TEST-- +GH-15330 004: Do not scan generator frames more than once +--FILE-- +current()); + $iterable->next(); + var_dump("not executed"); +}); + +$canary->value = $fiber; + +$fiber->start(); + +$iterable->current(); + +$fiber = $iterable = $canary = null; + +gc_collect_cycles(); + +?> +==DONE== +--EXPECTF-- +object(Canary)#%d (1) { + ["value"]=> + object(Fiber)#%d (0) { + } +} +string(3) "foo" +string(18) "Canary::__destruct" +==DONE== diff --git a/Zend/tests/gh15330-005.phpt b/Zend/tests/gh15330-005.phpt new file mode 100644 index 00000000000..774b141baf9 --- /dev/null +++ b/Zend/tests/gh15330-005.phpt @@ -0,0 +1,53 @@ +--TEST-- +GH-15330 005: Do not scan generator frames more than once +--FILE-- +current()); + $f = $iterable->next(...); + $f(); + var_dump("not executed"); +}); + +$canary->value = $fiber; + +$fiber->start(); + +$iterable->current(); + +$fiber = $iterable = $canary = null; + +gc_collect_cycles(); + +?> +==DONE== +--EXPECTF-- +object(Canary)#%d (1) { + ["value"]=> + object(Fiber)#%d (0) { + } +} +string(3) "foo" +string(18) "Canary::__destruct" +==DONE== diff --git a/Zend/tests/gh15330-006.phpt b/Zend/tests/gh15330-006.phpt new file mode 100644 index 00000000000..fb9195663de --- /dev/null +++ b/Zend/tests/gh15330-006.phpt @@ -0,0 +1,56 @@ +--TEST-- +GH-15330 006: Do not scan generator frames more than once +--FILE-- +current()); + $iterable->next(); + var_dump("not executed"); +}); + +$canary->value = $fiber; + +$fiber->start(); + +$iterable->current(); + +$fiber = $iterable = $canary = null; + +gc_collect_cycles(); + +?> +==DONE== +--EXPECTF-- +object(Canary)#%d (1) { + ["value"]=> + object(Fiber)#%d (0) { + } +} +string(3) "foo" +string(18) "Canary::__destruct" +==DONE== diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 73f7995779d..bb3991f695f 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -4564,7 +4564,20 @@ ZEND_API ZEND_ATTRIBUTE_DEPRECATED HashTable *zend_unfinished_execution_gc(zend_ 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) { - if (!EX(func) || !ZEND_USER_CODE(EX(func)->common.type)) { + if (!EX(func)) { + return NULL; + } + + if (EX_CALL_INFO() & ZEND_CALL_RELEASE_THIS) { + zend_get_gc_buffer_add_obj(gc_buffer, Z_OBJ(execute_data->This)); + } + + if (EX_CALL_INFO() & ZEND_CALL_CLOSURE) { + zend_get_gc_buffer_add_obj(gc_buffer, ZEND_CLOSURE_OBJECT(EX(func))); + } + + if (!ZEND_USER_CODE(EX(func)->common.type)) { + ZEND_ASSERT(!(EX_CALL_INFO() & (ZEND_CALL_HAS_SYMBOL_TABLE|ZEND_CALL_FREE_EXTRA_ARGS|ZEND_CALL_HAS_EXTRA_NAMED_PARAMS))); return NULL; } @@ -4585,12 +4598,6 @@ ZEND_API HashTable *zend_unfinished_execution_gc_ex(zend_execute_data *execute_d } } - if (EX_CALL_INFO() & ZEND_CALL_RELEASE_THIS) { - zend_get_gc_buffer_add_obj(gc_buffer, Z_OBJ(execute_data->This)); - } - if (EX_CALL_INFO() & ZEND_CALL_CLOSURE) { - zend_get_gc_buffer_add_obj(gc_buffer, ZEND_CLOSURE_OBJECT(EX(func))); - } if (EX_CALL_INFO() & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) { zval extra_named_params; ZVAL_ARR(&extra_named_params, EX(extra_named_params)); diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index d6a4e5f3693..6b6c1eaae1a 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -19,6 +19,7 @@ #include "zend.h" #include "zend_API.h" +#include "zend_gc.h" #include "zend_ini.h" #include "zend_vm.h" #include "zend_exceptions.h" @@ -27,6 +28,7 @@ #include "zend_mmap.h" #include "zend_compile.h" #include "zend_closures.h" +#include "zend_generators.h" #include "zend_fibers.h" #include "zend_fibers_arginfo.h" @@ -763,7 +765,25 @@ static HashTable *zend_fiber_object_gc(zend_object *object, zval **table, int *n HashTable *lastSymTable = NULL; zend_execute_data *ex = fiber->execute_data; for (; ex; ex = ex->prev_execute_data) { - HashTable *symTable = zend_unfinished_execution_gc_ex(ex, ex->func && ZEND_USER_CODE(ex->func->type) ? ex->call : NULL, buf, false); + HashTable *symTable; + if (ZEND_CALL_INFO(ex) & ZEND_CALL_GENERATOR) { + /* The generator object is stored in ex->return_value */ + zend_generator *generator = (zend_generator*)ex->return_value; + /* There are two cases to consider: + * - If the generator is currently running, the Generator's GC + * handler will ignore it because it is not collectable. However, + * in this context the generator is suspended in Fiber::suspend() + * and may be collectable, so we can inspect it. + * - If the generator is not running, the Generator's GC handler + * will inspect it. In this case we have to skip the frame. + */ + if (!(generator->flags & ZEND_GENERATOR_CURRENTLY_RUNNING)) { + continue; + } + symTable = zend_generator_frame_gc(buf, generator); + } else { + symTable = zend_unfinished_execution_gc_ex(ex, ex->func && ZEND_USER_CODE(ex->func->type) ? ex->call : NULL, buf, false); + } if (symTable) { if (lastSymTable) { zval *val; diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c index c17ce5c4800..dd400446c86 100644 --- a/Zend/zend_generators.c +++ b/Zend/zend_generators.c @@ -398,11 +398,38 @@ static void zend_generator_free_storage(zend_object *object) /* {{{ */ } /* }}} */ +HashTable *zend_generator_frame_gc(zend_get_gc_buffer *gc_buffer, zend_generator *generator) +{ + zend_execute_data *execute_data = generator->execute_data; + zend_execute_data *call = NULL; + + zend_get_gc_buffer_add_zval(gc_buffer, &generator->value); + zend_get_gc_buffer_add_zval(gc_buffer, &generator->key); + zend_get_gc_buffer_add_zval(gc_buffer, &generator->retval); + zend_get_gc_buffer_add_zval(gc_buffer, &generator->values); + + if (UNEXPECTED(generator->frozen_call_stack)) { + /* The frozen stack is linked in reverse order */ + call = zend_generator_revert_call_stack(generator->frozen_call_stack); + } + + HashTable *ht = zend_unfinished_execution_gc_ex(execute_data, call, gc_buffer, true); + + if (UNEXPECTED(generator->frozen_call_stack)) { + zend_generator_revert_call_stack(call); + } + + if (generator->node.parent) { + zend_get_gc_buffer_add_obj(gc_buffer, &generator->node.parent->std); + } + + return ht; +} + static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int *n) /* {{{ */ { zend_generator *generator = (zend_generator*)object; zend_execute_data *execute_data = generator->execute_data; - zend_execute_data *call = NULL; if (!execute_data) { /* If the generator has been closed, it can only hold on to three values: The value, key @@ -422,34 +449,11 @@ static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int * return NULL; } - zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); - zend_get_gc_buffer_add_zval(gc_buffer, &generator->value); - zend_get_gc_buffer_add_zval(gc_buffer, &generator->key); - zend_get_gc_buffer_add_zval(gc_buffer, &generator->retval); - zend_get_gc_buffer_add_zval(gc_buffer, &generator->values); - - if (UNEXPECTED(generator->frozen_call_stack)) { - /* The frozen stack is linked in reverse order */ - call = zend_generator_revert_call_stack(generator->frozen_call_stack); - } - - zend_unfinished_execution_gc_ex(execute_data, call, gc_buffer, true); - - if (UNEXPECTED(generator->frozen_call_stack)) { - zend_generator_revert_call_stack(call); - } - - if (generator->node.parent) { - zend_get_gc_buffer_add_obj(gc_buffer, &generator->node.parent->std); - } - + HashTable *ht = zend_generator_frame_gc(gc_buffer, generator); zend_get_gc_buffer_use(gc_buffer, table, n); - if (EX_CALL_INFO() & ZEND_CALL_HAS_SYMBOL_TABLE) { - return execute_data->symbol_table; - } else { - return NULL; - } + + return ht; } /* }}} */ diff --git a/Zend/zend_generators.h b/Zend/zend_generators.h index a93c315f50a..fc489d06ef3 100644 --- a/Zend/zend_generators.h +++ b/Zend/zend_generators.h @@ -129,6 +129,8 @@ static zend_always_inline zend_generator *zend_generator_get_current(zend_genera return zend_generator_update_current(generator); } +HashTable *zend_generator_frame_gc(zend_get_gc_buffer *gc_buffer, zend_generator *generator); + END_EXTERN_C() #endif