cleanup the signals module

[ShaharNaveh]

• Closes #xxxx
• This PR follows our AI policy

Summary

Summary by CodeRabbit

• Refactor

  • Reorganized and consolidated signal handling and wakeup logic for clearer cross-platform behavior and maintainability.
  • Streamlined interval timer and mask handling paths.

• Bug Fixes

  • Improved signal validation and error reporting to reduce incorrect/ambiguous errors.

• Notes

  • No public API/signature changes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 80e9d1f7-4b7e-4502-b4a1-af3681e3c62d

📥 Commits

Reviewing files that changed from the base of the PR and between 31723bd and aaba3fa.

📒 Files selected for processing (2)

• crates/vm/src/signal.rs
• crates/vm/src/stdlib/_signal.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• crates/vm/src/signal.rs
• crates/vm/src/stdlib/_signal.rs

---

📝 Walkthrough

Walkthrough

Adds a shared SIGNUM_RANGE and refactors signal modules: reorganizes imports, centralizes signal-number iteration/validation, consolidates itimer error handling, standardizes wakeup-fd bookkeeping, and unifies notification and CPython-compatible signal APIs across Unix and Windows.

Changes

Signal Handling Consolidation

Layer / File(s)	Summary
Signal.rs module import and attribute cleanup crates/vm/src/signal.rs	Reorders imports and declarations, replaces a file-level WASI cfg_attr, changes a clippy workaround to an expect attribute, and combines platform gating for clear_after_fork.
SIGNUM_RANGE constant and module reorganization crates/vm/src/stdlib/_signal.rs	Introduces SIGNUM_RANGE (1..signal::NSIG), moves platform cfg imports into cfg_select! blocks, adds new_itimer_error() helper, and adjusts Android ITIMER_VIRTUAL placement and compile-time assertions.
Signal handler initialization using SIGNUM_RANGE crates/vm/src/stdlib/_signal.rs	Initializes handlers by iterating over SIGNUM_RANGE and indexes the handler table using the signum-derived usize.
Signal registration and validation refactoring crates/vm/src/stdlib/_signal.rs	Simplifies Windows out-of-range checks with early return, calls signal::check_signals(vm), and maps host_signal::install_handler errors via map_err to OS errors.
Interval timer error handling consolidation crates/vm/src/stdlib/_signal.rs	setitimer()/getitimer() map host results to tuples and convert errors using new_itimer_error instead of inline exception construction.
Wakeup fd platform-specific handling crates/vm/src/stdlib/_signal.rs	set_wakeup_fd() extracts raw fd with cfg_select!, records socket-ness in WAKEUP_IS_SOCKET on Windows, and returns -1 when previous wakeup fd was invalid.
Signal API consolidation and range standardization crates/vm/src/stdlib/_signal.rs	Consolidates raise_signal(), strsignal(), and valid_signals() under #[cfg(any(unix, windows))] and standardizes range checks to SIGNUM_RANGE.
sigset_to_pyset and pthread_sigmask validation crates/vm/src/stdlib/_signal.rs	sigset_to_pyset() iterates SIGNUM_RANGE; pthread_sigmask() converts mask entries to i32, validates with SIGNUM_RANGE.contains(), and updates the out-of-range ValueError message.
Unified signal notification with conditional Windows arguments crates/vm/src/stdlib/_signal.rs	run_signal() invokes host_signal::notify_signal once and supplies Windows-only args (WAKEUP_IS_SOCKET, get_sigint_event()) via cfg.

🎯 4 (Complex) | ⏱️ ~60 minutes

"I hop through code with keen delight,
I stitch the signals, day and night,
With SIGNUM_RANGE I guide the way,
Unix and Windows now both play,
A rabbit's patchwork, snug and bright." 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'cleanup the signals module' is vague and generic, using non-descriptive terminology that doesn't convey meaningful information about the specific changes made.	Provide a more specific title that describes the actual changes, such as 'Refactor signal module with SIGNUM_RANGE and cfg improvements' or similar, to clarify what cleanup was performed.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/vm/src/stdlib/_signal.rs`:
- Around line 419-424: The change incorrectly reuses SIGNUM_RANGE to reject
signal 0 in strsignal(), causing ValueError for lookups that
host_signal::strsignal(signalnum) can legitimately handle (e.g., on Unix).
Remove or relax the SIGNUM_RANGE check from the strsignal function (the
#[pyfunction] fn strsignal(signalnum: i32, vm: &VirtualMachine) ->
PyResult<Option<String>>) so it does not reject 0 up front; instead, call
host_signal::strsignal(signalnum) directly and only error if that host call
indicates an invalid signal, keeping the existing ValueError behavior elsewhere
that uses SIGNUM_RANGE for delivery-only validation.
- Around line 482-485: The ValueError message created via vm.new_value_error
currently claims a closed interval "[1, {SIGNUM_RANGE.end}]" while SIGNUM_RANGE
is half-open; update the error text in the vm.new_value_error call so the upper
bound reflects the actual allowed value (e.g., use format!("signal number out of
range [1, {}]", SIGNUM_RANGE.end - 1) or state the half-open form like "[1,
{})") so the message matches SIGNUM_RANGE.contains() validation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 73fe0c56-24f1-48d5-b1b6-000b66de91df

📥 Commits

Reviewing files that changed from the base of the PR and between 65dee6c and 31723bd.

📒 Files selected for processing (2)

• crates/vm/src/signal.rs
• crates/vm/src/stdlib/_signal.rs