feat: prevent calling root fiber and refactor fiber state tracking

Add explicit check in `runFiber()` to reject calls to the root fiber, preventing VM crashes when a completed fiber's caller tries to resume it. Replace the boolean `callerIsTrying` field with a `FiberState` enum (`FIBER_TRY`, `FIBER_ROOT`, `FIBER_OTHER`) to track fiber invocation mode, updating all relevant primitives and initialization. Include new test cases (`call_root.wren`, `call_wren_call_root.*`) and re-entrancy documentation notes.
This commit is contained in:
Bob Nystrom
2018-07-21 17:02:29 +00:00
parent daeff98b83
commit 4b160f017b
11 changed files with 240 additions and 12 deletions
+9 -3
View File
@@ -88,8 +88,14 @@ static bool runFiber(WrenVM* vm, ObjFiber* fiber, Value* args, bool isCall,
if (isCall)
{
// You can't call a called fiber, but you can transfer directly to it,
// which is why this check is gated on `isCall`. This way, after resuming a
// suspended fiber, it will run and then return to the fiber that called it
// and so on.
if (fiber->caller != NULL) RETURN_ERROR("Fiber has already been called.");
if (fiber->state == FIBER_ROOT) RETURN_ERROR("Cannot call root fiber.");
// Remember who ran it.
fiber->caller = vm->fiber;
}
@@ -181,7 +187,7 @@ DEF_PRIMITIVE(fiber_try)
runFiber(vm, AS_FIBER(args[0]), args, true, false, "try");
// If we're switching to a valid fiber to try, remember that we're trying it.
if (IS_NULL(vm->fiber->error)) vm->fiber->callerIsTrying = true;
if (IS_NULL(vm->fiber->error)) vm->fiber->state = FIBER_TRY;
return false;
}
@@ -192,7 +198,7 @@ DEF_PRIMITIVE(fiber_yield)
// Unhook this fiber from the one that called it.
current->caller = NULL;
current->callerIsTrying = false;
current->state = FIBER_OTHER;
if (vm->fiber != NULL)
{
@@ -210,7 +216,7 @@ DEF_PRIMITIVE(fiber_yield1)
// Unhook this fiber from the one that called it.
current->caller = NULL;
current->callerIsTrying = false;
current->state = FIBER_OTHER;
if (vm->fiber != NULL)
{
+1 -1
View File
@@ -171,7 +171,7 @@ ObjFiber* wrenNewFiber(WrenVM* vm, ObjClosure* closure)
fiber->openUpvalues = NULL;
fiber->caller = NULL;
fiber->error = NULL_VAL;
fiber->callerIsTrying = false;
fiber->state = FIBER_OTHER;
if (closure != NULL)
{
+19 -4
View File
@@ -293,6 +293,24 @@ typedef struct
Value* stackStart;
} CallFrame;
// Tracks how this fiber has been invoked, aside from the ways that can be
// detected from the state of other fields in the fiber.
typedef enum
{
// The fiber is being run from another fiber using a call to `try()`.
FIBER_TRY,
// The fiber was directly invoked by `runInterpreter()`. This means it's the
// initial fiber used by a call to `wrenCall()` or `wrenInterpret()`.
FIBER_ROOT,
// The fiber is invoked some other way. If [caller] is `NULL` then the fiber
// was invoked using `call()`. If [numFrames] is zero, then the fiber has
// finished running and is done. If [numFrames] is one and that frame's `ip`
// points to the first byte of code, the fiber has not been started yet.
FIBER_OTHER,
} FiberState;
typedef struct sObjFiber
{
Obj obj;
@@ -331,10 +349,7 @@ typedef struct sObjFiber
// error object. Otherwise, it will be null.
Value error;
// This will be true if the caller that called this fiber did so using "try".
// In that case, if this fiber fails with an error, the error will be given
// to the caller.
bool callerIsTrying;
FiberState state;
} ObjFiber;
typedef enum
+3 -3
View File
@@ -399,7 +399,7 @@ static void runtimeError(WrenVM* vm)
current->error = error;
// If the caller ran this fiber using "try", give it the error and stop.
if (current->callerIsTrying)
if (current->state == FIBER_TRY)
{
// Make the caller's try method return the error message.
current->caller->stackTop[-1] = vm->fiber->error;
@@ -768,12 +768,12 @@ static Value getModuleVariable(WrenVM* vm, ObjModule* module,
}
// The main bytecode interpreter loop. This is where the magic happens. It is
// also, as you can imagine, highly performance critical. Returns `true` if the
// fiber completed without error.
// also, as you can imagine, highly performance critical.
static WrenInterpretResult runInterpreter(WrenVM* vm, register ObjFiber* fiber)
{
// Remember the current fiber so we can find it if a GC happens.
vm->fiber = fiber;
fiber->state = FIBER_ROOT;
// Hoist these into local variables. They are accessed frequently in the loop
// but assigned less frequently. Keeping them in locals and updating them when