E522 signal.pthread_sigmask by youknowone · Pull Request #6475 · RustPython/RustPython · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@youknowone
Copy link
Member
@youknowone youknowone commented Dec 24, 2025

fix #6302

Summary by CodeRabbit

  • New Features
    • Added pthread_sigmask() function for signal mask manipulation on Unix systems.
    • Exposed signal masking constants: SIG_BLOCK, SIG_SETMASK, and SIG_UNBLOCK for use with signal operations.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor
coderabbitai bot commented Dec 24, 2025
📝 Walkthrough

Walkthrough

The changes add support for signal.pthread_sigmask() on Unix systems, enabling applications to manage signal masks across threads. The implementation exposes three constants (SIG_BLOCK, SIG_SETMASK, SIG_UNBLOCK), validates signal masks with range checks, calls the system pthread_sigmask, and returns the previous mask as a Python set.

Changes

Cohort / File(s) Summary
Unix Signal Mask Control
crates/vm/src/stdlib/signal.rs
Added pthread_sigmask() function for managing signal masks across threads, including constants SIG_BLOCK, SIG_SETMASK, SIG_UNBLOCK; introduced helper sigset_to_pyset() for converting system signal sets to Python sets with validation and error handling (Unix only).

Sequence Diagram(s)

sequenceDiagram
    actor User as Python Code
    participant VM as RustPython VM
    participant Sig as Signal Handler
    participant Libc as libc::pthread_sigmask
    
    User->>VM: Call signal.pthread_sigmask(how, mask)
    activate VM
    VM->>Sig: Validate parameters & signals
    Note over Sig: Range check each signal<br/>Build sigset_t
    Sig-->>VM: Validated sigset
    VM->>Libc: Call pthread_sigmask(how, &mask)
    Libc-->>VM: Returns old_mask, status
    alt Success
        VM->>Sig: Convert old_mask to Python set
        Sig-->>VM: Python set of old signals
        VM->>VM: Trigger signal checks
        VM-->>User: Return old_mask as set
    else Error
        VM-->>User: Raise Python exception
    end
    deactivate VM
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A thread weaves signals through the air,
Now pthread masks can block with care,
Old signals dance back in a set,
Xonsh shell won't break a sweat,
One file holds the magic tight! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'signal.pthread_sigmask' clearly and concisely describes the main change—implementation of the pthread_sigmask function in the signal module.
Linked Issues check ✅ Passed The PR implements pthread_sigmask for Unix systems, directly addressing issue #6302 which requires the signal module to provide pthread_sigmask attribute for xonsh compatibility.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing pthread_sigmask functionality in the signal module for Unix systems as required by issue #6302.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c35cb89 and 688cd43.

📒 Files selected for processing (1)
  • crates/vm/src/stdlib/signal.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

  • crates/vm/src/stdlib/signal.rs
🧬 Code graph analysis (1)
crates/vm/src/stdlib/signal.rs (1)
crates/vm/src/vm/mod.rs (1)
  • check_signals (779-788)
🔇 Additional comments (4)
crates/vm/src/stdlib/signal.rs (4)

72-75: LGTM!

The constants are correctly exposed following the same pattern as SIG_DFL and SIG_IGN on lines 69-70.


408-419: LGTM!

Using sigfillset followed by sigismember iteration is the correct approach to determine valid signals on Unix. The error handling for sigfillset failure is appropriate.


443-455: LGTM!

The helper correctly converts a sigset_t to a Python set. The iteration range 1..NSIG ensures valid signal numbers are passed to sigismember.


457-510: LGTM!

The implementation correctly:

  1. Validates signal numbers are in range [1, NSIG)
  2. Handles pthread_sigmask error codes properly (it returns the error directly, not via errno)
  3. Converts the old mask to a Python set using the helper
  4. Calls check_signals after modifying the mask, consistent with raise_signal

The how parameter validation is delegated to pthread_sigmask itself, which will return EINVAL for invalid values—matching CPython's behavior.


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 and usage tips.

@youknowone youknowone marked this pull request as ready for review December 24, 2025 04:09
@youknowone youknowone merged commit 215c5c6 into RustPython:main Dec 24, 2025
13 checks passed
@youknowone youknowone deleted the pthread_sigmask branch December 24, 2025 04:15
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.

AttributeError: module 'signal' has no attribute 'pthread_sigmask'

1 participant

0