GeezerVison is showing github.com — Align signal wakeup errors and delivery with Python 3.15 by youknowdot · Pull Request #9199 · RustPython/RustPython
Conversation
youknowdot
commented
Oct 19, 2026
•
edited by youknowone
Loading
Uh oh!
There was an error while loading. Please reload this page.
Summary
Independent main-based signal-wakeup closure from #8954, following Modules/signalmodule.c of Python 3.15:
run_signal saves and restores errno around trip_signal, as signal_handler does.
- Wakeup-fd writes go through the new
crt_fd::write_noraise (_Py_write_noraise): EINTR is retried and the errno is returned without allocating. On Windows it shares the nonblocking-pipe ENOSPC handling with _write.
- Write errors (and Windows
send errors) are stored in bounded pending slots and honor warn_on_full_buffer on all platforms. They are reported only by the main interpreter's main thread, after Python signal handlers and VM pending calls, matching the handle_signals → make_pending_calls order of _Py_HandlePending.
- When a Python signal handler or a pending call raises, the eval breaker is re-armed so the remaining signals and pending wakeup errors are handled at the next check.
- Process signals are left pending for the main interpreter's main thread (
_Py_ThreadCanHandleSignals).
_set_thread_local_invalid_parameter_handler is declared with a nullable Option<InvalidParamHandler>, so a NULL previous handler is restored without UB.
test_signal is synced with CPython v3.15.0 (force_not_colorized on test_wakeup_write_error); existing RustPython markers are kept.
Validation
test_signal passes (run=57, skipped=26) on macOS.
test_wakeup_write_error is skipped without _testcapi. Its body (EBADF reported inside captured_stderr before a raising handler's exception escapes) and full-buffer reporting with default/True/False warn_on_full_buffer produce the same output on RustPython and CPython 3.15.0rc2.
- Workspace tests, CAPI tests (123 passed, 4 ignored) and both Clippy scopes pass.
rustpython-host_env Clippy passes for x86_64-pc-windows-msvc; Windows runtime behavior relies on CI.
AI assistance: OpenAI Codex (model version unavailable) prepared the initial split; Claude Code (claude-opus-5-5) reworked it against signalmodule.c.
🤖 Generated with Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Assisted-by: OpenAI Codex:model version unavailable
Assisted-by: OpenAI Codex:model version unavailable
Assisted-by: OpenAI Codex:model version unavailable
coderabbitai
Bot
commented
Oct 19, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|

Note Currently processing new changes in this PR. This may take a few minutes, please wait...
⚙️ Run configuration
-
- Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
- Review profile: CHILL
- Plan: Advanced
- Run ID:
7b37c7c8-f9ad-4bbe-a3b8-c96cfa4319bd
📥 Commits
-
Reviewing files that changed from the base of the PR and between e7e3dfd and 1bb5aa8.
⛔ Files ignored due to path filters: 1
-
Lib/test/test_signal.py is excluded by !Lib/**
📒 Files selected for processing: 5
-
crates/host_env/src/crt_fd.rs
crates/host_env/src/macros.rs
crates/host_env/src/signal.rs
crates/vm/src/signal.rs
crates/vm/src/stdlib/_signal.rs
+----------------------------------------------------------------------+
| You used `any` like it's a life jacket. Spoiler: it's a pool noodle. |
+----------------------------------------------------------------------+
\
\ /)/)
( '')
/ |
o(___)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
❤️ Share
-
Comment @coderabbitai help to get the list of available commands.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
📦 Library Dependencies
The following Lib/ modules were modified. Here are their dependencies:
:4: SyntaxWarning: invalid decimal literal
:7: SyntaxWarning: "\z" is an invalid escape sequence. Such sequences will not work in the future. Did you mean "\z"? A raw string is also an option.
:10: SyntaxWarning: "\400" is an invalid octal escape sequence. Such sequences will not work in the future. Did you mean "\400"? A raw string is also an option.
:4: SyntaxWarning: invalid decimal literal
:7: SyntaxWarning: "\z" is an invalid escape sequence. Such sequences will not work in the future. Did you mean "\z"? A raw string is also an option.
:10: SyntaxWarning: "\400" is an invalid octal escape sequence. Such sequences will not work in the future. Did you mean "\400"? A raw string is also an option.
:4: SyntaxWarning: invalid decimal literal
:7: SyntaxWarning: "\z" is an invalid escape sequence. Such sequences will not work in the future. Did you mean "\z"? A raw string is also an option.
:10: SyntaxWarning: "\400" is an invalid octal escape sequence. Such sequences will not work in the future. Did you mean "\400"? A raw string is also an option.
[x] lib: cpython/Lib/signal.py
[x] test: cpython/Lib/test/signalinterproctester.py
[x] test: cpython/Lib/test/test_signal.py (TODO: 9)
dependencies:
dependent tests: (102 tests)
- signal: test_asyncio test_builtin test_concurrent_futures test_eintr test_faulthandler test_file_eintr test_fork1 test_inspect test_io test_list test_os test_pty test_pyrepl test_regrtest test_runpy test_selectors test_signal test_socket test_socketserver test_subprocess test_support test_threading test_threadsignals test_unittest test_wsgiref
- asyncio: test_asyncio test_concurrent_futures test_external_inspection test_logging test_pdb test_unittest
- multiprocessing: test_compileall test_concurrent_futures test_fcntl test_genericalias test_importlib test_memoryview test_multiprocessing_forkserver test_multiprocessing_main_handling test_re
- concurrent.futures.process: test_concurrent_futures
- subprocess: test_android test_atexit test_audit test_bz2 test_c_locale_coercion test_cmd_line test_cmd_line_script test_ctypes test_dtrace test_embed test_gc test_gzip test_json test_launcher test_msvcrt test_ntpath test_osx_env test_peg_generator test_platform test_plistlib test_poll test_py_compile test_pyrepl test_quopri test_repl test_script_helper test_select test_shutil test_site test_sqlite3 test_sys test_sys_settrace test_sysconfig test_tempfile test_traceback test_tstring test_unittest test_urllib2 test_utf8_mode test_venv test_wait3 test_webbrowser test_xpickle test_zipfile
- ctypes.util: test_ctypes
- ensurepip: test_ensurepip
- http.server: test_httpservers test_robotparser test_urllib2_localnet test_xmlrpc
- imaplib: test_imaplib
- platform: test__locale test__osx_support test_baseexception test_cmath test_math test_mimetypes test_ssl test_strptime test_time test_winreg
Legend:
[+] path exists in CPython
[x] up-to-date, [ ] outdated
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Merging this PR will not alter performance
✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1
Comparing youknowdot:split/signal-wakeup-315 (00fba93) with main (e7e3dfd)
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
- Report pending wakeup write/send errors after Python signal handlers
and VM pending calls, and re-arm the eval breaker when a handler or
pending call raises.
- Save and restore errno once in run_signal around trip_signal instead
of inside each write path; stop preserving _doserrno.
- Add crt_fd::write_noraise and use it for the Unix and Windows wakeup
writes, sharing the single-write helper with _write.
- Keep pending wakeup errors across fork.
- Revert the suppress_iph handler type change and the unrelated quote
change in test_signal.
Assisted-by: Claude Code:claude-opus-5-5
_set_thread_local_invalid_parameter_handler takes and returns a nullable
function pointer; declare it as Option<InvalidParamHandler> so a NULL
previous handler is restored as None.
Assisted-by: Claude Code:claude-opus-5-5
Uh oh!
There was an error while loading. Please reload this page.
Codex Review Summary
This comment shows the latest Codex review activity on this pull request.
| Review |
Status |
Commit |
Review trigger |
| 📝 Code Review |
✅ Completed 2026-10-20T11:18:23.016555Z |
1bb5aa8 |
Draft marked ready |
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review" or "@codex security review".
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bb5aa8e0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
|
.signal_handlers
|
|
|
.get()
|
|
|
.expect("should never fail since we check above");
|
|
|
let can_handle_signals = vm.is_main_thread() && vm.state.is_main_interpreter();
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reject subinterpreter signal registrations
When _interpreters.exec() runs a subinterpreter on the process main OS thread, signal.signal() still passes its existing guard because that guard only checks vm.is_main_thread(), and it stores the callable in the subinterpreter's signal_handlers. This new predicate then refuses to drain that table; once the main interpreter resumes, it consumes the trigger against a different handler table, so the registered callable is never invoked and the process signal may be silently swallowed. Add the missing is_main_interpreter() rejection to signal() before restricting delivery this way.
Useful? React with 👍 / 👎.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
|
|
|
|
@unittest.skipIf(_testcapi is None, 'need _testcapi')
|
|
|
@unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()")
|
|
|
@force_not_colorized
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert the non-marker test edit
Adding @force_not_colorized and its import changes test execution metadata, but this repository explicitly limits test-file edits to adding or removing expectedFailure markers. Revert this synchronization and validate the runtime behavior without modifying the CPython test in this way.
AGENTS.md reference: AGENTS.md:L280-L282
Useful? React with 👍 / 👎.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Comment on lines
+556
to
557
|
|
#[cfg(target_os = "wasi")]
|
|
|
host_signal::notify_signal(signum, WAKEUP.load(Ordering::Relaxed));
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Route WASI wakeup failures through error reporting
On wasm32-wasip1, a closed, read-only, or nonblocking-full wakeup fd still takes this branch, and crates/host_env/src/signal.rs's WASI notify_signal() discards the libc::write result while WAKEUP_WRITE_ERRORS and report_wakeup_errors() are not compiled for WASI. Consequently the warn_on_full_buffer value now stored by set_wakeup_fd() has no effect there and even the default True never reports the unraisable OSError; extend the pending-error path to WASI or retain the still-accurate TODO that this commit removed.
AGENTS.md reference: AGENTS.md:L165-L165
Useful? React with 👍 / 👎.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
youknowdot
added a commit
to youknowdot/RustPython
that referenced
this pull request
Oct 24, 2026
Restore the intentional policy from RustPython#9199 follow-up b46dcae.
The mega replay incorrectly retained the old pending-error clearing loop.
Remove only that loop, leaving the complete signal module identical to
landed main and keeping the existing wakeup-fd invalidation unchanged.
CPython retains these pending-call records across fork separately from
signal flags. This correction does not claim prompt child delivery or
change the existing RustPython eval-breaker scheduling behavior.
Assisted-by: OpenAI Codex: model version unavailable
youknowdot
added a commit
to youknowdot/RustPython
that referenced
this pull request
Oct 24, 2026
Integrate main at 5bccf31 without replacing the existing Ruff vendor.
Preserve the landed Cranelift dependency family and RustPython#9282 original filename
handling, including surrogate/bytes filenames and arbitrary warning-hook
exceptions. Thread the same filename object through remaining mega numeric
and relative-lazy warning/recovery paths without changing event ordering.
The preceding signal correction restores RustPython#9199 pending-error preservation;
it does not claim immediate post-fork delivery. Keep all tests unchanged.
Assisted-by: OpenAI Codex: model version unavailable
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|