unix: Wake sleeping operations immediately on scheduled callbacks.#9
Open
andrewleech wants to merge 3 commits intoreview/unix-sleep-process-pendingfrom
Open
unix: Wake sleeping operations immediately on scheduled callbacks.#9andrewleech wants to merge 3 commits intoreview/unix-sleep-process-pendingfrom
andrewleech wants to merge 3 commits intoreview/unix-sleep-process-pendingfrom
Conversation
Install an empty signal handler (without SA_RESTART) for a dedicated signal so that select() calls return EINTR when the scheduler queues a callback. mp_hal_signal_event() sends this signal via kill(getpid()), and MICROPY_SCHED_HOOK_SCHEDULED calls it from mp_sched_schedule(). This replaces the need for a self-pipe mechanism while remaining async-signal-safe. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Rewrite time.sleep() to use mp_unix_sched_sleep(), a select() call with no file descriptors that returns early on EINTR from the scheduler signal. The sleep loop recomputes remaining time on each iteration, handling both signal wakeups and time.sleep(0) correctly. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Replace the fixed 500us delay in MICROPY_INTERNAL_WFE with mp_unix_sched_sleep(), which sleeps for the full requested timeout but wakes immediately on EINTR from the scheduler signal. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Owner
Author
|
/review |
| } | ||
|
|
||
| void mp_unix_init_sched_signal(void) { | ||
| struct sigaction sa; |
There was a problem hiding this comment.
sa is a stack variable; sa_restorer (and any padding) is uninitialised. Use:
Suggested change
| struct sigaction sa; | |
| struct sigaction sa = {0}; |
|
|
||
| void mp_unix_deinit_sched_signal(void) { | ||
| MP_SIGMASK(SIG_UNBLOCK, &sched_signal_mask, NULL); | ||
| signal(MP_SCHED_SIGNAL, SIG_DFL); |
There was a problem hiding this comment.
Use sigaction consistently rather than mixing with signal:
Suggested change
| signal(MP_SCHED_SIGNAL, SIG_DFL); | |
| struct sigaction sa = {0}; | |
| sa.sa_handler = SIG_DFL; | |
| sigemptyset(&sa.sa_mask); | |
| sigaction(MP_SCHED_SIGNAL, &sa, NULL); |
| // Note that we don't delay for the full TIMEOUT_MS, as execution | ||
| // can't be woken from the delay. | ||
| // Wait for an event (scheduled callback) or timeout. A signal from | ||
| // mp_hal_signal_event() causes select() to return EINTR, waking the wait. |
| mp_raise_ValueError(MP_ERROR_TEXT("sleep length must be non-negative")); | ||
| } | ||
| RAISE_ERRNO(res, errno); | ||
| uint64_t total_ms = (uint64_t)(val * MICROPY_FLOAT_CONST(1000.0)); |
There was a problem hiding this comment.
This truncates to milliseconds, so time.sleep(0.0001) becomes a no-op (total_ms=0 → loop exits immediately without sleeping). The old code had microsecond precision. At minimum, round up:
Suggested change
| uint64_t total_ms = (uint64_t)(val * MICROPY_FLOAT_CONST(1000.0)); | |
| uint64_t total_ms = (uint64_t)MICROPY_FLOAT_C_FUN(ceil)(val * MICROPY_FLOAT_CONST(1000.0)); |
|
Code size report: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On the unix port,
time.sleep()andMICROPY_INTERNAL_WFE()were unresponsive to scheduled callbacks —time.sleep()usedselect()orsleep()with no wakeup mechanism, and WFE busy-polled with a 500us delay.This adds a signal-based notification so that when something is scheduled (via
MICROPY_SCHED_HOOK_SCHEDULED), a signal is sent to the process. Asig_atomic_tflag is set by bothmp_hal_signal_event()and the signal handler.mp_unix_sched_sleep()usespselect()to atomically unblock the signal and enter the wait, avoiding the TOCTOU race where signals for already-queued callbacks could be consumed before the sleep. The signal is blocked process-wide at init so only the thread insidepselect()can receive it, andpthread_sigmaskis used on threaded builds for POSIX correctness.time.sleep()is reworked to loop overmp_unix_sched_sleep()with elapsed-time tracking, processing pending callbacks each iteration. Negative values now raiseValueErrorto match CPython. WFE uses the same sleep primitive so it blocks properly and wakes on events rather than spinning.Uses
SIGRTMIN+7where real-time signals are available, falling back toSIGURG.Testing
Tested on the unix port (Linux), both standard and coverage variants. Windows codepath (
_WIN32) is left unchanged — it keeps the existingselect()/delay_usbehaviour.Trade-offs and Alternatives
Could have used a pipe/eventfd self-pipe pattern instead of signals, which would avoid any signal-number collision concerns. Signals are simpler and don't require managing an fd, and the chosen signal numbers avoid known conflicts with GC (
SIGRTMIN+5) and thread terminate (SIGRTMIN+6).The
time.sleep()rewrite replaces theMICROPY_SELECT_REMAINING_TIMELinux-specific assumption aboutselect()modifying the timeout struct with explicitmp_hal_ticks_ms()tracking, which is portable. Precision drops from microseconds to milliseconds which is acceptable fortime.sleep().Generative AI
I used generative AI tools when creating this PR, but a human has checked the code and is responsible for the description above.