mirror of
https://github.com/mruby/mruby
synced 2026-06-08 16:11:16 +00:00
Fix execute_task() so unhandled task exceptions become task results
This patch fixes a bug thatc09196cintroduced. ## Background `mrb_task_run()` has two usage patterns: 1. Called directly from `main()` as the top-level scheduler (PicoRuby and R2P2). There is no surrounding C exception handler, so mrb->jmp is NULL on entry 2. Called from Ruby code via Task.run, bootstrapped on top of mruby's regular call chain. mrb->jmp is non-NULL Historically, an unhandled exception raised inside a task body was turned into the task's result value by `mrb_vm_exec()`: the L_RAISE path walked callinfo down to cibase, ran `fiber_terminate()`, and - because c->vmexec was TRUE and prev_jmp was NULL in pattern 1 - took `return mrb_obj_value(mrb->exc)`. That value landed in t->result and could be read back through `mrb_task_value()` / `join()`. ## Whatc09196cbroke It consider only pattern 2 and wrapped `mrb_task_run()` in a protect frame (MRB_TRY / mrb_protect_error) to guarantee that loop_running is cleared on exception. As a side effect, mrb->jmp is now always non-NULL while a task body is executing, so the L_RAISE path takes `MRB_THROW(prev_jmp)` instead of returning the exception value. In pattern 2 this merely changed the semantics (exceptions started propagating out of `Task.run` instead of being stored as task results). In pattern 1 it was FATAL: the throw unwound to mrb_task_run's catch handler, which called `mrb_exc_raise()` to re-propagate, and with no outer jmpbuf this aborted the process. PicoRuby/R2P2 could no longer retrieve task exceptions via `mrb_task_value()`. ## Fix Restore the "task exception becomes task result" contract uniformly for both patterns, independent of mrb->jmp: * Add `mrb_task_state.exception_as_result`. When set, `mrb_vm_exec()`'s non-root_c L_RAISE branch returns the exception as a value even if prev_jmp is non-NULL, instead of throwing * `execute_task_vm()` raises the flag around `mrb_vm_exec()`, captures the exception into `t->result`, and clears `mrb->exc` * Wrap `execute_task_vm()` in `mrb_protect_error()` as a safety net for rare paths that still unwind via MRB_THROW (e.g. CINFO_SKIP frames). exception_as_result is reset both at the end of the body and immediately after `mrb_protect_error()` returns, so a caught throw does not leave the llag set * Expose `Task#value` to retrieve t->result from Ruby, since Task#join cannot deliver the value through its return path under cooperative scheduling * Add a test asserting that `Task#join` on a task that raised returns the exception object, matching the pre-c09196c observable behavior ## Notes The "task exception becomes task result" semantics match the mruby/c's rrt0.c and the spirit of CRuby's Thread (an unhandled exception in a thread does not kill the scheduler / process; it surfaces when the thread is joined). The visible API shape still differs from CRuby - `Task#join` here returns the exception object rather than re-raising it - but the scheduler is no longer destabilized by task errors in either invocation pattern.
This commit is contained in:
@@ -275,6 +275,7 @@ typedef struct mrb_task_state {
|
||||
struct mrb_task *main_task; /* Main task wrapper for root context */
|
||||
uint8_t scheduler_lock; /* Lock counter for synchronous execution */
|
||||
mrb_bool loop_running; /* Active mrb_task_run loop flag */
|
||||
mrb_bool exception_as_result; /* Return unhandled task exceptions as values */
|
||||
} mrb_task_state;
|
||||
#endif
|
||||
|
||||
|
||||
@@ -314,6 +314,27 @@ task_change_state(mrb_state *mrb, mrb_task *t, uint8_t new_status)
|
||||
mrb_task_enable_irq();
|
||||
}
|
||||
|
||||
typedef struct execute_task_vm_args {
|
||||
mrb_task *t;
|
||||
const struct RProc *proc;
|
||||
const mrb_code *pc;
|
||||
} execute_task_vm_args;
|
||||
|
||||
static mrb_value
|
||||
execute_task_vm(mrb_state *mrb, void *ud)
|
||||
{
|
||||
execute_task_vm_args *args = (execute_task_vm_args*)ud;
|
||||
|
||||
mrb->task.exception_as_result = TRUE;
|
||||
args->t->result = mrb_vm_exec(mrb, args->proc, args->pc);
|
||||
if (mrb->exc) {
|
||||
args->t->result = mrb_obj_value(mrb->exc);
|
||||
mrb->exc = NULL;
|
||||
}
|
||||
mrb->task.exception_as_result = FALSE;
|
||||
return args->t->result;
|
||||
}
|
||||
|
||||
/* Execute a single task - core task execution logic */
|
||||
static void
|
||||
execute_task(mrb_state *mrb, mrb_task *t)
|
||||
@@ -348,8 +369,13 @@ execute_task(mrb_state *mrb, mrb_task *t)
|
||||
/* Set vmexec flag to prevent fiber_terminate from being called */
|
||||
t->c.vmexec = TRUE;
|
||||
|
||||
/* Execute task - PC is saved in ci->pc from previous run */
|
||||
t->result = mrb_vm_exec(mrb, proc, pc);
|
||||
/* Execute task - PC is saved in ci->pc from previous run.
|
||||
Unhandled task exceptions are converted to the task result by
|
||||
mrb_vm_exec() in task mode, so the scheduler protect frame stays intact. */
|
||||
execute_task_vm_args args = { t, proc, pc };
|
||||
mrb_bool error = FALSE;
|
||||
t->result = mrb_protect_error(mrb, execute_task_vm, &args, &error);
|
||||
mrb->task.exception_as_result = FALSE;
|
||||
|
||||
/* Clear vmexec flag */
|
||||
t->c.vmexec = FALSE;
|
||||
@@ -379,6 +405,15 @@ execute_task(mrb_state *mrb, mrb_task *t)
|
||||
/* Task yielded but still running - move to ready queue */
|
||||
t->status = MRB_TASK_STATUS_READY;
|
||||
}
|
||||
|
||||
/* Fallback for abnormal cases that bypass exception_as_result:
|
||||
e.g. a CINFO_SKIP frame or some other path inside mrb_vm_exec()
|
||||
unwound via MRB_THROW instead of returning the exception as a
|
||||
value. Normal unhandled task exceptions never reach this branch;
|
||||
they are captured into t->result by execute_task_vm() above. */
|
||||
if (error) {
|
||||
mrb_exc_raise(mrb, t->result);
|
||||
}
|
||||
}
|
||||
|
||||
/* Tick handler - called by timer interrupt */
|
||||
@@ -1524,6 +1559,7 @@ mrb_mruby_task_gem_init(mrb_state *mrb)
|
||||
mrb->task.main_task = NULL;
|
||||
mrb->task.scheduler_lock = 0;
|
||||
mrb->task.loop_running = FALSE;
|
||||
mrb->task.exception_as_result = FALSE;
|
||||
|
||||
task_class = mrb_define_class_id(mrb, MRB_SYM(Task), mrb->object_class);
|
||||
MRB_SET_INSTANCE_TT(task_class, MRB_TT_DATA);
|
||||
@@ -1555,6 +1591,7 @@ mrb_mruby_task_gem_init(mrb_state *mrb)
|
||||
mrb_define_method_id(mrb, task_class, MRB_SYM(resume), mrb_task_resume, MRB_ARGS_NONE());
|
||||
mrb_define_method_id(mrb, task_class, MRB_SYM(terminate), mrb_task_terminate, MRB_ARGS_NONE());
|
||||
mrb_define_method_id(mrb, task_class, MRB_SYM(join), mrb_task_join, MRB_ARGS_NONE());
|
||||
mrb_define_method_id(mrb, task_class, MRB_SYM(value), mrb_task_value, MRB_ARGS_NONE());
|
||||
|
||||
/* Kernel methods (module functions like CRuby)
|
||||
* Note: sleep and usleep override mruby-sleep's implementation to be task-aware
|
||||
|
||||
@@ -194,3 +194,17 @@ assert("Task.run inside Task.run is a noop") do
|
||||
Task.run
|
||||
end
|
||||
end
|
||||
|
||||
assert("Task#value returns exception object for unhandled task errors") do
|
||||
child = nil
|
||||
|
||||
Task.new do
|
||||
child = Task.new { raise "boom" }
|
||||
end
|
||||
|
||||
Task.run
|
||||
|
||||
result = child.value
|
||||
assert_kind_of RuntimeError, result
|
||||
assert_equal "boom", result.message
|
||||
end
|
||||
|
||||
@@ -1635,9 +1635,11 @@ task_across_c_boundary(mrb_state *mrb)
|
||||
if (mrb->c->status != MRB_TASK_STOPPED) \
|
||||
mrb->c->status = MRB_TASK_STOPPED; \
|
||||
} while (0)
|
||||
#define TASK_RETURN_EXCEPTION_AS_VALUE(mrb) ((mrb)->task.exception_as_result)
|
||||
#else
|
||||
#define RETURN_IF_TASK_STOPPED(mrb)
|
||||
#define TASK_STOP(mrb)
|
||||
#define TASK_RETURN_EXCEPTION_AS_VALUE(mrb) FALSE
|
||||
#endif
|
||||
|
||||
/**
|
||||
@@ -2691,6 +2693,7 @@ RETRY_TRY_BLOCK:
|
||||
fiber_terminate(mrb, c, ci);
|
||||
if (mrb_unlikely(!c->vmexec)) goto L_RAISE;
|
||||
mrb->jmp = prev_jmp;
|
||||
if (TASK_RETURN_EXCEPTION_AS_VALUE(mrb)) return mrb_obj_value(mrb->exc);
|
||||
if (!prev_jmp) return mrb_obj_value(mrb->exc);
|
||||
MRB_THROW(prev_jmp);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user