From 2854418275fac53eb1ae3aaa712d4bf75d3e39a5 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Sat, 22 Oct 2022 12:22:04 +0200 Subject: [PATCH] GC fiber unfinished executions --- Zend/tests/fibers/gh9735-001.phpt | 37 ++++++++++++++++++ Zend/tests/fibers/gh9735-002.phpt | 41 +++++++++++++++++++ Zend/tests/fibers/gh9735-003.phpt | 40 +++++++++++++++++++ Zend/tests/fibers/gh9735-004.phpt | 40 +++++++++++++++++++ Zend/tests/fibers/gh9735-005.phpt | 44 +++++++++++++++++++++ Zend/tests/fibers/gh9735-006.phpt | 47 ++++++++++++++++++++++ Zend/tests/fibers/gh9735-007.phpt | 47 ++++++++++++++++++++++ Zend/tests/fibers/gh9735-008.phpt | 42 ++++++++++++++++++++ Zend/tests/fibers/gh9735-009.phpt | 42 ++++++++++++++++++++ Zend/zend_execute.c | 65 +++++++++++++++++++++++++++++++ Zend/zend_execute.h | 1 + Zend/zend_fibers.c | 25 +++++++++++- Zend/zend_generators.c | 53 +++---------------------- 13 files changed, 475 insertions(+), 49 deletions(-) create mode 100644 Zend/tests/fibers/gh9735-001.phpt create mode 100644 Zend/tests/fibers/gh9735-002.phpt create mode 100644 Zend/tests/fibers/gh9735-003.phpt create mode 100644 Zend/tests/fibers/gh9735-004.phpt create mode 100644 Zend/tests/fibers/gh9735-005.phpt create mode 100644 Zend/tests/fibers/gh9735-006.phpt create mode 100644 Zend/tests/fibers/gh9735-007.phpt create mode 100644 Zend/tests/fibers/gh9735-008.phpt create mode 100644 Zend/tests/fibers/gh9735-009.phpt diff --git a/Zend/tests/fibers/gh9735-001.phpt b/Zend/tests/fibers/gh9735-001.phpt new file mode 100644 index 0000000000000..327e74323924a --- /dev/null +++ b/Zend/tests/fibers/gh9735-001.phpt @@ -0,0 +1,37 @@ +--TEST-- +Bug GH-9735 001 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECT-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-002.phpt b/Zend/tests/fibers/gh9735-002.phpt new file mode 100644 index 0000000000000..9a56c65f0abd9 --- /dev/null +++ b/Zend/tests/fibers/gh9735-002.phpt @@ -0,0 +1,41 @@ +--TEST-- +Bug GH-9735 002 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECT-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-003.phpt b/Zend/tests/fibers/gh9735-003.phpt new file mode 100644 index 0000000000000..03ea973b61dd4 --- /dev/null +++ b/Zend/tests/fibers/gh9735-003.phpt @@ -0,0 +1,40 @@ +--TEST-- +Bug GH-9735 003 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECT-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-004.phpt b/Zend/tests/fibers/gh9735-004.phpt new file mode 100644 index 0000000000000..cfcf381d83d08 --- /dev/null +++ b/Zend/tests/fibers/gh9735-004.phpt @@ -0,0 +1,40 @@ +--TEST-- +Bug GH-9735 004 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECT-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-005.phpt b/Zend/tests/fibers/gh9735-005.phpt new file mode 100644 index 0000000000000..c18a42fc37082 --- /dev/null +++ b/Zend/tests/fibers/gh9735-005.phpt @@ -0,0 +1,44 @@ +--TEST-- +Bug GH-9735 005 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECTF-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-006.phpt b/Zend/tests/fibers/gh9735-006.phpt new file mode 100644 index 0000000000000..59bb79d2404bc --- /dev/null +++ b/Zend/tests/fibers/gh9735-006.phpt @@ -0,0 +1,47 @@ +--TEST-- +Bug GH-9735 006 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECTF-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-007.phpt b/Zend/tests/fibers/gh9735-007.phpt new file mode 100644 index 0000000000000..dbb6a33821119 --- /dev/null +++ b/Zend/tests/fibers/gh9735-007.phpt @@ -0,0 +1,47 @@ +--TEST-- +Bug GH-9735 007 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECTF-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-008.phpt b/Zend/tests/fibers/gh9735-008.phpt new file mode 100644 index 0000000000000..ec6f29fb79de4 --- /dev/null +++ b/Zend/tests/fibers/gh9735-008.phpt @@ -0,0 +1,42 @@ +--TEST-- +Bug GH-9735 008 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECTF-- +1 +2 +C::__destruct +3 diff --git a/Zend/tests/fibers/gh9735-009.phpt b/Zend/tests/fibers/gh9735-009.phpt new file mode 100644 index 0000000000000..c471499443bd4 --- /dev/null +++ b/Zend/tests/fibers/gh9735-009.phpt @@ -0,0 +1,42 @@ +--TEST-- +Bug GH-9735 009 (Fiber stack variables do not participate in cycle collector) +--FILE-- +start(); +gc_collect_cycles(); + +print "2\n"; + +$fiber = null; +gc_collect_cycles(); + +print "3\n"; + +?> +--EXPECTF-- +1 +2 +C::__destruct +3 diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 40b89e062e758..fc555bfce1184 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -4450,6 +4450,71 @@ ZEND_API void zend_cleanup_unfinished_execution(zend_execute_data *execute_data, cleanup_live_vars(execute_data, op_num, catch_op_num); } +ZEND_API HashTable *zend_unfinished_execution_gc(zend_execute_data *execute_data, zend_execute_data *call, zend_get_gc_buffer *gc_buffer) +{ + if (!EX(func) || !ZEND_USER_CODE(EX(func)->common.type)) { + return NULL; + } + + zend_op_array *op_array = &EX(func)->op_array; + + if (!(EX_CALL_INFO() & ZEND_CALL_HAS_SYMBOL_TABLE)) { + uint32_t i, num_cvs = EX(func)->op_array.last_var; + for (i = 0; i < num_cvs; i++) { + zend_get_gc_buffer_add_zval(gc_buffer, EX_VAR_NUM(i)); + } + } + + if (EX_CALL_INFO() & ZEND_CALL_FREE_EXTRA_ARGS) { + zval *zv = EX_VAR_NUM(op_array->last_var + op_array->T); + zval *end = zv + (EX_NUM_ARGS() - op_array->num_args); + while (zv != end) { + zend_get_gc_buffer_add_zval(gc_buffer, zv++); + } + } + + 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)); + zend_get_gc_buffer_add_zval(gc_buffer, &extra_named_params); + } + + if (call) { + /* -1 required because we want the last run opcode, not the next to-be-run one. */ + uint32_t op_num = execute_data->opline - op_array->opcodes - 1; + zend_unfinished_calls_gc(execute_data, call, op_num, gc_buffer); + } + + if (execute_data->opline != op_array->opcodes) { + uint32_t i, op_num = execute_data->opline - op_array->opcodes - 1; + for (i = 0; i < op_array->last_live_range; i++) { + const zend_live_range *range = &op_array->live_range[i]; + if (range->start > op_num) { + break; + } else if (op_num < range->end) { + uint32_t kind = range->var & ZEND_LIVE_MASK; + uint32_t var_num = range->var & ~ZEND_LIVE_MASK; + zval *var = EX_VAR(var_num); + if (kind == ZEND_LIVE_TMPVAR || kind == ZEND_LIVE_LOOP) { + zend_get_gc_buffer_add_zval(gc_buffer, var); + } + } + } + } + + if (EX_CALL_INFO() & ZEND_CALL_HAS_SYMBOL_TABLE) { + return execute_data->symbol_table; + } else { + return NULL; + } +} + #if ZEND_VM_SPEC static void zend_swap_operands(zend_op *op) /* {{{ */ { diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index f09a8fe769417..3e61967cb2bea 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -378,6 +378,7 @@ ZEND_API void zend_clean_and_cache_symbol_table(zend_array *symbol_table); ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *execute_data); ZEND_API void zend_unfinished_calls_gc(zend_execute_data *execute_data, zend_execute_data *call, uint32_t op_num, zend_get_gc_buffer *buf); ZEND_API void zend_cleanup_unfinished_execution(zend_execute_data *execute_data, uint32_t op_num, uint32_t catch_op_num); +ZEND_API HashTable *zend_unfinished_execution_gc(zend_execute_data *execute_data, zend_execute_data *call, zend_get_gc_buffer *gc_buffer); zval * ZEND_FASTCALL zend_handle_named_arg( zend_execute_data **call_ptr, zend_string *arg_name, diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index 1fec85528fbb3..4b46447a460bb 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -24,6 +24,8 @@ #include "zend_exceptions.h" #include "zend_builtin_functions.h" #include "zend_observer.h" +#include "zend_compile.h" +#include "zend_closures.h" #include "zend_fibers.h" #include "zend_fibers_arginfo.h" @@ -637,9 +639,30 @@ static HashTable *zend_fiber_object_gc(zend_object *object, zval **table, int *n zend_get_gc_buffer_add_zval(buf, &fiber->fci.function_name); zend_get_gc_buffer_add_zval(buf, &fiber->result); + if (fiber->context.status != ZEND_FIBER_STATUS_SUSPENDED) { + zend_get_gc_buffer_use(buf, table, num); + return NULL; + } + + 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->call, buf); + if (symTable) { + if (lastSymTable) { + zval *val; + ZEND_HASH_FOREACH_VAL(lastSymTable, val) { + ZEND_ASSERT(Z_TYPE_P(val) == IS_INDIRECT); + zend_get_gc_buffer_add_zval(buf, Z_INDIRECT_P(val)); + } ZEND_HASH_FOREACH_END(); + } + lastSymTable = symTable; + } + } + zend_get_gc_buffer_use(buf, table, num); - return NULL; + return lastSymTable; } ZEND_METHOD(Fiber, __construct) diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c index 71bf2f092debd..d5253e46eddf2 100644 --- a/Zend/zend_generators.c +++ b/Zend/zend_generators.c @@ -332,7 +332,7 @@ static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int * { zend_generator *generator = (zend_generator*)object; zend_execute_data *execute_data = generator->execute_data; - zend_op_array *op_array; + 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 @@ -352,7 +352,6 @@ static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int * return NULL; } - op_array = &EX(func)->op_array; zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); zend_get_gc_buffer_add_zval(gc_buffer, &generator->value); @@ -360,59 +359,17 @@ static HashTable *zend_generator_get_gc(zend_object *object, zval **table, int * zend_get_gc_buffer_add_zval(gc_buffer, &generator->retval); zend_get_gc_buffer_add_zval(gc_buffer, &generator->values); - if (!(EX_CALL_INFO() & ZEND_CALL_HAS_SYMBOL_TABLE)) { - uint32_t i, num_cvs = EX(func)->op_array.last_var; - for (i = 0; i < num_cvs; i++) { - zend_get_gc_buffer_add_zval(gc_buffer, EX_VAR_NUM(i)); - } - } - - if (EX_CALL_INFO() & ZEND_CALL_FREE_EXTRA_ARGS) { - zval *zv = EX_VAR_NUM(op_array->last_var + op_array->T); - zval *end = zv + (EX_NUM_ARGS() - op_array->num_args); - while (zv != end) { - zend_get_gc_buffer_add_zval(gc_buffer, zv++); - } + if (UNEXPECTED(generator->frozen_call_stack)) { + /* The frozen stack is linked in reverse order */ + call = zend_generator_revert_call_stack(generator->frozen_call_stack); } - 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)); - zend_get_gc_buffer_add_zval(gc_buffer, &extra_named_params); - } + zend_unfinished_execution_gc(execute_data, call, gc_buffer); if (UNEXPECTED(generator->frozen_call_stack)) { - /* The frozen stack is linked in reverse order */ - zend_execute_data *call = zend_generator_revert_call_stack(generator->frozen_call_stack); - /* -1 required because we want the last run opcode, not the next to-be-run one. */ - uint32_t op_num = execute_data->opline - op_array->opcodes - 1; - zend_unfinished_calls_gc(execute_data, call, op_num, gc_buffer); zend_generator_revert_call_stack(call); } - if (execute_data->opline != op_array->opcodes) { - uint32_t i, op_num = execute_data->opline - op_array->opcodes - 1; - for (i = 0; i < op_array->last_live_range; i++) { - const zend_live_range *range = &op_array->live_range[i]; - if (range->start > op_num) { - break; - } else if (op_num < range->end) { - uint32_t kind = range->var & ZEND_LIVE_MASK; - uint32_t var_num = range->var & ~ZEND_LIVE_MASK; - zval *var = EX_VAR(var_num); - if (kind == ZEND_LIVE_TMPVAR || kind == ZEND_LIVE_LOOP) { - zend_get_gc_buffer_add_zval(gc_buffer, var); - } - } - } - } - if (generator->node.parent) { zend_get_gc_buffer_add_obj(gc_buffer, &generator->node.parent->std); }