From 2668619143ceba8e70dc3d356ddb44d23349e039 Mon Sep 17 00:00:00 2001 From: Hendrik Date: Sun, 17 May 2026 07:31:08 +0200 Subject: [PATCH 1/2] Improve wakeup tick condition check We got a off by one error here, checking against UINT32_MAX doesn't silently set a wrong value. When running the task mgem with a busy loop this doesn't cause an issue, but with a tickless timer only one timer ever gets fired and then the task mgem stops working. --- mrbgems/mruby-task/src/task.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mrbgems/mruby-task/src/task.c b/mrbgems/mruby-task/src/task.c index 7e364cbca..afe9b2438 100644 --- a/mrbgems/mruby-task/src/task.c +++ b/mrbgems/mruby-task/src/task.c @@ -568,7 +568,8 @@ sleep_us_impl(mrb_state *mrb, uint32_t usec) t->wait.wakeup_tick = tick_ + USEC_TO_TICKS(usec); /* Update next wakeup time if this task wakes earlier */ - if ((int32_t)(t->wait.wakeup_tick - wakeup_tick_) < 0) { + if (wakeup_tick_ == UINT32_MAX || + (int32_t)(t->wait.wakeup_tick - wakeup_tick_) < 0) { wakeup_tick_ = t->wait.wakeup_tick; } From 8b63ddbfd1b60d9eb15f395a796d3cb0fc64fa5a Mon Sep 17 00:00:00 2001 From: Hendrik Date: Sun, 17 May 2026 08:14:58 +0200 Subject: [PATCH 2/2] Refactor task wakeup logic and add comments Enhance wakeup logic for sleeping tasks to handle UINT32_MAX sentinel case and prevent race conditions with mrb_tick. This enables tickless Task hals to be build. --- mrbgems/mruby-task/src/task.c | 38 +++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/mrbgems/mruby-task/src/task.c b/mrbgems/mruby-task/src/task.c index afe9b2438..27347ace7 100644 --- a/mrbgems/mruby-task/src/task.c +++ b/mrbgems/mruby-task/src/task.c @@ -417,8 +417,16 @@ mrb_tick(mrb_state *mrb) } } - /* Wake up sleeping tasks whose wakeup time has passed */ - if ((int32_t)(wakeup_tick_ - tick_) <= 0) { + /* Wake up sleeping tasks whose wakeup time has passed. + * + * UINT32_MAX is the "no sleepers" sentinel. Without the explicit + * check, (int32_t)(UINT32_MAX - tick_) evaluates negative for the + * first half of the 32-bit range, so the queue walk fires every + * tick with nothing to do. Short-circuiting saves the walk in the + * common no-sleepers case -- benefits every HAL, not just + * tickless ones. */ + if (wakeup_tick_ != UINT32_MAX && + (int32_t)(wakeup_tick_ - tick_) <= 0) { mrb_task *curr = q_waiting_; mrb_task *next; uint32_t next_wakeup = UINT32_MAX; @@ -567,12 +575,19 @@ sleep_us_impl(mrb_state *mrb, uint32_t usec) /* Convert microseconds to ticks (tick unit is in milliseconds) */ t->wait.wakeup_tick = tick_ + USEC_TO_TICKS(usec); - /* Update next wakeup time if this task wakes earlier */ + /* Update next wakeup time if this task wakes earlier. + * + * When wakeup_tick_ is UINT32_MAX (no prior sleepers), the + * unsigned subtraction wraps to a small positive value and the + * int32_t cast stays non-negative, so the update is skipped and + * the global stays stale until the next mrb_tick self-heals it. + * Handle the sentinel explicitly so tickless HALs that read this + * field get a consistent value as soon as the sleeper is + * installed. */ if (wakeup_tick_ == UINT32_MAX || (int32_t)(t->wait.wakeup_tick - wakeup_tick_) < 0) { wakeup_tick_ = t->wait.wakeup_tick; } - q_insert_task(mrb, t); mrb_task_enable_irq(); @@ -1321,11 +1336,22 @@ resume_task_internal(mrb_state *mrb, mrb_task *t) } } - /* Update wakeup_tick if task has sleep reason */ + /* Update wakeup_tick if task has sleep reason. + * + * Two fixes here vs the original: + * - The UINT32_MAX sentinel case (see comments in + * sleep_us_impl). + * - The read-modify-write on wakeup_tick_ races with + * mrb_tick, which also rewrites this field. sleep_us_impl + * already wraps its update in the IRQ pair; we need the + * same here to match the locking discipline. */ if (t->reason == MRB_TASK_REASON_SLEEP) { - if ((int32_t)(t->wait.wakeup_tick - wakeup_tick_) < 0) { + mrb_task_disable_irq(); + if (wakeup_tick_ == UINT32_MAX || + (int32_t)(t->wait.wakeup_tick - wakeup_tick_) < 0) { wakeup_tick_ = t->wait.wakeup_tick; } + mrb_task_enable_irq(); } }