From 77b6febc191fb06d1efd7998a88fae15196ad21e Mon Sep 17 00:00:00 2001 From: "Yukihiro \"Matz\" Matsumoto" Date: Fri, 10 Oct 2025 16:30:50 +0900 Subject: [PATCH] mruby-task: improve sleep implementation with efficiency and safety improved sleep_us_impl() in several ways: 1. dynamic sleep intervals: now sleeps for actual remaining time instead of fixed 1ms polling, reducing unnecessary wakeups and improving efficiency for longer sleeps 2. error handling: added checks for clock_gettime() failures with fallback to usleep(), and input validation to handle negative values 3. overflow prevention: use named constant USEC_PER_MSEC instead of literal 1000 for microsecond-to-nanosecond conversion, and validate input before conversion 4. wraparound handling: fixed tick comparison at line 580 to use signed arithmetic like other tick comparisons in the codebase 5. code clarity: added time conversion constants (NSEC_PER_MSEC, NSEC_PER_SEC, USEC_PER_MSEC) to replace magic numbers all tests pass. Co-authored-by: Claude --- mrbgems/mruby-task/src/task.c | 53 ++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/mrbgems/mruby-task/src/task.c b/mrbgems/mruby-task/src/task.c index 887c20500..a1f754e60 100644 --- a/mrbgems/mruby-task/src/task.c +++ b/mrbgems/mruby-task/src/task.c @@ -52,6 +52,11 @@ } \ } while (0) +/* Time conversion constants */ +#define NSEC_PER_MSEC 1000000ULL +#define NSEC_PER_SEC 1000000000ULL +#define USEC_PER_MSEC 1000ULL + /* Convert microseconds to tick count */ #define USEC_TO_TICKS(usec) (((usec) / 1000) / MRB_TICK_UNIT) @@ -504,28 +509,56 @@ sleep_us_impl(mrb_state *mrb, mrb_int usec) if (mrb->c == mrb->root_c) { /* Not in task context - sleep in real wall-clock time */ #ifdef __unix__ - struct timespec start, now; - clock_gettime(CLOCK_MONOTONIC, &start); + struct timespec start, now, sleep_time; + int ret; - uint64_t target_ns = (uint64_t)usec * 1000ULL; + /* Validate input to prevent overflow */ + if (usec < 0) { + return; + } + + ret = clock_gettime(CLOCK_MONOTONIC, &start); + if (ret != 0) { + /* Fallback to simple usleep if clock_gettime fails */ + usleep(usec); + switching_ = FALSE; + return; + } + + uint64_t target_ns = (uint64_t)usec * USEC_PER_MSEC; /* Loop until enough real time has elapsed */ while (1) { - clock_gettime(CLOCK_MONOTONIC, &now); - uint64_t elapsed_ns = (uint64_t)(now.tv_sec - start.tv_sec) * 1000000000ULL + + ret = clock_gettime(CLOCK_MONOTONIC, &now); + if (ret != 0) { + break; /* Clock failure - exit loop */ + } + + uint64_t elapsed_ns = (uint64_t)(now.tv_sec - start.tv_sec) * NSEC_PER_SEC + (uint64_t)(now.tv_nsec - start.tv_nsec); if (elapsed_ns >= target_ns) { break; } - /* Sleep for a short interval (1ms) to allow signals and reduce CPU usage */ - struct timespec short_sleep = {0, 1000000}; /* 1ms */ - nanosleep(&short_sleep, NULL); /* Interrupted by signals - that's fine */ + /* Sleep for remaining time, but at least 1ms to allow timer interrupts */ + uint64_t remaining_ns = target_ns - elapsed_ns; + if (remaining_ns > NSEC_PER_MSEC) { + sleep_time.tv_sec = remaining_ns / NSEC_PER_SEC; + sleep_time.tv_nsec = remaining_ns % NSEC_PER_SEC; + } + else { + sleep_time.tv_sec = 0; + sleep_time.tv_nsec = NSEC_PER_MSEC; + } + + nanosleep(&sleep_time, NULL); /* Interrupted by signals - that's OK */ } #elif defined(_WIN32) /* Windows: just use Sleep, it handles interruptions */ - Sleep(usec / 1000); + if (usec >= 0) { + Sleep(usec / 1000); + } #endif /* Clear switching flag - we're in root context, not switching to a task */ switching_ = FALSE; @@ -547,7 +580,7 @@ sleep_us_impl(mrb_state *mrb, mrb_int usec) t->wakeup_tick = tick_ + USEC_TO_TICKS(usec); /* Update next wakeup time if this task wakes earlier */ - if (t->wakeup_tick < wakeup_tick_) { + if ((int32_t)(t->wakeup_tick - wakeup_tick_) < 0) { wakeup_tick_ = t->wakeup_tick; }