refactor signal handling, and clear sigmask before spawning

src/main.rs

@@ -9,7 +9,6 @@ use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::{env, mem};
 
-use calloop::signals::{Signal, Signals};
 use calloop::EventLoop;
 use clap::{CommandFactory, Parser};
 use clap_complete::Shell;
@@ -197,16 +196,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
     let mut event_loop = EventLoop::<State>::try_new().unwrap();
 
     // Handle Ctrl+C and other signals.
-    event_loop
+    niri::utils::signals::listen(&event_loop.handle());
-        .handle()
-        .insert_source(
-            Signals::new(&[Signal::SIGINT, Signal::SIGTERM, Signal::SIGHUP]).unwrap(),
-            |event, _, state| {
-                info!("quitting due to receiving signal {:?}", event.signal());
-                state.niri.stop_signal.stop();
-            },
-        )
-        .unwrap();
 
     // Create the compositor.
     let display = Display::new().unwrap();

src/utils/mod.rs

@@ -34,6 +34,7 @@ use crate::niri::ClientState;
 
 pub mod id;
 pub mod scale;
+pub mod signals;
 pub mod spawning;
 pub mod transaction;
 pub mod watcher;

src/utils/signals.rs

@@ -0,0 +1,57 @@
+//! We set a signal handler with `calloop::signals::Signals::new`.
+//! This does two things:
+//! 1. It blocks the thread from receiving these signals normally (pthread_sigmask)
+//! 2. It creates a signalfd to read them in the event loop.
+//!
+//! When spawning children, calloop already deals with the signalfd.
+//! `Signals::new` creates it with CLOEXEC, so it will not be inherited by children.
+//!
+//! But, the sigmask is always inherited, so we want to clear it before spawning children.
+//! That way, we don't affect their normal signal handling.
+//!
+//! In particular, if a child doesn't care about signals, we must not block it from receiving them.
+//!
+//! This module provides functions to clear the sigmask. Call them before spawning children.
+//!
+//! Technically, a "more correct" solution would be to remember the original sigmask and restore it
+//! after the child exits, but that's painful *and* likely to cause issues, because the user almost
+//! never intended to spawn niri with a nonempty sigmask. It indicates a bug in whoever spawned us,
+//! so we may as well clean up after them (which is easier than not doing so).
+
+use std::{io, mem};
+
+use calloop::signals::{Signal, Signals};
+
+pub fn listen(handle: &calloop::LoopHandle<crate::niri::State>) {
+    handle
+        .insert_source(
+            Signals::new(&[Signal::SIGINT, Signal::SIGTERM, Signal::SIGHUP]).unwrap(),
+            |event, _, state| {
+                info!("quitting due to receiving signal {:?}", event.signal());
+                state.niri.stop_signal.stop();
+            },
+        )
+        .unwrap();
+}
+
+pub fn unblock_all() -> io::Result<()> {
+    set_sigmask(&empty_sigset()?)
+}
+
+pub fn empty_sigset() -> io::Result<libc::sigset_t> {
+    let mut sigset = mem::MaybeUninit::uninit();
+    if unsafe { libc::sigemptyset(sigset.as_mut_ptr()) } == 0 {
+        Ok(unsafe { sigset.assume_init() })
+    } else {
+        Err(io::Error::last_os_error())
+    }
+}
+
+pub fn set_sigmask(set: &libc::sigset_t) -> io::Result<()> {
+    let oldset = std::ptr::null_mut(); // ignore old mask
+    if unsafe { libc::pthread_sigmask(libc::SIG_SETMASK, set, oldset) } == 0 {
+        Ok(())
+    } else {
+        Err(io::Error::last_os_error())
+    }
+}

src/utils/spawning.rs

@@ -141,6 +141,8 @@ fn spawn_sync(
         process.env("DESKTOP_STARTUP_ID", token.as_str());
     }
 
+    unsafe { process.pre_exec(crate::utils::signals::unblock_all) };
+
     let Some(mut child) = do_spawn(command, process) else {
         return;
     };

src/utils/xwayland/satellite.rs

@@ -238,6 +238,8 @@ fn spawn(path: String, xwl: &Satellite) {
         .stdout(Stdio::null())
         .stderr(Stdio::null());
 
+    unsafe { process.pre_exec(crate::utils::signals::unblock_all) };
+
     // Spawning and waiting takes some milliseconds, so do it in a thread.
     let res = thread::Builder::new()
         .name("Xwl-s Spawner".to_owned())