Preserve Unix signal dispositions when probing handlers by 1ndahous3 · Pull Request #8961 · RustPython/RustPython · GitHub

Summary

Read Unix signal dispositions with sigaction without temporarily installing SIG_IGN. This preserves handler flags and masks and avoids a window in which signals can be discarded.

Extracted from #8944 as an independent fix.

AI assistance

This PR follows our AI policy.

Written with Codex (GPT-6), reviewed by a human before submission.

Summary by CodeRabbit

Bug Fixes
- On Unix, checking a signal handler no longer changes its configured handler, flags, or signal mask. If the system cannot inspect the handler, the check now reports no result.
- Windows signal probing continues to use its existing behavior.

Assisted-by: Codex:gpt-6

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used 📚 Code guidelines (1)

AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
Review profile: CHILL
Plan: Advanced
Run ID: 9f74f57c-bb66-4452-bdd5-74bbf88c106e

📥 Commits

Reviewing files that changed from the base of the PR and between 7536730 and c78d4a0.

📒 Files selected for processing (1)

crates/host_env/src/signal.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

On Unix, probe_handler now reads a signal disposition with sigaction without changing it. The Windows implementation retains its signal-based probe-and-restore behavior. A Unix-only test checks that probing preserves the handler, flags, and mask.

Changes

Signal Handler Probe

Layer / File(s)	Summary
Probe behavior and preservation test crates/host_env/src/signal.rs	On Unix, probe_handler reads the current handler with sigaction and returns None if the query fails. The Windows probe retains its prior behavior. A Unix-only test checks that probing preserves the signal handler, flags, and mask.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: youknowone

Merge Risk: ⚪ Minimal · up to c78d4

This change makes Unix signal probing read-only, which removes a window in which signals could be discarded. No merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to c78d4

Unix signal inspection no longer temporarily changes handlers or risks discarding signals. The caller’s ownership restrictions and error handling remain intact, and Windows behavior is unchanged. No material security risk introduced or worsened by this change was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The relevant mutable resource is the hosting process’s signal disposition, shared by its interpreters and other in-process components. The change reduces mutation of that shared resource rather than introducing cross-process authority.

Trust Boundaries and Controls

observed — The inspected caller supplies signal numbers from its valid range and retains the main-interpreter ownership gate. The probe returns a handler value without invoking it. The flagged Restore and Drop declarations belong to a Unix test-only module, not an expanded production entrypoint.

Resilience and Maintainability Implications

observed — A failed Unix query requires no restoration because the probe has not changed signal state. Windows still performs the pre-existing two-call mutation and restoration sequence; that residual behavior was not introduced or worsened by this PR.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: Unix signal probing now preserves signal dispositions.
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.

Fix all pre-merge checks with AI

✨ Finishing Touches 🧪 Generate unit tests (beta)

Create a new PR

Autopilot · 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.

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1

Comparing 1ndahous3:signal_probe (c78d4a0) with main (7536730)

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 can we merge since it has already been approved?

I've got a few questions.

Is this the best way to order these attributes?

Does this unsafe block need to be this long?

this is test. simple test is better than perfect test

👍

Preserve Unix signal dispositions when probing handlers

c78d4a0

Assisted-by: Codex:gpt-6

youknowone approved these changes Oct 4, 2026

View reviewed changes

fanninpm reviewed Oct 7, 2026

View reviewed changes

youknowone approved these changes Oct 7, 2026

View reviewed changes

youknowone merged commit f9ebb23 into RustPython:main Oct 7, 2026
31 checks passed

+              #[cfg(test)]
+              #[cfg(unix)]
+              mod tests {
+                  #[test]
+                  fn probing_preserves_signal_flags_and_mask() {

+                      unsafe {
+                          let mut action: libc::sigaction = core::mem::zeroed();
+                          action.sa_sigaction = handler as *const () as libc::sighandler_t;
+                          action.sa_flags = libc::SA_NODEFER;
+                          assert_eq!(libc::sigemptyset(&mut action.sa_mask), 0);
+                          assert_eq!(libc::sigaddset(&mut action.sa_mask, libc::SIGUSR1), 0);
+                          let mut original = core::mem::MaybeUninit::uninit();
+                          assert_eq!(
+                              libc::sigaction(libc::SIGUSR2, &action, original.as_mut_ptr()),
+
+                          );
+                          let _restore = Restore(original.assume_init());
+                          assert_eq!(
+                              super::probe_handler(libc::SIGUSR2),
+                              Some(action.sa_sigaction)
+                          );
+                          let mut observed = core::mem::MaybeUninit::<libc::sigaction>::uninit();
+                          assert_eq!(
+                              libc::sigaction(libc::SIGUSR2, core::ptr::null(), observed.as_mut_ptr()),
+
+                          );
+                          let observed = observed.assume_init();
+                          assert_eq!(observed.sa_sigaction, action.sa_sigaction);
+                          assert_ne!(observed.sa_flags & libc::SA_NODEFER, 0);
+                          assert_eq!(libc::sigismember(&observed.sa_mask, libc::SIGUSR1), 1);
+                      }

Preserve Unix signal dispositions when probing handlers by 1ndahous3 · Pull Request #8961 · RustPython/RustPython · GitHub

Repository navigation

Preserve Unix signal dispositions when probing handlers - #8961

Preserve Unix signal dispositions when probing handlers#8961
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:signal_probe

1ndahous3 commented Oct 4, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Oct 4, 2026

Uh oh!

coderabbitai Bot commented Oct 4, 2026 •
edited
Loading

Uh oh!

Uh oh!

codspeed Bot commented Oct 4, 2026

Uh oh!

youknowone left a comment

Uh oh!

1ndahous3 commented Oct 7, 2026

Uh oh!

fanninpm left a comment

Uh oh!

fanninpm Oct 7, 2026

Uh oh!

fanninpm Oct 7, 2026

Uh oh!

youknowone Oct 7, 2026

Uh oh!

youknowone left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Preserve Unix signal dispositions when probing handlers by 1ndahous3 · Pull Request #8961 · RustPython/RustPython · GitHub

Repository navigation

Preserve Unix signal dispositions when probing handlers - #8961

Preserve Unix signal dispositions when probing handlers#8961youknowone merged 1 commit intoRustPython:mainRustPython/RustPython:mainfrom 1ndahous3:signal_probe1ndahous3/RustPython:signal_probeCopy head branch name to clipboard

Conversation

1ndahous3 commented Oct 4, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

AI assistance

Summary by CodeRabbit

Uh oh!

chatgpt-codex-connector Bot commented Oct 4, 2026

Uh oh!

coderabbitai Bot commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

codspeed Bot commented Oct 4, 2026

Merging this PR will not alter performance

Footnotes

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

1ndahous3 commented Oct 7, 2026

Uh oh!

fanninpm left a comment

Choose a reason for hiding this comment

Uh oh!

fanninpm Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

fanninpm Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

youknowone Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Preserve Unix signal dispositions when probing handlers#8961
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:signal_probe

1ndahous3 commented Oct 4, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Oct 4, 2026 •
edited
Loading