GeezerVison

Walkthrough

Refactors VM signal handler storage from an Option to a OnceCell-backed lazy container, adds a crate helper to create the handler array, enforces main-thread-only signal operations, and ensures signal handlers are initialized in forked child processes.

Changes

Sequence Diagram(s)

sequenceDiagram
    autonumber
    rect rgba(200,220,255,0.5)
    participant Py as Python code
    participant VM as VirtualMachine
    participant SIG as signal_handlers (OnceCell<Box<RefCell...>>)
    participant OS as OS/Thread
    end
    Py->>VM: call signal.signal(signum, handler)
    VM->>OS: check is_main_thread()
    alt not main thread
        VM-->>Py: raise ValueError
    else main thread
        VM->>SIG: get_or_init(new_signal_handlers)
        SIG->>SIG: borrow & store handler at index
        SIG-->>VM: stored
        VM-->>Py: return old handler
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• impl {raise_,str}signal #6411: Edits crates/vm/src/stdlib/signal.rs with overlapping signal handler access and initialization changes.
• clear_after_fork #6933: Also touches child after-fork initialization for signal state in py_os_after_fork_child.
• Update faulthandler to 3.14.2 #6949: Modifies crates/vm/src/signal.rs and signal processing logic that overlaps with the handler storage changes.

Suggested reviewers

• ShaharNaveh

Poem

🐰 I hopped through OnceCell, neat and spry,
Handlers tucked where they safely lie,
Forked children wake with tidy stacks,
Main threads guard the signal tracks,
A little rabbit cheers — hop, apply! ✨

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

• 📝 Generate docstrings

crates/vm/src/stdlib/signal.rs (1)

254-262: getsignal() lazily initializes signal handlers on non-main threads — consider using get() instead.

When getsignal is called on a child thread's VM (where OnceCell is empty), get_or_init allocates a new [Option<PyObjectRef>; NSIG] array just to return None for every signal. Using get() with a fallback would avoid this unnecessary allocation:

Suggested change

 fn getsignal(signalnum: i32, vm: &VirtualMachine) -> PyResult {
     signal::assert_in_range(signalnum, vm)?;
-    let signal_handlers = vm.signal_handlers.get_or_init(signal::new_signal_handlers);
-    let handler = signal_handlers.borrow()[signalnum as usize]
-        .clone()
-        .unwrap_or_else(|| vm.ctx.none());
-    Ok(handler)
+    let handler = vm
+        .signal_handlers
+        .get()
+        .and_then(|h| h.borrow()[signalnum as usize].clone())
+        .unwrap_or_else(|| vm.ctx.none());
+    Ok(handler)
 }

• X
• Mastodon
• Reddit
• LinkedIn