Skip to content

Translate WASI signal dispositions through libc sentinels - #9024

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:fix-wasi-signal-sentinels
Oct 9, 2026
Merged

youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:fix-wasi-signal-sentinels

Conversation

@youknowdot

Copy link
Copy Markdown
Contributor

Scope

Translate Python's SIG_DFL and SIG_IGN values through the actual libc signal dispositions on WASI, and recognize libc's SIG_ERR before updating Python's handler table.

WASI uses function-table pointers for its signal sentinels. Treating them as the usual integer values lets an ignored synchronous signal trap and lets an uncatchable signal registration appear to succeed. A typed host boundary keeps Python's numeric values distinct from libc's default, ignored, and custom handlers.

This is a narrow Python 3.14 compatibility extraction from #8954. It does not include the separate wakeup-fd error-reporting changes.

Regression coverage

The existing signal snippet adds synchronous custom delivery, replacement with SIG_IGN, ignored delivery, and restoration. On WASI it also checks that SIGKILL/SIGSTOP registration raises OSError without changing the Python handler table. Existing canonical tests and assertions are unchanged.

Verification

Actual RustPython modules were built from current main bd8fdb7 and the isolated commit 59f9e3c, using the same source station, official wasm32-wasip1 target, and build flags:

cargo build --locked --profile wasm-release --target wasm32-wasip1 --no-default-features --features freeze-stdlib,stdlib,stdio,importlib,host_env,doc -j 2

Under Wasmtime 49.0.0:

  • Baseline: raising an ignored signal traps with an indirect-call type mismatch; registering an uncatchable signal incorrectly succeeds.
  • Fixed: both regressions pass, including handler-table preservation. Custom delivery and restoration pass.
  • The unchanged canonical RaiseSignalTest.test_sigint and test_handler pass on both versions.
  • The complete updated signal snippet passes when run directly as extra_tests/snippets/stdlib_signal.py by the fixed RustPython WASM module.

The updated snippet also passes the repository pytest runner on CPython 3.14.7 and the Linux RustPython baseline. Normal commit hooks pass. Native Linux Clippy with warnings denied, the workspace compile-only aggregate, and the separate C-API compile-only aggregate all pass on the exact isolated source tree. Broad runtime suites were deliberately not run during this extraction.

Reference implementations: CPython 3.14 signalmodule.c, wasi-libc signal constants, and wasi-libc signal implementation.

AI assistance: OpenAI Codex assisted with source comparison, implementation, regression design, and automated verification under the user's direction. No manual validation is claimed.

Keep Python disposition values distinct from WASI function-table pointers. Recognize wasi-libc SIG_ERR before updating Python handlers and preserve ignored/default/custom dispositions. Add synchronous signal and rejected-registration regressions to the existing snippet.

AI assistance covered source investigation, implementation, and automated reference/baseline tests. Isolated patched runtime validation is pending. No human validation is claimed.

Assisted-by: Codex:model-version-unavailable
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: 7d573662-7d51-4c23-85c1-8579672a339c

📥 Commits

Reviewing files that changed from the base of the PR and between bd8fdb7 and 59f9e3c.


📒 Files selected for processing (3)
  • crates/host_env/src/signal.rs
  • crates/vm/src/stdlib/_signal.rs
  • extra_tests/snippets/stdlib_signal.py

 __________________________________________
< Dollars to donuts, you didn't test this. >
 ------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR


  • 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.

@codspeed

codspeed Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1


Comparing youknowdot:fix-wasi-signal-sentinels (59f9e3c) with main (bd8fdb7)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@youknowone youknowone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @jiwahn

@youknowone
youknowone marked this pull request as ready for review October 9, 2026 14:59
@youknowone
youknowone merged commit d4b43b8 into RustPython:main Oct 9, 2026
31 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

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-09T15:04:37.824056Z 59f9e3c 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

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: 59f9e3ccd7

ℹ️ 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".

Comment on lines +33 to +35
previous = signal.signal(signal.SIGILL, synchronous_handler)
try:
signal.raise_signal(signal.SIGILL)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Remove the added signal test block

This starts a new signal exercise and the following assertions in an existing test file, but the repository's test-code policy permits only adding an expected-failure decorator or removing one and its TODO once the test passes, and explicitly forbids modifying test logic or assertions. Remove this added snippet coverage rather than editing this test file.

AGENTS.md reference: AGENTS.md:L275-L279

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants