GC fiber unfinished executions (#9810)

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--
+<?php
+
+class C {
+    public function __destruct() {
+        echo __METHOD__, "\n";
+    }
+}
+
+$fiber = new Fiber(function () {
+    $c = new C();
+
+    $fiber = Fiber::getCurrent();
+
+    Fiber::suspend();
+});
+
+print "1\n";
+
+$fiber->start();
+gc_collect_cycles();
+
+print "2\n";
+
+$fiber = null;
+gc_collect_cycles();
+
+print "3\n";
+
+?>
+--EXPECT--
+1
+2
+C::__destruct
+3

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--
+<?php
+
+class C {
+    public function __destruct() {
+        echo __METHOD__, "\n";
+    }
+}
+
+function f() {
+    Fiber::suspend();
+}
+
+$fiber = new Fiber(function () {
+    $c = new C();
+
+    $fiber = Fiber::getCurrent();
+
+    f();
+});
+
+print "1\n";
+
+$fiber->start();
+gc_collect_cycles();
+
+print "2\n";
+
+$fiber = null;
+gc_collect_cycles();
+
+print "3\n";
+
+?>
+--EXPECT--
+1
+2
+C::__destruct
+3

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--
+<?php
+
+class C {
+    public function __destruct() {
+        echo __METHOD__, "\n";
+    }
+}
+
+function f() {
+    $fiber = Fiber::getCurrent();
+    Fiber::suspend();
+}
+
+$fiber = new Fiber(function () {
+    $c = new C();
+
+    f();
+});
+
+print "1\n";
+
+$fiber->start();
+gc_collect_cycles();
+
+print "2\n";
+
+$fiber = null;
+gc_collect_cycles();
+
+print "3\n";
+
+?>
+--EXPECT--
+1
+2
+C::__destruct
+3

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--
+<?php
+
+class C {
+    public function __destruct() {
+        echo __METHOD__, "\n";
+    }
+}
+
+function f() {
+    $fiber = Fiber::getCurrent();
+    Fiber::suspend();
+}
+
+$fiber = new Fiber(function () {
+    $c = new C();
+
+    preg_replace_callback('#.#', f(...), '.');
+});
+
+print "1\n";
+
+$fiber->start();
+gc_collect_cycles();
+
+print "2\n";
+
+$fiber = null;
+gc_collect_cycles();
+
+print "3\n";
+
+?>
+--EXPECT--
+1
+2
+C::__destruct
+3

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--
+<?php
+
+class C {
+    public function __destruct() {
+        echo __METHOD__, "\n";
+    }
+}
+
+function f() {
+    Fiber::suspend();
+}
+
+$fiber = new Fiber(function () {
+    $c = new C();
+
+    $fiber = Fiber::getCurrent();
+
+    // Force symbol table
+    get_defined_vars();
+
+    f();
+});
+
+print "1\n";
+
+$fiber->start();
+gc_collect_cycles();
+
+print "2\n";
+
+$fiber = null;
+gc_collect_cycles();
+
+print "3\n";
+
+?>
+--EXPECTF--
+1
+2
+C::__destruct
+3

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--
+<?php
+
+class C {
+    public function __destruct() {
+        echo __METHOD__, "\n";
+    }
+}
+
+function f() {
+    // Force symbol table
+    get_defined_vars();
+
+    Fiber::suspend();
+}
+
+$fiber = new Fiber(function () {
+    $c = new C();
+
+    $fiber = Fiber::getCurrent();
+
+    // Force symbol table
+    get_defined_vars();
+
+    f();
+});
+
+print "1\n";
+
+$fiber->start();
+gc_collect_cycles();
+
+print "2\n";
+
+$fiber = null;
+gc_collect_cycles();
+
+print "3\n";
+
+?>
+--EXPECTF--
+1
+2
+C::__destruct
+3

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--
+<?php
+
+class C {
+    public function __destruct() {
+        echo __METHOD__, "\n";
+    }
+}
+
+function f() {
+    $fiber = Fiber::getCurrent();
+
+    // Force symbol table
+    get_defined_vars();
+
+    Fiber::suspend();
+}
+
+$fiber = new Fiber(function () {
+    $c = new C();
+
+    // Force symbol table
+    get_defined_vars();
+
+    f();
+});
+
+print "1\n";
+
+$fiber->start();
+gc_collect_cycles();
+
+print "2\n";
+
+$fiber = null;
+gc_collect_cycles();
+
+print "3\n";
+
+?>
+--EXPECTF--
+1
+2
+C::__destruct
+3

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--
+<?php
+
+class C {
+    public function __destruct() {
+        echo __METHOD__, "\n";
+    }
+}
+
+function f($a, $b) {
+}
+
+function g() {
+    Fiber::suspend();
+}
+
+$fiber = new Fiber(function () {
+    $c = new C();
+
+    f(Fiber::getCurrent(), g());
+});
+
+print "1\n";
+
+$fiber->start();
+gc_collect_cycles();
+
+print "2\n";
+
+$fiber = null;
+gc_collect_cycles();
+
+print "3\n";
+
+?>
+--EXPECTF--
+1
+2
+C::__destruct
+3

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--
+<?php
+
+class C {
+    public function __destruct() {
+        echo __METHOD__, "\n";
+    }
+}
+
+function f($a, $b) {
+}
+
+function g() {
+    g(Fiber::suspend());
+}
+
+$fiber = new Fiber(function () {
+    $c = new C();
+
+    f(Fiber::getCurrent(), g());
+});
+
+print "1\n";
+
+$fiber->start();
+gc_collect_cycles();
+
+print "2\n";
+
+$fiber = null;
+gc_collect_cycles();
+
+print "3\n";
+
+?>
+--EXPECTF--
+1
+2
+C::__destruct
+3

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) /* {{{ */
 {

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,

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"
@@ -641,9 +643,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)

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,57 +359,15 @@ 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 (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 (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);
+		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) {
+	zend_unfinished_execution_gc(execute_data, call, gc_buffer);
-		uint32_t i, op_num = execute_data->opline - op_array->opcodes - 1;
+
-		for (i = 0; i < op_array->last_live_range; i++) {
+	if (UNEXPECTED(generator->frozen_call_stack)) {
-			const zend_live_range *range = &op_array->live_range[i];
+		zend_generator_revert_call_stack(call);
-			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) {