Hi @matz , I'm pretty new to mruby (this is my first time digging into the mruby codebase), and I ran into some weird task-scheduling behaviorwhile working with mruby-task that led me down a rabbit hole into task.c. Found two bugs in the process, filing them together since they're both small and in the same file - I'll open separate PRs per the contributing guide, just wanted to write up what I found first incase I'm missing some context on why this is written the way it is.

Bug 1: sleep with no argument suspends the wrong task

Kernel#sleep with no arguments is supposed to suspend the calling task indefinitely. What it actually suspends is whichever task happens to be at the head of the ready queue, which isn't always the same task.

mrbgems/mruby-task/src/task.c, mrb_f_sleep:

mrb_int n = mrb_get_args(mrb, "|f", &sec);
if (n == 0) {
  mrb_task *t = q_ready_;   // head of ready queue, not necessarily the caller
  ...
  t->status = MRB_TASK_STATUS_SUSPENDED;
  ...
}

q_ready_ is just the head of the priority-sorted ready list. What you
actually want here is the task that's currently executing, and there's
already a macro for that - MRB2TASK(mrb) - used elsewhere in this
same file for exactly this purpose (e.g. the join-waiter wakeup path).

How I ended up looking at this: three tasks, A(50) / B(100) /
C(128), ready queue sorted A→B→C. C is running and calls bare sleep
expecting to suspend itself.

• Expected: C suspends.
• Actual: A gets suspended instead, C keeps running.

Fix I'm proposing:

mrb_task *t = MRB2TASK(mrb);

(plus a guard for mrb->c == mrb->root_c, since MRB2TASK assumes
mrb->c is embedded in an mrb_task - calling it from the root
context would be UB.)

Bug 2: terminate_task_internal self-check never actually fires

When a task terminates itself, the scheduler is supposed to trigger a
context switch so the next task runs. The check for that looks like
it's dead code:

mrb_task_disable_irq();
mrb_task_q_delete(mrb, t);       // removed from ready queue
t->status = MRB_TASK_STATUS_DORMANT;
t->c.status = MRB_TASK_STOPPED;
mrb_task_q_insert(mrb, t);        // now in the dormant queue
mrb_task_enable_irq();

wake_up_join_waiters(mrb, t);

if (t == q_ready_) {              // this can't be true anymore
  switching_ = TRUE;
}

By the time this runs, t has already been pulled out of the ready queue and dropped into the dormant queue, so t == q_ready_ can never be true. Net effect: switching_ never gets set when a task terminates itself, so it can keep executing past the terminate call until something else forces a switch (I hit this as a task running one extra statement after calling terminate on itself - took me a while to realize that's not supposed to happen).

Fix I'm proposing: grab whether it was running before moving it:

mrb_bool was_running = (t->status == MRB_TASK_STATUS_RUNNING);
// ... delete / mark dormant / insert, same as before ...
if (was_running) {
  switching_ = TRUE;
}

Let me know if I'm missing something about why these are written this way, or if you'd rather I structure the PRs differently. Happy to add
regression tests for both - bug 2 is straightforward to test at the mtest level, bug 1 might need something lower-level since it's a
timing-dependent race on the interrupt path, still thinking through the cleanest way to pin that down deterministically.