Separate the union of timeslice and result in struct mrb_task

Bug scenario:
* VM returns an Exception `t->state.result = mrb_vm_exec(...);`
* Despite task is still MRB_TASK_STATUS_RUNNING, IRQ triggered by chance and `mrb_tick()` executes `t->state.timeslice--;`
* But the same memory area already holds the `result`, `timeslice--` reduces `result.value.p`'s top byte

The fix is to separate `timeslice` and `result` into different fields.
I have considered improving critical sections, but I ended up with this patch because I believe it is widely effective and less error-prone.
This commit is contained in:
HASUMI Hitoshi
2026-05-28 18:13:18 +09:00
parent d2a1c43a5f
commit 030a08092a
2 changed files with 14 additions and 18 deletions
+2 -6
View File
@@ -41,7 +41,6 @@ struct mrb_task_queue;
* - Removed started flag (inferred from c.status): 1 byte
* - Unified wakeup_tick/join/mutex into single union: 4 bytes
* - Removed redundant proc field (stored in c.ci->proc): 8 bytes
* - Unified timeslice/result into state union: ~4 bytes
* Total savings: ~18 bytes per task (14% reduction)
*/
typedef struct mrb_task {
@@ -61,11 +60,8 @@ typedef struct mrb_task {
mrb_value self; /* Ruby Task object reference */
/* State-specific data - mutually exclusive based on status */
union {
volatile uint8_t timeslice; /* Remaining ticks (RUNNING only) */
mrb_value result; /* Task return value (DORMANT only) */
} state;
volatile uint8_t timeslice; /* Remaining ticks while RUNNING */
mrb_value result; /* Task return value */
struct mrb_context c; /* Execution context (stack, callinfo, etc) */
} mrb_task;
+12 -12
View File
@@ -114,7 +114,7 @@ mrb_task_mark_all(mrb_state *mrb)
/* Mark task-specific values */
mrb_gc_mark_value(mrb, t->self);
if (t->status == MRB_TASK_STATUS_DORMANT) {
mrb_gc_mark_value(mrb, t->state.result);
mrb_gc_mark_value(mrb, t->result);
}
mrb_gc_mark_value(mrb, t->name);
@@ -326,7 +326,7 @@ execute_task(mrb_state *mrb, mrb_task *t)
/* Set task as running */
t->status = MRB_TASK_STATUS_RUNNING;
t->state.timeslice = MRB_TIMESLICE_TICK_COUNT;
t->timeslice = MRB_TIMESLICE_TICK_COUNT;
/* Switch to task context */
prev_c = mrb->c;
@@ -351,7 +351,7 @@ execute_task(mrb_state *mrb, mrb_task *t)
t->c.vmexec = TRUE;
/* Execute task - PC is saved in ci->pc from previous run */
t->state.result = mrb_vm_exec(mrb, proc, pc);
t->result = mrb_vm_exec(mrb, proc, pc);
/* Clear vmexec flag */
t->c.vmexec = FALSE;
@@ -394,9 +394,9 @@ mrb_tick(mrb_state *mrb)
/* Decrease timeslice for running task */
t = q_ready_;
if (t && t->status == MRB_TASK_STATUS_RUNNING && t->state.timeslice > 0) {
t->state.timeslice--;
if (t->state.timeslice == 0) {
if (t && t->status == MRB_TASK_STATUS_RUNNING && t->timeslice > 0) {
t->timeslice--;
if (t->timeslice == 0) {
switching_ = TRUE; /* Trigger context switch */
}
}
@@ -1115,7 +1115,7 @@ mrb_task_join(mrb_state *mrb, mrb_value self)
/* If task is already dormant, return immediately */
if (t->status == MRB_TASK_STATUS_DORMANT) {
return t->state.result;
return t->result;
}
/* Wait for task to complete */
@@ -1130,7 +1130,7 @@ mrb_task_join(mrb_state *mrb, mrb_value self)
/* Trigger context switch */
switching_ = TRUE;
return t->state.result;
return t->result;
}
/*
@@ -1190,16 +1190,16 @@ mrb_execute_proc_synchronously(mrb_state *mrb, mrb_value proc_val, mrb_int argc,
mrb->c = &t->c;
while (t->c.status != MRB_TASK_STOPPED) {
t->state.result = mrb_vm_exec(mrb, mrb->c->ci->proc, mrb->c->ci->pc);
t->result = mrb_vm_exec(mrb, mrb->c->ci->proc, mrb->c->ci->pc);
}
/* If there's an unhandled exception after VM stops, save it as result */
if (mrb->exc) {
t->state.result = mrb_obj_value(mrb->exc);
t->result = mrb_obj_value(mrb->exc);
}
/* 5. Get result and clean up */
mrb_value result = t->state.result;
mrb_value result = t->result;
if (mrb_obj_ptr(result) == mrb->exc) {
mrb->exc = NULL; /* Clear exception */
}
@@ -1437,7 +1437,7 @@ mrb_task_value(mrb_state *mrb, mrb_value task)
mrb_task *t = (mrb_task*)mrb_data_check_get_ptr(mrb, task, &mrb_task_type);
if (!t) return mrb_nil_value();
return t->state.result;
return t->result;
}
/*