Skip to content

Split fiber status and flags #7094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 3, 2021
Merged

Split fiber status and flags #7094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions Zend/zend_fibers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ static void zend_fiber_switch_to(zend_fiber *fiber)

zend_observer_fiber_switch_notify(fiber, previous);

if (UNEXPECTED(fiber->status == ZEND_FIBER_STATUS_BAILOUT)) {
if (UNEXPECTED(fiber->flags & ZEND_FIBER_FLAG_BAILOUT)) {
// zend_bailout() was called in the fiber, so call it again in the previous fiber or {main}.
zend_bailout();
}
Expand Down Expand Up @@ -379,20 +379,20 @@ static void ZEND_STACK_ALIGNED zend_fiber_execute(zend_fiber_context *context)
zval_ptr_dtor(&fiber->fci.function_name);

if (EG(exception)) {
if (fiber->status == ZEND_FIBER_STATUS_SHUTDOWN) {
if (fiber->flags & ZEND_FIBER_FLAG_DESTROYED) {
if (EXPECTED(zend_is_graceful_exit(EG(exception)) || zend_is_unwind_exit(EG(exception)))) {
zend_clear_exception();
}
} else {
fiber->status = ZEND_FIBER_STATUS_THREW;
fiber->flags = ZEND_FIBER_FLAG_THREW;
}
} else {
fiber->status = ZEND_FIBER_STATUS_RETURNED;
}
} zend_catch {
fiber->status = ZEND_FIBER_STATUS_BAILOUT;
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;
Expand Down Expand Up @@ -422,7 +422,8 @@ static void zend_fiber_object_destroy(zend_object *object)
zend_object *exception = EG(exception);
EG(exception) = NULL;

fiber->status = ZEND_FIBER_STATUS_SHUTDOWN;
fiber->status = ZEND_FIBER_STATUS_RUNNING;
fiber->flags |= ZEND_FIBER_FLAG_DESTROYED;

zend_fiber_switch_to(fiber);

Expand Down Expand Up @@ -499,7 +500,7 @@ ZEND_API void zend_fiber_start(zend_fiber *fiber, zval *params, uint32_t param_c

zend_fiber_switch_to(fiber);

if (fiber->status & ZEND_FIBER_STATUS_FINISHED) {
if (fiber->status == ZEND_FIBER_STATUS_DEAD) {
RETURN_NULL();
}

Expand Down Expand Up @@ -530,7 +531,7 @@ ZEND_API void zend_fiber_suspend(zval *value, zval *return_value)
RETURN_THROWS();
}

if (UNEXPECTED(fiber->status == ZEND_FIBER_STATUS_SHUTDOWN)) {
if (UNEXPECTED(fiber->flags & ZEND_FIBER_FLAG_DESTROYED)) {
zend_throw_error(zend_ce_fiber_error, "Cannot suspend in a force-closed fiber");
RETURN_THROWS();
}
Expand All @@ -549,7 +550,7 @@ ZEND_API void zend_fiber_suspend(zval *value, zval *return_value)

zend_fiber_suspend_from(fiber);

if (fiber->status == ZEND_FIBER_STATUS_SHUTDOWN) {
if (fiber->flags & ZEND_FIBER_FLAG_DESTROYED) {
// This occurs when the fiber is GC'ed while suspended.
zend_throw_graceful_exit();
RETURN_THROWS();
Expand Down Expand Up @@ -599,7 +600,7 @@ ZEND_API void zend_fiber_resume(zend_fiber *fiber, zval *value, zval *return_val

zend_fiber_switch_to(fiber);

if (fiber->status & ZEND_FIBER_STATUS_FINISHED) {
if (fiber->status == ZEND_FIBER_STATUS_DEAD) {
RETURN_NULL();
}

Expand Down Expand Up @@ -637,7 +638,7 @@ ZEND_API void zend_fiber_throw(zend_fiber *fiber, zval *exception, zval *return_

zend_fiber_switch_to(fiber);

if (fiber->status & ZEND_FIBER_STATUS_FINISHED) {
if (fiber->status == ZEND_FIBER_STATUS_DEAD) {
RETURN_NULL();
}

Expand Down Expand Up @@ -700,7 +701,7 @@ ZEND_METHOD(Fiber, isTerminated)

fiber = (zend_fiber *) Z_OBJ_P(getThis());

RETURN_BOOL(fiber->status & ZEND_FIBER_STATUS_FINISHED);
RETURN_BOOL(fiber->status == ZEND_FIBER_STATUS_DEAD);
}

ZEND_METHOD(Fiber, getReturn)
Expand All @@ -711,13 +712,15 @@ ZEND_METHOD(Fiber, getReturn)

fiber = (zend_fiber *) Z_OBJ_P(getThis());

if (fiber->status != ZEND_FIBER_STATUS_RETURNED) {
if (fiber->status != ZEND_FIBER_STATUS_DEAD || fiber->flags) {
Copy link
Member

@twose twose Jun 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can only get the return value after the fiber is dead, so the || fiber->flags here seem unnecessary, and it assumes that the flags are always used to indicate the error state, and we may add more other flags in the future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A fiber can be dead but have not returned a value (i.e., threw an exception). If flags are used for something other than error conditions then we'll have to revisit this, but it's fine for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean

if (fiber->status == ZEND_FIBER_STATUS_INIT) {
    ...
} else if (fiber->status == ZEND_FIBER_STATUS_DEAD) {
   if (fiber->flags ...) { ... }
} else {
   ...
}

It looks like you're being lazy.

const char *message;

if (fiber->status == ZEND_FIBER_STATUS_INIT) {
message = "The fiber has not been started";
} else if (fiber->status == ZEND_FIBER_STATUS_THREW) {
} else if (fiber->flags & ZEND_FIBER_FLAG_THREW) {
message = "The fiber threw an exception";
} else if (fiber->flags & ZEND_FIBER_FLAG_BAILOUT) {
message = "The fiber exited with a fatal error";
} else {
message = "The fiber has not returned";
}
Expand Down
30 changes: 18 additions & 12 deletions Zend/zend_fibers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@

BEGIN_EXTERN_C()

typedef enum {
ZEND_FIBER_STATUS_INIT,
ZEND_FIBER_STATUS_RUNNING,
ZEND_FIBER_STATUS_SUSPENDED,
ZEND_FIBER_STATUS_DEAD,
} zend_fiber_status;

typedef enum {
ZEND_FIBER_FLAG_THREW = 1 << 0,
ZEND_FIBER_FLAG_BAILOUT = 1 << 1,
ZEND_FIBER_FLAG_DESTROYED = 1 << 2,
} zend_fiber_flag;

void zend_register_fiber_ce(void);
void zend_fiber_init(void);

Expand Down Expand Up @@ -59,8 +72,11 @@ typedef struct _zend_fiber {
/* Fiber PHP object handle. */
zend_object std;

/* Status of the fiber, one of the ZEND_FIBER_STATUS_* constants. */
zend_uchar status;
/* Status of the fiber, one of the zend_fiber_status values. */
zend_fiber_status status;

/* Flags of the fiber, bit field of the zend_fiber_flag values. */
zend_uchar flags;

/* Callback and info / cache to be used when fiber is started. */
zend_fcall_info fci;
Expand All @@ -82,16 +98,6 @@ typedef struct _zend_fiber {
zval value;
} zend_fiber;

static const zend_uchar ZEND_FIBER_STATUS_INIT = 0x0;
static const zend_uchar ZEND_FIBER_STATUS_SUSPENDED = 0x1;
static const zend_uchar ZEND_FIBER_STATUS_RUNNING = 0x2;
static const zend_uchar ZEND_FIBER_STATUS_RETURNED = 0x4;
static const zend_uchar ZEND_FIBER_STATUS_THREW = 0x8;
static const zend_uchar ZEND_FIBER_STATUS_SHUTDOWN = 0x10;
static const zend_uchar ZEND_FIBER_STATUS_BAILOUT = 0x20;

static const zend_uchar ZEND_FIBER_STATUS_FINISHED = 0x2c;

/* 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);
Expand Down
4 changes: 2 additions & 2 deletions ext/reflection/php_reflection.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6873,7 +6873,7 @@ ZEND_METHOD(ReflectionFiber, getFiber)
}

#define REFLECTION_CHECK_VALID_FIBER(fiber) do { \
if (fiber == NULL || fiber->status == ZEND_FIBER_STATUS_INIT || fiber->status & ZEND_FIBER_STATUS_FINISHED) { \
if (fiber == NULL || fiber->status == ZEND_FIBER_STATUS_INIT || fiber->status == ZEND_FIBER_STATUS_DEAD) { \
zend_throw_error(NULL, "Cannot fetch information from a fiber that has not been started or is terminated"); \
RETURN_THROWS(); \
} \
Expand Down Expand Up @@ -6948,7 +6948,7 @@ ZEND_METHOD(ReflectionFiber, getCallable)

ZEND_PARSE_PARAMETERS_NONE();

if (fiber == NULL || fiber->status & ZEND_FIBER_STATUS_FINISHED) {
if (fiber == NULL || fiber->status == ZEND_FIBER_STATUS_DEAD) {
zend_throw_error(NULL, "Cannot fetch the callable from a fiber that has terminated"); \
RETURN_THROWS();
}
Expand Down
22 changes: 13 additions & 9 deletions ext/zend_test/test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,9 +425,11 @@ static void fiber_enter_observer(zend_fiber *from, zend_fiber *to)
if (to->status == ZEND_FIBER_STATUS_INIT) {
php_printf("<init '%p'>\n", to);
} else if (to->status == ZEND_FIBER_STATUS_RUNNING && (!from || from->status == ZEND_FIBER_STATUS_RUNNING)) {
php_printf("<resume '%p'>\n", to);
} else if (to->status == ZEND_FIBER_STATUS_SHUTDOWN) {
php_printf("<destroying '%p'>\n", to);
if (to->flags & ZEND_FIBER_FLAG_DESTROYED) {
php_printf("<destroying '%p'>\n", to);
} else if (to->status != ZEND_FIBER_STATUS_DEAD) {
php_printf("<resume '%p'>\n", to);
}
}
}
}
Expand All @@ -439,12 +441,14 @@ static void fiber_suspend_observer(zend_fiber *from, zend_fiber *to)
if (from) {
if (from->status == ZEND_FIBER_STATUS_SUSPENDED) {
php_printf("<suspend '%p'>\n", from);
} else if (from->status == ZEND_FIBER_STATUS_RETURNED) {
php_printf("<returned '%p'>\n", from);
} else if (from->status == ZEND_FIBER_STATUS_THREW) {
php_printf("<threw '%p'>\n", from);
} else if (from->status == ZEND_FIBER_STATUS_SHUTDOWN) {
php_printf("<destroyed '%p'>\n", from);
} else if (from->status == ZEND_FIBER_STATUS_DEAD) {
if (from->flags & ZEND_FIBER_FLAG_THREW) {
php_printf("<threw '%p'>\n", from);
} else if (from->flags & ZEND_FIBER_FLAG_DESTROYED) {
php_printf("<destroyed '%p'>\n", from);
} else {
php_printf("<returned '%p'>\n", from);
}
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions ext/zend_test/tests/observer_fiber_05.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@ $fiber->resume();
<destroying '%s'>
<!-- switching from fiber %s to %s -->
<destroying '%s'>
<destroyed '%s'>
<!-- switching from fiber %s to %s -->
<destroying '%s'>
<destroyed '%s'>
<!-- switching from fiber %s to 0 -->
<destroyed '%s'>