fibers: refactor fiber to only rely on a FCC

Zend/tests/fibers/destructors_015.phpt

@@ -2,6 +2,8 @@
 Fibers in destructors 015: Exception thrown by a destructor called by the GC from internal code
 --INI--
 zend.enable_gc=1
+--XFAIL--
+Destructor exception is not caught if fiber is produced from create_fiber()???
 --FILE--
 <?php
 

Zend/tests/fibers/fiber-callable-object-outlives-caller.phpt

@@ -66,20 +66,20 @@ echo "end\n";
 --EXPECT--
 -- started after its creator's last reference went
 m on a1
-released a1
 after start
 -- a string callable
+released a1
 m on a2
-released a2
 after start
 -- never started, released with the fiber
+released a2
 released a3
 after unset
 -- suspended while its object has no other reference
 resumed on a4
-released a4
 after resume
 -- a cycle through the fiber is collected
 released a5
 int(2)
 end
+released a4

Zend/zend_fibers.c

@@ -563,39 +563,6 @@ static void zend_fiber_cleanup(zend_fiber_context *context)
 	fiber->caller = NULL;
 }
 
-static void zend_fiber_addref_callable(zend_fiber *fiber)
-{
-	// Keep a reference to closures or callable objects while the fiber is running.
-	Z_TRY_ADDREF(fiber->fci.function_name);
-
-	/* For [A::class, 'm'] or "A::m" inside a method of A, fci.object is the caller's $this. */
-	if (fiber->fci.object != NULL) {
-		GC_ADDREF(fiber->fci.object);
-	}
-}
-
-static void zend_fiber_get_gc_callable(zend_fiber *fiber, zend_get_gc_buffer *buf)
-{
-	zend_get_gc_buffer_add_zval(buf, &fiber->fci.function_name);
-
-	if (fiber->fci.object != NULL) {
-		zend_get_gc_buffer_add_obj(buf, fiber->fci.object);
-	}
-}
-
-static void zend_fiber_release_callable(zend_fiber *fiber)
-{
-	zval_ptr_dtor(&fiber->fci.function_name);
-	ZVAL_UNDEF(&fiber->fci.function_name);
-	fiber->fci_cache = empty_fcall_info_cache;
-
-	if (fiber->fci.object != NULL) {
-		zend_object *object = fiber->fci.object;
-		fiber->fci.object = NULL;
-		OBJ_RELEASE(object);
-	}
-}
-
 static ZEND_STACK_ALIGNED void zend_fiber_execute(zend_fiber_transfer *transfer)
 {
 	ZEND_ASSERT(Z_TYPE(transfer->value) == IS_NULL && "Initial transfer value to fiber context must be NULL");
@@ -633,12 +600,7 @@ static ZEND_STACK_ALIGNED void zend_fiber_execute(zend_fiber_transfer *transfer)
 		EG(stack_limit) = zend_fiber_stack_limit(fiber->context.stack);
 #endif
 
-		fiber->fci.retval = &fiber->result;
-
-		zend_call_function(&fiber->fci, &fiber->fci_cache);
-
-		/* Cleanup callback and unset field to prevent GC / duplicate dtor issues. */
-		zend_fiber_release_callable(fiber);
+		zend_call_known_fcc(&fiber->fci_cache, &fiber->result, fiber->param_count, fiber->params, fiber->named_params);
 
 		if (EG(exception)) {
 			if (!(fiber->flags & ZEND_FIBER_FLAG_DESTROYED) || !zend_is_graceful_exit(EG(exception))) {
@@ -838,7 +800,9 @@ static void zend_fiber_object_free(zend_object *object)
 {
 	zend_fiber *fiber = (zend_fiber *) object;
 
-	zend_fiber_release_callable(fiber);
+	if (ZEND_FCC_INITIALIZED(fiber->fci_cache)) {
+		zend_fcc_dtor(&fiber->fci_cache);
+	}
 	zval_ptr_dtor(&fiber->result);
 
 	zend_object_std_dtor(&fiber->std);
@@ -849,7 +813,7 @@ static HashTable *zend_fiber_object_gc(zend_object *object, zval **table, int *n
 	zend_fiber *fiber = (zend_fiber *) object;
 	zend_get_gc_buffer *buf = zend_get_gc_buffer_create();
 
-	zend_fiber_get_gc_callable(fiber, buf);
+	zend_get_gc_buffer_add_fcc(buf, &fiber->fci_cache);
 	zend_get_gc_buffer_add_zval(buf, &fiber->result);
 
 	if (fiber->context.status != ZEND_FIBER_STATUS_SUSPENDED || fiber->caller != NULL) {
@@ -904,28 +868,27 @@ ZEND_METHOD(Fiber, __construct)
 	zend_fcall_info_cache fcc;
 
 	ZEND_PARSE_PARAMETERS_START(1, 1)
-		Z_PARAM_FUNC(fci, fcc)
+		Z_PARAM_FUNC_NO_TRAMPOLINE_FREE(fci, fcc)
 	ZEND_PARSE_PARAMETERS_END();
 
 	zend_fiber *fiber = (zend_fiber *) Z_OBJ_P(ZEND_THIS);
 
-	if (UNEXPECTED(fiber->context.status != ZEND_FIBER_STATUS_INIT || Z_TYPE(fiber->fci.function_name) != IS_UNDEF)) {
+	if (UNEXPECTED(fiber->context.status != ZEND_FIBER_STATUS_INIT || ZEND_FCC_INITIALIZED(fiber->fci_cache))) {
 		zend_throw_error(zend_ce_fiber_error, "Cannot call constructor twice");
+		/* Release potential trampoline as ZPP will not have freed it. */
+		zend_release_fcall_info_cache(&fcc);
 		RETURN_THROWS();
 	}
 
-	fiber->fci = fci;
-	fiber->fci_cache = fcc;
-
-	zend_fiber_addref_callable(fiber);
+	zend_fcc_dup(&fiber->fci_cache, &fcc);
 }
 
 ZEND_METHOD(Fiber, start)
 {
 	zend_fiber *fiber = (zend_fiber *) Z_OBJ_P(ZEND_THIS);
 
 	ZEND_PARSE_PARAMETERS_START(0, -1)
-		Z_PARAM_VARIADIC_WITH_NAMED(fiber->fci.params, fiber->fci.param_count, fiber->fci.named_params);
+		Z_PARAM_VARIADIC_WITH_NAMED(fiber->params, fiber->param_count, fiber->named_params);
 	ZEND_PARSE_PARAMETERS_END();
 
 	if (UNEXPECTED(zend_fiber_switch_blocked())) {

Zend/zend_fibers.h

@@ -101,9 +101,6 @@ struct _zend_fiber {
 	/* PHP object handle. */
 	zend_object std;
 
-	/* Flags are defined in enum zend_fiber_flag. */
-	uint8_t flags;
-
 	/* Native C fiber context. */
 	zend_fiber_context context;
 
@@ -113,8 +110,13 @@ struct _zend_fiber {
 	/* Fiber that suspended us. */
 	zend_fiber_context *previous;
 
-	/* Callback and info / cache to be used when fiber is started. */
-	zend_fcall_info fci;
+	/* Flags are defined in enum zend_fiber_flag. */
+	uint8_t flags;
+
+	/* Callback and parameters to be used when fiber is started. */
+	uint32_t param_count;
+	zval *params;
+	HashTable *named_params;
 	zend_fcall_info_cache fci_cache;
 
 	/* Current Zend VM execute data being run by the fiber. */

Zend/zend_gc.c

@@ -1923,7 +1923,6 @@ static zend_fiber *gc_create_destructor_fiber(void)
 	}
 
 	fiber = (zend_fiber *)Z_OBJ(zobj);
-	fiber->fci.size = sizeof(fiber->fci);
 	fiber->fci_cache.function_handler = (zend_function*) &gc_destructor_fiber;
 
 	GC_G(dtor_fiber) = fiber;

ext/reflection/php_reflection.c

@@ -7937,7 +7937,7 @@ ZEND_METHOD(ReflectionFiber, getCallable)
 		RETURN_THROWS();
 	}
 
-	RETURN_COPY(&fiber->fci.function_name);
+	zend_get_callable_zval_from_fcc(&fiber->fci_cache, return_value);
 }
 
 /* {{{ _reflection_write_property */

ext/zend_test/fiber.c

@@ -98,12 +98,10 @@ static ZEND_STACK_ALIGNED void zend_test_fiber_execute(zend_fiber_transfer *tran
 		EG(stack_base) = zend_fiber_stack_base(fiber->context.stack);
 		EG(stack_limit) = zend_fiber_stack_limit(fiber->context.stack);
 #endif
-		fiber->fci.retval = &retval;
 
-		zend_call_function(&fiber->fci, &fiber->fci_cache);
+		zend_call_known_fcc(&fiber->fci_cache, &retval, fiber->param_count, fiber->params, fiber->named_params);
 
 		zval_ptr_dtor(&fiber->result); // Destroy param from symmetric coroutine.
-		zval_ptr_dtor(&fiber->fci.function_name);
 
 		if (EG(exception)) {
 			if (!(fiber->flags & ZEND_FIBER_FLAG_DESTROYED)
@@ -133,8 +131,8 @@ static ZEND_STACK_ALIGNED void zend_test_fiber_execute(zend_fiber_transfer *tran
 		transfer->context = target;
 
 		ZVAL_COPY(&fiber->target->result, &fiber->result);
-		fiber->target->fci.params = &fiber->target->result;
-		fiber->target->fci.param_count = 1;
+		fiber->target->params = &fiber->target->result;
+		fiber->target->param_count = 1;
 
 		fiber->target->caller = fiber->caller;
 		ZT_G(active_fiber) = fiber->target;
@@ -195,9 +193,8 @@ static void zend_test_fiber_object_free(zend_object *object)
 {
 	zend_test_fiber *fiber = (zend_test_fiber *) object;
 
-	if (fiber->context.status == ZEND_FIBER_STATUS_INIT) {
-		// Fiber was never started, so we need to release the reference to the callback.
-		zval_ptr_dtor(&fiber->fci.function_name);
+	if (ZEND_FCC_INITIALIZED(fiber->fci_cache)) {
+		zend_fcc_dtor(&fiber->fci_cache);
 	}
 
 	if (fiber->target) {
@@ -227,14 +224,14 @@ static zend_always_inline void delegate_transfer_result(
 
 static ZEND_METHOD(_ZendTestFiber, __construct)
 {
+	zend_fcall_info fci;
 	zend_test_fiber *fiber = (zend_test_fiber *) Z_OBJ_P(ZEND_THIS);
 
 	ZEND_PARSE_PARAMETERS_START(1, 1)
-		Z_PARAM_FUNC(fiber->fci, fiber->fci_cache)
+		Z_PARAM_FUNC_NO_TRAMPOLINE_FREE(fci, fiber->fci_cache)
 	ZEND_PARSE_PARAMETERS_END();
 
-	// Keep a reference to closures or callable objects while the fiber is running.
-	Z_TRY_ADDREF(fiber->fci.function_name);
+	zend_fcc_addref(&fiber->fci_cache);
 }
 
 static ZEND_METHOD(_ZendTestFiber, start)
@@ -255,9 +252,9 @@ static ZEND_METHOD(_ZendTestFiber, start)
 		RETURN_THROWS();
 	}
 
-	fiber->fci.params = params;
-	fiber->fci.param_count = param_count;
-	fiber->fci.named_params = named_params;
+	fiber->params = params;
+	fiber->param_count = param_count;
+	fiber->named_params = named_params;
 
 	zend_fiber_init_context(&fiber->context, zend_test_fiber_class, zend_test_fiber_execute, EG(fiber_stack_size));
 
@@ -321,15 +318,13 @@ static ZEND_METHOD(_ZendTestFiber, pipeTo)
 	zend_fcall_info_cache fci_cache;
 
 	ZEND_PARSE_PARAMETERS_START(1, 1)
-		Z_PARAM_FUNC(fci, fci_cache)
+		Z_PARAM_FUNC_NO_TRAMPOLINE_FREE(fci, fci_cache)
 	ZEND_PARSE_PARAMETERS_END();
 
 	zend_test_fiber *fiber = (zend_test_fiber *) Z_OBJ_P(ZEND_THIS);
 	zend_test_fiber *target = (zend_test_fiber *) zend_test_fiber_class->create_object(zend_test_fiber_class);
 
-	target->fci = fci;
-	target->fci_cache = fci_cache;
-	Z_TRY_ADDREF(target->fci.function_name);
+	zend_fcc_dup(&target->fci_cache, &fci_cache);
 
 	target->previous = &fiber->context;
 

ext/zend_test/fiber.h

@@ -21,12 +21,14 @@ typedef struct _zend_test_fiber zend_test_fiber;
 
 struct _zend_test_fiber {
 	zend_object std;
-	uint8_t flags;
 	zend_fiber_context context;
 	zend_fiber_context *caller;
 	zend_fiber_context *previous;
 	zend_test_fiber *target;
-	zend_fcall_info fci;
+	uint8_t flags;
+	uint32_t param_count;
+	zval *params;
+	HashTable *named_params;
 	zend_fcall_info_cache fci_cache;
 	zval result;
 };