fix(task): sleep() no-arg suspends calling task, not queue head#6924
fix(task): sleep() no-arg suspends calling task, not queue head#6924ARYA-mgc wants to merge 2 commits into
Conversation
mrb_f_sleep() with no arguments used q_ready_ (the head of the ready queue) instead of the currently running task. When a lower-priority task called sleep(), the highest-priority ready task was incorrectly suspended instead. Use MRB2TASK(mrb) to obtain the running task via pointer arithmetic, matching the pattern used by sleep_us_impl() and Task.pass. Also handle root-context calls which have no task to suspend.
There was a problem hiding this comment.
Code Review
This pull request updates mrb_f_sleep in mruby-task to suspend the current calling task (MRB2TASK(mrb)) instead of the head of the ready queue (q_ready_) when called with no arguments, and adds a corresponding test case. The reviewer noted that calling sleep with no arguments from within a C function boundary is unsafe and can cause VM corruption or crashes, suggesting a check to raise a runtime error in this scenario.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| /* Root context has no task to suspend */ | ||
| return mrb_nil_value(); | ||
| } | ||
| mrb_task *t = MRB2TASK(mrb); |
There was a problem hiding this comment.
When sleep is called with no arguments (indefinite suspend) from within a C function boundary, attempting to perform a cooperative context switch is unsafe and can lead to VM corruption or crashes.
Similar to Task.pass and Queue#pop, we should check for a C function boundary (ci->cci > 0) and raise an E_RUNTIME_ERROR if one is encountered.
mrb_callinfo *ci;
for (ci = mrb->c->ci; ci >= mrb->c->cibase; ci--) {
if (ci->cci > 0) {
mrb_raise(mrb, E_RUNTIME_ERROR, "can't sleep across C function boundary");
}
}
mrb_task *t = MRB2TASK(mrb);There was a problem hiding this comment.
Fixed it now - moved the root-context and C-boundary checks before the suspend logic so it raises properly.
As pointed out in code review, attempting a cooperative context switch from within a C function boundary can corrupt the VM. Check ci->cci and raise E_RUNTIME_ERROR if one is encountered.
|
Nice find. Using Merged into master as 9eb5b5e and 8ceeea6. I resolved the small test/task.rb append overlap with #6923 by keeping both tests. |
mrb_f_sleep() with no arguments used q_ready_ (the head of the ready queue) instead of the currently running task. When a lower-priority task called sleep(), the highest-priority ready task was incorrectly suspended instead.
Use MRB2TASK(mrb) to obtain the running task via pointer arithmetic, matching the pattern used by sleep_us_impl() and Task.pass. Also handle root-context calls which have no task to suspend.
Part of #6922.