From e2a058d93f7ed39c378705ccae12f8d25d601979 Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sat, 5 Jun 2021 13:01:57 -0500 Subject: [PATCH 1/9] Merge fiber switching functions --- Zend/tests/fibers/fiber-status.phpt | 18 ++++++ Zend/zend_fibers.c | 85 ++++++++++++----------------- Zend/zend_fibers.h | 14 ++--- ext/zend_test/test.c | 4 +- 4 files changed, 63 insertions(+), 58 deletions(-) diff --git a/Zend/tests/fibers/fiber-status.phpt b/Zend/tests/fibers/fiber-status.phpt index 45b2c60e05711..92c37a2ef67ba 100644 --- a/Zend/tests/fibers/fiber-status.phpt +++ b/Zend/tests/fibers/fiber-status.phpt @@ -10,6 +10,18 @@ $fiber = new Fiber(function (): void { var_dump($fiber->isRunning()); var_dump($fiber->isSuspended()); var_dump($fiber->isTerminated()); + + $nested = new Fiber(function () use ($fiber): void { + echo "\nWithin Nested Fiber:\n"; + var_dump($fiber->isStarted()); + var_dump($fiber->isRunning()); + var_dump($fiber->isSuspended()); + var_dump($fiber->isTerminated()); + Fiber::suspend(); + }); + + $nested->start(); + Fiber::suspend(); }); @@ -49,6 +61,12 @@ bool(true) bool(false) bool(false) +Within Nested Fiber: +bool(true) +bool(true) +bool(false) +bool(false) + After Start: bool(true) bool(false) diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index e3276a28bf326..573595fcfda1d 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -192,8 +192,12 @@ static ZEND_NORETURN void zend_fiber_trampoline(transfer_t transfer) __sanitizer_finish_switch_fiber(NULL, &from->stack.prior_pointer, &from->stack.prior_size); #endif + from = context->function(context); + + context->status = ZEND_FIBER_STATUS_DEAD; + /* Final context switch, the fiber must not be resumed afterwards! */ - zend_fiber_switch_context(context->function(context)); + zend_fiber_switch_context(from); /* Abort here because we are in an inconsistent program state. */ abort(); @@ -225,11 +229,23 @@ ZEND_API void zend_fiber_destroy_context(zend_fiber_context *context) ZEND_API void zend_fiber_switch_context(zend_fiber_context *to) { zend_fiber_context *from = EG(current_fiber); + zend_fiber_vm_state state; ZEND_ASSERT(to && to->handle && "Invalid fiber context"); ZEND_ASSERT(from && "From fiber context must be present"); ZEND_ASSERT(to != from && "Cannot switch into the running fiber context"); + zend_observer_fiber_switch_notify(from, to); + + zend_fiber_capture_vm_state(&state); + + to->status = ZEND_FIBER_STATUS_RUNNING; + if (to->caller == NULL) { + to->caller = from; + } + + EG(current_fiber) = to; + #ifdef __SANITIZE_ADDRESS__ void *fake_stack = NULL; __sanitizer_start_switch_fiber( @@ -238,50 +254,30 @@ ZEND_API void zend_fiber_switch_context(zend_fiber_context *to) to->stack.prior_size); #endif - EG(current_fiber) = to; - transfer_t transfer = jump_fcontext(to->handle, from); - ((zend_fiber_context *) transfer.data)->handle = transfer.context; - - EG(current_fiber) = from; #ifdef __SANITIZE_ADDRESS__ __sanitizer_finish_switch_fiber(fake_stack, &to->stack.prior_pointer, &to->stack.prior_size); #endif -} -static void zend_fiber_suspend_from(zend_fiber *fiber) -{ - zend_fiber_vm_state state; - - ZEND_ASSERT(fiber->caller && "Fiber has no caller"); - - zend_fiber_capture_vm_state(&state); - zend_fiber_switch_context(fiber->caller); - zend_fiber_restore_vm_state(&state); -} - -static void zend_fiber_switch_to(zend_fiber *fiber) -{ - zend_fiber_context *context = zend_fiber_get_context(fiber); - zend_fiber_vm_state state; + ((zend_fiber_context *) transfer.data)->handle = transfer.context; - zend_observer_fiber_switch_notify(EG(current_fiber), context); + EG(current_fiber) = from; - fiber->caller = EG(current_fiber); + if (to->caller == from && to->status == ZEND_FIBER_STATUS_RUNNING) { + to->status = ZEND_FIBER_STATUS_SUSPENDED; + } - zend_fiber_capture_vm_state(&state); - zend_fiber_switch_context(context); zend_fiber_restore_vm_state(&state); - fiber->caller = NULL; - - zend_observer_fiber_switch_notify(context, EG(current_fiber)); - - if (UNEXPECTED(fiber->flags & ZEND_FIBER_FLAG_BAILOUT)) { + if (UNEXPECTED(to->flags & ZEND_FIBER_FLAG_BAILOUT)) { // zend_bailout() was called in the fiber, so call it again in the previous fiber or {main}. zend_bailout(); } + + if (to->status == ZEND_FIBER_STATUS_DEAD) { + zend_fiber_destroy_context(to); + } } @@ -328,8 +324,6 @@ static ZEND_STACK_ALIGNED zend_fiber_context *zend_fiber_execute(zend_fiber_cont fiber->fci.retval = &fiber->value; - fiber->status = ZEND_FIBER_STATUS_RUNNING; - zend_call_function(&fiber->fci, &fiber->fci_cache); zval_ptr_dtor(&fiber->fci.function_name); @@ -347,8 +341,6 @@ static ZEND_STACK_ALIGNED zend_fiber_context *zend_fiber_execute(zend_fiber_cont fiber->flags |= ZEND_FIBER_FLAG_BAILOUT; } zend_end_try(); - fiber->status = ZEND_FIBER_STATUS_DEAD; - zend_vm_stack_destroy(); fiber->execute_data = NULL; fiber->stack_bottom = NULL; @@ -380,10 +372,9 @@ static void zend_fiber_object_destroy(zend_object *object) zend_object *exception = EG(exception); EG(exception) = NULL; - fiber->status = ZEND_FIBER_STATUS_RUNNING; fiber->flags |= ZEND_FIBER_FLAG_DESTROYED; - zend_fiber_switch_to(fiber); + zend_fiber_switch_context(zend_fiber_get_context(fiber)); if (EG(exception)) { if (!exception && EG(current_execute_data) && EG(current_execute_data)->func @@ -412,8 +403,6 @@ static void zend_fiber_object_free(zend_object *object) zval_ptr_dtor(&fiber->value); - zend_fiber_destroy_context(zend_fiber_get_context(fiber)); - zend_object_std_dtor(&fiber->std); } @@ -443,6 +432,8 @@ ZEND_METHOD(Fiber, __construct) ZEND_API void zend_fiber_start(zend_fiber *fiber, zval *params, uint32_t param_count, zend_array *named_params, zval *return_value) { + zend_fiber_context *context = zend_fiber_get_context(fiber); + if (fiber->status != ZEND_FIBER_STATUS_INIT) { zend_throw_error(zend_ce_fiber_error, "Cannot start a fiber that has already been started"); RETURN_THROWS(); @@ -452,11 +443,11 @@ ZEND_API void zend_fiber_start(zend_fiber *fiber, zval *params, uint32_t param_c fiber->fci.param_count = param_count; fiber->fci.named_params = named_params; - if (!zend_fiber_init_context(zend_fiber_get_context(fiber), zend_ce_fiber, zend_fiber_execute, EG(fiber_stack_size))) { + if (!zend_fiber_init_context(context, zend_ce_fiber, zend_fiber_execute, EG(fiber_stack_size))) { RETURN_THROWS(); } - zend_fiber_switch_to(fiber); + zend_fiber_switch_context(context); if (fiber->status == ZEND_FIBER_STATUS_DEAD) { RETURN_NULL(); @@ -503,10 +494,9 @@ ZEND_API void zend_fiber_suspend(zval *value, zval *return_value) } fiber->execute_data = EG(current_execute_data); - fiber->status = ZEND_FIBER_STATUS_SUSPENDED; fiber->stack_bottom->prev_execute_data = NULL; - zend_fiber_suspend_from(fiber); + zend_fiber_switch_context(zend_fiber_get_context(fiber)->caller); if (fiber->flags & ZEND_FIBER_FLAG_DESTROYED) { // This occurs when the fiber is GC'ed while suspended. @@ -514,8 +504,6 @@ ZEND_API void zend_fiber_suspend(zval *value, zval *return_value) RETURN_THROWS(); } - fiber->status = ZEND_FIBER_STATUS_RUNNING; - if (fiber->exception) { zval *exception = fiber->exception; fiber->exception = NULL; @@ -553,10 +541,9 @@ ZEND_API void zend_fiber_resume(zend_fiber *fiber, zval *value, zval *return_val ZVAL_NULL(&fiber->value); } - fiber->status = ZEND_FIBER_STATUS_RUNNING; fiber->stack_bottom->prev_execute_data = EG(current_execute_data); - zend_fiber_switch_to(fiber); + zend_fiber_switch_context(zend_fiber_get_context(fiber)); if (fiber->status == ZEND_FIBER_STATUS_DEAD) { RETURN_NULL(); @@ -591,10 +578,9 @@ ZEND_API void zend_fiber_throw(zend_fiber *fiber, zval *exception, zval *return_ Z_ADDREF_P(exception); fiber->exception = exception; - fiber->status = ZEND_FIBER_STATUS_RUNNING; fiber->stack_bottom->prev_execute_data = EG(current_execute_data); - zend_fiber_switch_to(fiber); + zend_fiber_switch_context(zend_fiber_get_context(fiber)); if (fiber->status == ZEND_FIBER_STATUS_DEAD) { RETURN_NULL(); @@ -731,6 +717,7 @@ void zend_fiber_init(void) zend_fiber_context *context = ecalloc(1, sizeof(zend_fiber_context)); context->status = ZEND_FIBER_STATUS_RUNNING; + context->caller = (zend_fiber_context *) -1; EG(main_fiber) = context; EG(current_fiber) = context; diff --git a/Zend/zend_fibers.h b/Zend/zend_fibers.h index df909b55c4e1e..767a85be1c651 100644 --- a/Zend/zend_fibers.h +++ b/Zend/zend_fibers.h @@ -76,6 +76,7 @@ typedef zend_fiber_context *(*zend_fiber_coroutine)(zend_fiber_context *context) void *handle; \ /* Pointer that identifies the fiber type. */ \ void *kind; \ + zend_fiber_context *caller; \ zend_fiber_coroutine function; \ zend_fiber_stack stack; \ zend_fiber_status status; \ @@ -89,6 +90,8 @@ struct _zend_fiber_context { /* Zend VM state that needs to be captured / restored during fiber context switch. */ typedef struct _zend_fiber_vm_state { zend_vm_stack vm_stack; + zval* vm_stack_top; + zval* vm_stack_end; size_t vm_stack_page_size; zend_execute_data *current_execute_data; int error_reporting; @@ -100,9 +103,6 @@ struct _zend_fiber { /* PHP object handle. */ zend_object std; - /* Fiber that resumed us. */ - zend_fiber_context *caller; - /* Fiber context fields (embedded to avoid memory allocation). */ ZEND_FIBER_CONTEXT_FIELDS; @@ -152,8 +152,8 @@ static zend_always_inline zend_fiber_context *zend_fiber_get_context(zend_fiber static zend_always_inline void zend_fiber_capture_vm_state(zend_fiber_vm_state *state) { state->vm_stack = EG(vm_stack); - state->vm_stack->top = EG(vm_stack_top); - state->vm_stack->end = EG(vm_stack_end); + state->vm_stack_top = EG(vm_stack_top); + state->vm_stack_end = EG(vm_stack_end); state->vm_stack_page_size = EG(vm_stack_page_size); state->current_execute_data = EG(current_execute_data); state->error_reporting = EG(error_reporting); @@ -164,8 +164,8 @@ static zend_always_inline void zend_fiber_capture_vm_state(zend_fiber_vm_state * static zend_always_inline void zend_fiber_restore_vm_state(zend_fiber_vm_state *state) { EG(vm_stack) = state->vm_stack; - EG(vm_stack_top) = state->vm_stack->top; - EG(vm_stack_end) = state->vm_stack->end; + EG(vm_stack_top) = state->vm_stack_top; + EG(vm_stack_end) = state->vm_stack_end; EG(vm_stack_page_size) = state->vm_stack_page_size; EG(current_execute_data) = state->current_execute_data; EG(error_reporting) = state->error_reporting; diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index af4fb0950df34..df16958560b3a 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -436,7 +436,7 @@ static void fiber_enter_observer(zend_fiber_context *from, zend_fiber_context *t if (ZT_G(observer_fiber_switch)) { if (to->status == ZEND_FIBER_STATUS_INIT) { php_printf("\n", to); - } else if (to->status == ZEND_FIBER_STATUS_RUNNING && from->status == ZEND_FIBER_STATUS_RUNNING) { + } else if (from->status == ZEND_FIBER_STATUS_RUNNING && to->status == ZEND_FIBER_STATUS_SUSPENDED) { if (to->flags & ZEND_FIBER_FLAG_DESTROYED) { php_printf("\n", to); } else if (to->status != ZEND_FIBER_STATUS_DEAD) { @@ -449,7 +449,7 @@ static void fiber_enter_observer(zend_fiber_context *from, zend_fiber_context *t static void fiber_suspend_observer(zend_fiber_context *from, zend_fiber_context *to) { if (ZT_G(observer_fiber_switch)) { - if (from->status == ZEND_FIBER_STATUS_SUSPENDED) { + if (from->status == ZEND_FIBER_STATUS_RUNNING && to->status == ZEND_FIBER_STATUS_RUNNING) { php_printf("\n", from); } else if (from->status == ZEND_FIBER_STATUS_DEAD) { if (from->flags & ZEND_FIBER_FLAG_THREW) { From a3925562309a82562b3b7de1db1543c69f79c0af Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sat, 5 Jun 2021 23:03:37 -0500 Subject: [PATCH 2/9] Mark caller fiber as suspended --- Zend/tests/fibers/resume-previous-fiber.phpt | 28 +++++++++++++++++ Zend/zend_fibers.c | 33 +++++++++++--------- ext/zend_test/test.c | 4 +-- 3 files changed, 49 insertions(+), 16 deletions(-) create mode 100644 Zend/tests/fibers/resume-previous-fiber.phpt diff --git a/Zend/tests/fibers/resume-previous-fiber.phpt b/Zend/tests/fibers/resume-previous-fiber.phpt new file mode 100644 index 0000000000000..bcdc11ab139f5 --- /dev/null +++ b/Zend/tests/fibers/resume-previous-fiber.phpt @@ -0,0 +1,28 @@ +--TEST-- +Resume previous fiber +--FILE-- +resume(); + }); + + $fiber2->start(); +}); + +$fiber->start(); + +?> +--EXPECTF-- +Fatal error: Uncaught FiberError: Cannot resume a fiber that is not suspended in %sresume-previous-fiber.php:%d +Stack trace: +#0 %sresume-previous-fiber.php(%d): Fiber->resume() +#1 [internal function]: {closure}() +#2 %sresume-previous-fiber.php(%d): Fiber->start() +#3 [internal function]: {closure}() +#4 %sresume-previous-fiber.php(%d): Fiber->start() +#5 {main} + thrown in %sresume-previous-fiber.php on line %d diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index 573595fcfda1d..54b65e1e6530b 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -192,12 +192,12 @@ static ZEND_NORETURN void zend_fiber_trampoline(transfer_t transfer) __sanitizer_finish_switch_fiber(NULL, &from->stack.prior_pointer, &from->stack.prior_size); #endif - from = context->function(context); + zend_fiber_context *to = context->function(context); context->status = ZEND_FIBER_STATUS_DEAD; /* Final context switch, the fiber must not be resumed afterwards! */ - zend_fiber_switch_context(from); + zend_fiber_switch_context(to); /* Abort here because we are in an inconsistent program state. */ abort(); @@ -244,6 +244,14 @@ ZEND_API void zend_fiber_switch_context(zend_fiber_context *to) to->caller = from; } + if (from->status == ZEND_FIBER_STATUS_RUNNING) { + from->status = ZEND_FIBER_STATUS_SUSPENDED; + } + + if (from->caller == to) { + from->caller = NULL; + } + EG(current_fiber) = to; #ifdef __SANITIZE_ADDRESS__ @@ -260,22 +268,19 @@ ZEND_API void zend_fiber_switch_context(zend_fiber_context *to) __sanitizer_finish_switch_fiber(fake_stack, &to->stack.prior_pointer, &to->stack.prior_size); #endif - ((zend_fiber_context *) transfer.data)->handle = transfer.context; - EG(current_fiber) = from; - if (to->caller == from && to->status == ZEND_FIBER_STATUS_RUNNING) { - to->status = ZEND_FIBER_STATUS_SUSPENDED; - } - zend_fiber_restore_vm_state(&state); - if (UNEXPECTED(to->flags & ZEND_FIBER_FLAG_BAILOUT)) { + zend_fiber_context *previous = transfer.data; + previous->handle = transfer.context; + + if (UNEXPECTED(previous->flags & ZEND_FIBER_FLAG_BAILOUT)) { // zend_bailout() was called in the fiber, so call it again in the previous fiber or {main}. zend_bailout(); } - if (to->status == ZEND_FIBER_STATUS_DEAD) { + if (previous->status == ZEND_FIBER_STATUS_DEAD) { zend_fiber_destroy_context(to); } } @@ -530,7 +535,7 @@ ZEND_METHOD(Fiber, suspend) ZEND_API void zend_fiber_resume(zend_fiber *fiber, zval *value, zval *return_value) { - if (UNEXPECTED(fiber->status != ZEND_FIBER_STATUS_SUSPENDED)) { + if (UNEXPECTED(fiber->status != ZEND_FIBER_STATUS_SUSPENDED || fiber->caller != NULL)) { zend_throw_error(zend_ce_fiber_error, "Cannot resume a fiber that is not suspended"); RETURN_THROWS(); } @@ -570,7 +575,7 @@ ZEND_METHOD(Fiber, resume) ZEND_API void zend_fiber_throw(zend_fiber *fiber, zval *exception, zval *return_value) { - if (UNEXPECTED(fiber->status != ZEND_FIBER_STATUS_SUSPENDED)) { + if (UNEXPECTED(fiber->status != ZEND_FIBER_STATUS_SUSPENDED || fiber->caller != NULL)) { zend_throw_error(zend_ce_fiber_error, "Cannot resume a fiber that is not suspended"); RETURN_THROWS(); } @@ -623,7 +628,7 @@ ZEND_METHOD(Fiber, isSuspended) fiber = (zend_fiber *) Z_OBJ_P(getThis()); - RETURN_BOOL(fiber->status == ZEND_FIBER_STATUS_SUSPENDED); + RETURN_BOOL(fiber->status == ZEND_FIBER_STATUS_SUSPENDED && fiber->caller == NULL); } ZEND_METHOD(Fiber, isRunning) @@ -634,7 +639,7 @@ ZEND_METHOD(Fiber, isRunning) fiber = (zend_fiber *) Z_OBJ_P(getThis()); - RETURN_BOOL(fiber->status == ZEND_FIBER_STATUS_RUNNING); + RETURN_BOOL(fiber->status == ZEND_FIBER_STATUS_RUNNING || fiber->caller != NULL); } ZEND_METHOD(Fiber, isTerminated) diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index df16958560b3a..a3f11dd93df73 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -436,7 +436,7 @@ static void fiber_enter_observer(zend_fiber_context *from, zend_fiber_context *t if (ZT_G(observer_fiber_switch)) { if (to->status == ZEND_FIBER_STATUS_INIT) { php_printf("\n", to); - } else if (from->status == ZEND_FIBER_STATUS_RUNNING && to->status == ZEND_FIBER_STATUS_SUSPENDED) { + } else if (to->caller == NULL) { if (to->flags & ZEND_FIBER_FLAG_DESTROYED) { php_printf("\n", to); } else if (to->status != ZEND_FIBER_STATUS_DEAD) { @@ -449,7 +449,7 @@ static void fiber_enter_observer(zend_fiber_context *from, zend_fiber_context *t static void fiber_suspend_observer(zend_fiber_context *from, zend_fiber_context *to) { if (ZT_G(observer_fiber_switch)) { - if (from->status == ZEND_FIBER_STATUS_RUNNING && to->status == ZEND_FIBER_STATUS_RUNNING) { + if (from->caller == to && from->status == ZEND_FIBER_STATUS_RUNNING) { php_printf("\n", from); } else if (from->status == ZEND_FIBER_STATUS_DEAD) { if (from->flags & ZEND_FIBER_FLAG_THREW) { From 388d31bd63c892ebcf5e92939c7ccdc28b97033a Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sat, 5 Jun 2021 23:17:13 -0500 Subject: [PATCH 3/9] Return previous context from zend_fiber_switch_context --- Zend/zend_fibers.c | 4 +++- Zend/zend_fibers.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index 54b65e1e6530b..a668cc070a374 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -226,7 +226,7 @@ ZEND_API void zend_fiber_destroy_context(zend_fiber_context *context) zend_fiber_stack_free(&context->stack); } -ZEND_API void zend_fiber_switch_context(zend_fiber_context *to) +ZEND_API zend_fiber_context *zend_fiber_switch_context(zend_fiber_context *to) { zend_fiber_context *from = EG(current_fiber); zend_fiber_vm_state state; @@ -283,6 +283,8 @@ ZEND_API void zend_fiber_switch_context(zend_fiber_context *to) if (previous->status == ZEND_FIBER_STATUS_DEAD) { zend_fiber_destroy_context(to); } + + return previous; } diff --git a/Zend/zend_fibers.h b/Zend/zend_fibers.h index 767a85be1c651..1d7b68807ed67 100644 --- a/Zend/zend_fibers.h +++ b/Zend/zend_fibers.h @@ -133,7 +133,7 @@ ZEND_API void zend_fiber_throw(zend_fiber *fiber, zval *exception, zval *return_ /* These functions may be used to create custom fibers (coroutines) using the bundled fiber switching context. */ ZEND_API bool zend_fiber_init_context(zend_fiber_context *context, void *kind, zend_fiber_coroutine coroutine, size_t stack_size); ZEND_API void zend_fiber_destroy_context(zend_fiber_context *context); -ZEND_API void zend_fiber_switch_context(zend_fiber_context *to); +ZEND_API zend_fiber_context *zend_fiber_switch_context(zend_fiber_context *to); END_EXTERN_C() From 1a5da26c2b9ebe7564151fbe1a887a141c15465f Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sat, 5 Jun 2021 23:22:00 -0500 Subject: [PATCH 4/9] Drop zend_fiber API --- Zend/zend_fibers.c | 78 ++++++++++++++-------------------------------- Zend/zend_fibers.h | 9 +----- 2 files changed, 24 insertions(+), 63 deletions(-) diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index a668cc070a374..a022b46f56144 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -413,18 +413,6 @@ static void zend_fiber_object_free(zend_object *object) zend_object_std_dtor(&fiber->std); } -ZEND_API zend_fiber *zend_fiber_create(const zend_fcall_info *fci, const zend_fcall_info_cache *fci_cache) -{ - zend_fiber *fiber = (zend_fiber *) zend_fiber_object_create(zend_ce_fiber); - - fiber->fci = *fci; - fiber->fci_cache = *fci_cache; - - Z_TRY_ADDREF(fiber->fci.function_name); - - return fiber; -} - ZEND_METHOD(Fiber, __construct) { zend_fiber *fiber = (zend_fiber *) Z_OBJ_P(getThis()); @@ -437,9 +425,16 @@ ZEND_METHOD(Fiber, __construct) Z_TRY_ADDREF(fiber->fci.function_name); } -ZEND_API void zend_fiber_start(zend_fiber *fiber, zval *params, uint32_t param_count, zend_array *named_params, zval *return_value) +ZEND_METHOD(Fiber, start) { - zend_fiber_context *context = zend_fiber_get_context(fiber); + zend_fiber *fiber = (zend_fiber *) Z_OBJ_P(getThis()); + zval *params; + uint32_t param_count; + zend_array *named_params; + + ZEND_PARSE_PARAMETERS_START(0, -1) + Z_PARAM_VARIADIC_WITH_NAMED(params, param_count, named_params); + ZEND_PARSE_PARAMETERS_END(); if (fiber->status != ZEND_FIBER_STATUS_INIT) { zend_throw_error(zend_ce_fiber_error, "Cannot start a fiber that has already been started"); @@ -450,6 +445,8 @@ ZEND_API void zend_fiber_start(zend_fiber *fiber, zval *params, uint32_t param_c fiber->fci.param_count = param_count; fiber->fci.named_params = named_params; + zend_fiber_context *context = zend_fiber_get_context(fiber); + if (!zend_fiber_init_context(context, zend_ce_fiber, zend_fiber_execute, EG(fiber_stack_size))) { RETURN_THROWS(); } @@ -464,22 +461,15 @@ ZEND_API void zend_fiber_start(zend_fiber *fiber, zval *params, uint32_t param_c ZVAL_UNDEF(&fiber->value); } -ZEND_METHOD(Fiber, start) +ZEND_METHOD(Fiber, suspend) { - zend_fiber *fiber = (zend_fiber *) Z_OBJ_P(getThis()); - zval *params; - uint32_t param_count; - zend_array *named_params; + zval *value = NULL; - ZEND_PARSE_PARAMETERS_START(0, -1) - Z_PARAM_VARIADIC_WITH_NAMED(params, param_count, named_params); + ZEND_PARSE_PARAMETERS_START(0, 1) + Z_PARAM_OPTIONAL + Z_PARAM_ZVAL(value); ZEND_PARSE_PARAMETERS_END(); - zend_fiber_start(fiber, params, param_count, named_params, return_value); -} - -ZEND_API void zend_fiber_suspend(zval *value, zval *return_value) -{ if (UNEXPECTED(EG(current_fiber)->kind != zend_ce_fiber)) { zend_throw_error(zend_ce_fiber_error, "Cannot suspend outside of a fiber"); RETURN_THROWS(); @@ -523,8 +513,9 @@ ZEND_API void zend_fiber_suspend(zval *value, zval *return_value) ZVAL_UNDEF(&fiber->value); } -ZEND_METHOD(Fiber, suspend) +ZEND_METHOD(Fiber, resume) { + zend_fiber *fiber; zval *value = NULL; ZEND_PARSE_PARAMETERS_START(0, 1) @@ -532,11 +523,8 @@ ZEND_METHOD(Fiber, suspend) Z_PARAM_ZVAL(value); ZEND_PARSE_PARAMETERS_END(); - zend_fiber_suspend(value, return_value); -} + fiber = (zend_fiber *) Z_OBJ_P(getThis()); -ZEND_API void zend_fiber_resume(zend_fiber *fiber, zval *value, zval *return_value) -{ if (UNEXPECTED(fiber->status != ZEND_FIBER_STATUS_SUSPENDED || fiber->caller != NULL)) { zend_throw_error(zend_ce_fiber_error, "Cannot resume a fiber that is not suspended"); RETURN_THROWS(); @@ -560,23 +548,17 @@ ZEND_API void zend_fiber_resume(zend_fiber *fiber, zval *value, zval *return_val ZVAL_UNDEF(&fiber->value); } -ZEND_METHOD(Fiber, resume) +ZEND_METHOD(Fiber, throw) { zend_fiber *fiber; - zval *value = NULL; + zval *exception; - ZEND_PARSE_PARAMETERS_START(0, 1) - Z_PARAM_OPTIONAL - Z_PARAM_ZVAL(value); + ZEND_PARSE_PARAMETERS_START(1, 1) + Z_PARAM_OBJECT_OF_CLASS(exception, zend_ce_throwable) ZEND_PARSE_PARAMETERS_END(); fiber = (zend_fiber *) Z_OBJ_P(getThis()); - zend_fiber_resume(fiber, value, return_value); -} - -ZEND_API void zend_fiber_throw(zend_fiber *fiber, zval *exception, zval *return_value) -{ if (UNEXPECTED(fiber->status != ZEND_FIBER_STATUS_SUSPENDED || fiber->caller != NULL)) { zend_throw_error(zend_ce_fiber_error, "Cannot resume a fiber that is not suspended"); RETURN_THROWS(); @@ -597,20 +579,6 @@ ZEND_API void zend_fiber_throw(zend_fiber *fiber, zval *exception, zval *return_ ZVAL_UNDEF(&fiber->value); } -ZEND_METHOD(Fiber, throw) -{ - zend_fiber *fiber; - zval *exception; - - ZEND_PARSE_PARAMETERS_START(1, 1) - Z_PARAM_OBJECT_OF_CLASS(exception, zend_ce_throwable) - ZEND_PARSE_PARAMETERS_END(); - - fiber = (zend_fiber *) Z_OBJ_P(getThis()); - - zend_fiber_throw(fiber, exception, return_value); -} - ZEND_METHOD(Fiber, isStarted) { zend_fiber *fiber; diff --git a/Zend/zend_fibers.h b/Zend/zend_fibers.h index 1d7b68807ed67..4a011d9d7c33c 100644 --- a/Zend/zend_fibers.h +++ b/Zend/zend_fibers.h @@ -123,14 +123,7 @@ struct _zend_fiber { zval value; }; -/* These functions create and manipulate a Fiber object, allowing any internal function to start, resume, or suspend a fiber. */ -ZEND_API zend_fiber *zend_fiber_create(const zend_fcall_info *fci, const zend_fcall_info_cache *fci_cache); -ZEND_API void zend_fiber_start(zend_fiber *fiber, zval *params, uint32_t param_count, zend_array *named_params, zval *return_value); -ZEND_API void zend_fiber_suspend(zval *value, zval *return_value); -ZEND_API void zend_fiber_resume(zend_fiber *fiber, zval *value, zval *return_value); -ZEND_API void zend_fiber_throw(zend_fiber *fiber, zval *exception, zval *return_value); - -/* These functions may be used to create custom fibers (coroutines) using the bundled fiber switching context. */ +/* These functions may be used to create custom fiber objects using the bundled fiber switching context. */ ZEND_API bool zend_fiber_init_context(zend_fiber_context *context, void *kind, zend_fiber_coroutine coroutine, size_t stack_size); ZEND_API void zend_fiber_destroy_context(zend_fiber_context *context); ZEND_API zend_fiber_context *zend_fiber_switch_context(zend_fiber_context *to); From 5493fcc76bae32d3ed17de825b04e8e89cb4dc1b Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sat, 5 Jun 2021 23:32:24 -0500 Subject: [PATCH 5/9] Fix wrong pointer --- Zend/zend_fibers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index a022b46f56144..26b959d636f5e 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -281,7 +281,7 @@ ZEND_API zend_fiber_context *zend_fiber_switch_context(zend_fiber_context *to) } if (previous->status == ZEND_FIBER_STATUS_DEAD) { - zend_fiber_destroy_context(to); + zend_fiber_destroy_context(previous); } return previous; From 2319a5bca267e5fe5836555be68323c3087be204 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Schr=C3=B6der?= Date: Sun, 6 Jun 2021 10:39:42 +0200 Subject: [PATCH 6/9] Rearranged some code, added comment and simplified access to caller. --- Zend/zend_fibers.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index 26b959d636f5e..c23011cbe2edf 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -270,20 +270,21 @@ ZEND_API zend_fiber_context *zend_fiber_switch_context(zend_fiber_context *to) EG(current_fiber) = from; - zend_fiber_restore_vm_state(&state); - zend_fiber_context *previous = transfer.data; previous->handle = transfer.context; - if (UNEXPECTED(previous->flags & ZEND_FIBER_FLAG_BAILOUT)) { - // zend_bailout() was called in the fiber, so call it again in the previous fiber or {main}. - zend_bailout(); - } + zend_fiber_restore_vm_state(&state); + /* Destroy context first to ensure it does not leak if some extension does custom bailout handling. */ if (previous->status == ZEND_FIBER_STATUS_DEAD) { zend_fiber_destroy_context(previous); } + /* Propagate bailout to current fiber / main. */ + if (UNEXPECTED(previous->flags & ZEND_FIBER_FLAG_BAILOUT)) { + zend_bailout(); + } + return previous; } @@ -493,7 +494,7 @@ ZEND_METHOD(Fiber, suspend) fiber->execute_data = EG(current_execute_data); fiber->stack_bottom->prev_execute_data = NULL; - zend_fiber_switch_context(zend_fiber_get_context(fiber)->caller); + zend_fiber_switch_context(fiber->caller); if (fiber->flags & ZEND_FIBER_FLAG_DESTROYED) { // This occurs when the fiber is GC'ed while suspended. From 835279a579b2cadc68e2f8f3aa20389b9f220436 Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sat, 5 Jun 2021 23:43:33 -0500 Subject: [PATCH 7/9] Style fix --- Zend/zend_fibers.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Zend/zend_fibers.h b/Zend/zend_fibers.h index 4a011d9d7c33c..5f5c7d44a9d44 100644 --- a/Zend/zend_fibers.h +++ b/Zend/zend_fibers.h @@ -90,8 +90,8 @@ struct _zend_fiber_context { /* Zend VM state that needs to be captured / restored during fiber context switch. */ typedef struct _zend_fiber_vm_state { zend_vm_stack vm_stack; - zval* vm_stack_top; - zval* vm_stack_end; + zval *vm_stack_top; + zval *vm_stack_end; size_t vm_stack_page_size; zend_execute_data *current_execute_data; int error_reporting; From 681f29ebee047b8e75e7c39193d9e346c10e694f Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Sun, 6 Jun 2021 11:52:11 -0500 Subject: [PATCH 8/9] Move caller back to zend_fiber Caller is specific to treating fibers as a stack, so keep it with that implementation. If an implementation jumped between fibers, caller can be misleading or inaccurate. --- Zend/zend_fibers.c | 23 +++++++++++++---------- Zend/zend_fibers.h | 4 +++- ext/zend_test/test.c | 16 ++++++++++++---- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index c23011cbe2edf..556d589dc37f6 100644 --- a/Zend/zend_fibers.c +++ b/Zend/zend_fibers.c @@ -240,18 +240,11 @@ ZEND_API zend_fiber_context *zend_fiber_switch_context(zend_fiber_context *to) zend_fiber_capture_vm_state(&state); to->status = ZEND_FIBER_STATUS_RUNNING; - if (to->caller == NULL) { - to->caller = from; - } if (from->status == ZEND_FIBER_STATUS_RUNNING) { from->status = ZEND_FIBER_STATUS_SUSPENDED; } - if (from->caller == to) { - from->caller = NULL; - } - EG(current_fiber) = to; #ifdef __SANITIZE_ADDRESS__ @@ -353,7 +346,10 @@ static ZEND_STACK_ALIGNED zend_fiber_context *zend_fiber_execute(zend_fiber_cont fiber->execute_data = NULL; fiber->stack_bottom = NULL; - return fiber->caller; + zend_fiber_context *caller = fiber->caller; + fiber->caller = NULL; + + return caller; } static zend_object *zend_fiber_object_create(zend_class_entry *ce) @@ -380,6 +376,7 @@ static void zend_fiber_object_destroy(zend_object *object) zend_object *exception = EG(exception); EG(exception) = NULL; + fiber->caller = EG(current_fiber); fiber->flags |= ZEND_FIBER_FLAG_DESTROYED; zend_fiber_switch_context(zend_fiber_get_context(fiber)); @@ -452,6 +449,8 @@ ZEND_METHOD(Fiber, start) RETURN_THROWS(); } + fiber->caller = EG(current_fiber); + zend_fiber_switch_context(context); if (fiber->status == ZEND_FIBER_STATUS_DEAD) { @@ -477,6 +476,7 @@ ZEND_METHOD(Fiber, suspend) } zend_fiber *fiber = zend_fiber_from_context(EG(current_fiber)); + zend_fiber_context *caller = fiber->caller; if (UNEXPECTED(fiber->flags & ZEND_FIBER_FLAG_DESTROYED)) { zend_throw_error(zend_ce_fiber_error, "Cannot suspend in a force-closed fiber"); @@ -484,6 +484,7 @@ ZEND_METHOD(Fiber, suspend) } ZEND_ASSERT(fiber->status == ZEND_FIBER_STATUS_RUNNING); + ZEND_ASSERT(caller != NULL); if (value) { ZVAL_COPY(&fiber->value, value); @@ -491,10 +492,11 @@ ZEND_METHOD(Fiber, suspend) ZVAL_NULL(&fiber->value); } + fiber->caller = NULL; fiber->execute_data = EG(current_execute_data); fiber->stack_bottom->prev_execute_data = NULL; - zend_fiber_switch_context(fiber->caller); + zend_fiber_switch_context(caller); if (fiber->flags & ZEND_FIBER_FLAG_DESTROYED) { // This occurs when the fiber is GC'ed while suspended. @@ -537,6 +539,7 @@ ZEND_METHOD(Fiber, resume) ZVAL_NULL(&fiber->value); } + fiber->caller = EG(current_fiber); fiber->stack_bottom->prev_execute_data = EG(current_execute_data); zend_fiber_switch_context(zend_fiber_get_context(fiber)); @@ -568,6 +571,7 @@ ZEND_METHOD(Fiber, throw) Z_ADDREF_P(exception); fiber->exception = exception; + fiber->caller = EG(current_fiber); fiber->stack_bottom->prev_execute_data = EG(current_execute_data); zend_fiber_switch_context(zend_fiber_get_context(fiber)); @@ -693,7 +697,6 @@ void zend_fiber_init(void) zend_fiber_context *context = ecalloc(1, sizeof(zend_fiber_context)); context->status = ZEND_FIBER_STATUS_RUNNING; - context->caller = (zend_fiber_context *) -1; EG(main_fiber) = context; EG(current_fiber) = context; diff --git a/Zend/zend_fibers.h b/Zend/zend_fibers.h index 5f5c7d44a9d44..940f03c6d3847 100644 --- a/Zend/zend_fibers.h +++ b/Zend/zend_fibers.h @@ -76,7 +76,6 @@ typedef zend_fiber_context *(*zend_fiber_coroutine)(zend_fiber_context *context) void *handle; \ /* Pointer that identifies the fiber type. */ \ void *kind; \ - zend_fiber_context *caller; \ zend_fiber_coroutine function; \ zend_fiber_stack stack; \ zend_fiber_status status; \ @@ -106,6 +105,9 @@ struct _zend_fiber { /* Fiber context fields (embedded to avoid memory allocation). */ ZEND_FIBER_CONTEXT_FIELDS; + /* Fiber that resumed us. */ + zend_fiber_context *caller; + /* Callback and info / cache to be used when fiber is started. */ zend_fcall_info fci; zend_fcall_info_cache fci_cache; diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index a3f11dd93df73..985727aa53abf 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -436,7 +436,12 @@ static void fiber_enter_observer(zend_fiber_context *from, zend_fiber_context *t if (ZT_G(observer_fiber_switch)) { if (to->status == ZEND_FIBER_STATUS_INIT) { php_printf("\n", to); - } else if (to->caller == NULL) { + } else if (to != EG(main_fiber)) { + zend_fiber *fiber = zend_fiber_from_context(to); + if (fiber->caller != from) { + return; + } + if (to->flags & ZEND_FIBER_FLAG_DESTROYED) { php_printf("\n", to); } else if (to->status != ZEND_FIBER_STATUS_DEAD) { @@ -449,9 +454,7 @@ static void fiber_enter_observer(zend_fiber_context *from, zend_fiber_context *t static void fiber_suspend_observer(zend_fiber_context *from, zend_fiber_context *to) { if (ZT_G(observer_fiber_switch)) { - if (from->caller == to && from->status == ZEND_FIBER_STATUS_RUNNING) { - php_printf("\n", from); - } else if (from->status == ZEND_FIBER_STATUS_DEAD) { + if (from->status == ZEND_FIBER_STATUS_DEAD) { if (from->flags & ZEND_FIBER_FLAG_THREW) { php_printf("\n", from); } else if (from->flags & ZEND_FIBER_FLAG_DESTROYED) { @@ -459,6 +462,11 @@ static void fiber_suspend_observer(zend_fiber_context *from, zend_fiber_context } else { php_printf("\n", from); } + } else if (from != EG(main_fiber)) { + zend_fiber *fiber = zend_fiber_from_context(from); + if (fiber->caller == NULL) { + php_printf("\n", from); + } } } } From f44b7cf5bef7c37a2015068ac613807ca468684e Mon Sep 17 00:00:00 2001 From: Aaron Piotrowski Date: Mon, 7 Jun 2021 16:16:33 -0500 Subject: [PATCH 9/9] Use kind in zend_test observer --- ext/zend_test/test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 985727aa53abf..c4c42221c0701 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -436,7 +436,7 @@ static void fiber_enter_observer(zend_fiber_context *from, zend_fiber_context *t if (ZT_G(observer_fiber_switch)) { if (to->status == ZEND_FIBER_STATUS_INIT) { php_printf("\n", to); - } else if (to != EG(main_fiber)) { + } else if (to->kind == zend_ce_fiber) { zend_fiber *fiber = zend_fiber_from_context(to); if (fiber->caller != from) { return; @@ -462,7 +462,7 @@ static void fiber_suspend_observer(zend_fiber_context *from, zend_fiber_context } else { php_printf("\n", from); } - } else if (from != EG(main_fiber)) { + } else if (from->kind == zend_ce_fiber) { zend_fiber *fiber = zend_fiber_from_context(from); if (fiber->caller == NULL) { php_printf("\n", from);