From a2752a5bd31bd4c4a818eaa4863320814d19e661 Mon Sep 17 00:00:00 2001 From: "Pedro M. de Echanove Pasquin" Date: Tue, 25 Aug 2026 09:47:08 +0200 Subject: [PATCH] =?UTF-8?q?Handle=20SIGTERM/SIGINT=20with=20a=20sigaction?= =?UTF-8?q?=20self-pipe=20instead=20of=20a=20signalfd,=20and=20clear=20the?= =?UTF-8?q?=20signal=20mask=20of=20spawned=20children=20The=20signalfd-bas?= =?UTF-8?q?ed=20source=20(calloop's=20`signals`=20feature)=20needs=20SIGTE?= =?UTF-8?q?RM=20and=20SIGINT=20blocked=20in=20every=20thread=20of=20the=20?= =?UTF-8?q?process,=20since=20the=20kernel=20hands=20a=20process-directed?= =?UTF-8?q?=20signal=20to=20any=20thread=20that=20does=20not=20block=20it.?= =?UTF-8?q?=20calloop=20only=20blocks=20them=20in=20the=20thread=20that=20?= =?UTF-8?q?creates=20the=20source,=20so=20the=20runtime=20depended=20on=20?= =?UTF-8?q?`ltk::run`=20being=20entered=20before=20the=20app=20spawned=20a?= =?UTF-8?q?ny=20thread=20=E2=80=94=20a=20constraint=20apps=20do=20not=20ho?= =?UTF-8?q?nour:=20crustace-notifier=20builds=20its=20tokio=20runtime=20an?= =?UTF-8?q?d=20its=20D-Bus=20thread=20first,=20and=20in=20that=20configura?= =?UTF-8?q?tion=20a=20SIGTERM=20lands=20on=20a=20tokio=20worker=20and=20ki?= =?UTF-8?q?lls=20the=20process=20outright,=20skipping=20the=20clean=20exit?= =?UTF-8?q?=20(`save=5Fstate`,=20`mark=5Fclean=5Fexit`)=20entirely.=20The?= =?UTF-8?q?=20blocked=20mask=20had=20a=20second=20consequence:=20it=20is?= =?UTF-8?q?=20inherited=20across=20fork=20and=20exec,=20and=20Rust's=20std?= =?UTF-8?q?=20deliberately=20does=20not=20reset=20it=20in=20the=20child.?= =?UTF-8?q?=20Every=20process=20an=20ltk=20app=20spawns=20from=20a=20threa?= =?UTF-8?q?d=20created=20after=20`ltk::run`=20therefore=20started=20with?= =?UTF-8?q?=20SIGTERM=20blocked.=20For=20the=20`gsettings=20monitor`=20chi?= =?UTF-8?q?ld=20of=20the=20text-scale=20watcher=20this=20meant=20it=20neve?= =?UTF-8?q?r=20died=20on=20systemd's=20stop,=20so=20the=20unit's=20cgroup?= =?UTF-8?q?=20stayed=20populated=20until=20`TimeoutStopSec`=20(90=20s)=20e?= =?UTF-8?q?xpired=20and=20systemd=20resorted=20to=20SIGKILL=20=E2=80=94=20?= =?UTF-8?q?visible=20as=20a=20stop=20job=20holding=20up=20every=20session?= =?UTF-8?q?=20shutdown.=20`install=5Fsignal=5Fsource`=20now=20installs=20a?= =?UTF-8?q?=20`sigaction`=20handler=20for=20SIGTERM=20and=20SIGINT=20that?= =?UTF-8?q?=20writes=20the=20signal=20number=20to=20a=20non-blocking=20`O?= =?UTF-8?q?=5FCLOEXEC`=20pipe=20(an=20atomic=20load=20and=20a=20`write`,?= =?UTF-8?q?=20errno=20preserved);=20the=20read=20end=20is=20a=20calloop=20?= =?UTF-8?q?`Generic`=20source=20that=20drains=20the=20pipe=20and=20sets=20?= =?UTF-8?q?`exit=5Frequested`.=20A=20handler=20is=20process-wide,=20so=20t?= =?UTF-8?q?he=20clean=20exit=20works=20no=20matter=20which=20thread=20rece?= =?UTF-8?q?ives=20the=20signal=20or=20when=20it=20was=20created,=20and=20n?= =?UTF-8?q?o=20thread=20blocks=20anything,=20so=20children=20inherit=20a?= =?UTF-8?q?=20clean=20mask.=20`SA=5FRESTART`=20keeps=20the=20handler=20fro?= =?UTF-8?q?m=20injecting=20spurious=20EINTRs=20into=20the=20app's=20own=20?= =?UTF-8?q?threads;=20the=20pipe=20wakes=20the=20event=20loop=20regardless?= =?UTF-8?q?.=20`text=5Fscale`=20additionally=20clears=20the=20child's=20ma?= =?UTF-8?q?sk=20in=20`pre=5Fexec`=20before=20running=20`gsettings`,=20as?= =?UTF-8?q?=20a=20guard=20against=20a=20mask=20already=20dirty=20when=20th?= =?UTF-8?q?e=20process=20was=20launched.=20The=20`save=5Fstate`=20docs=20d?= =?UTF-8?q?rop=20the=20caveat=20about=20threads=20started=20before=20`run`?= =?UTF-8?q?,=20the=20ordering=20comment=20at=20the=20call=20site=20goes=20?= =?UTF-8?q?with=20the=20constraint=20it=20described,=20and=20the=20`signal?= =?UTF-8?q?s`=20feature=20is=20dropped=20from=20calloop.=20`libc`=20become?= =?UTF-8?q?s=20a=20direct=20dependency=20for=20`sigaction`,=20`pipe2`=20an?= =?UTF-8?q?d=20`sigprocmask`.=20Tests:=20`child=5Fstarts=5Fwith=5Fclear=5F?= =?UTF-8?q?signal=5Fmask`=20blocks=20SIGTERM=20in=20the=20test=20thread,?= =?UTF-8?q?=20spawns=20a=20child=20through=20the=20helper=20and=20checks?= =?UTF-8?q?=20its=20`SigBlk`=20in=20`/proc`,=20with=20the=20inverse=20cont?= =?UTF-8?q?rol=20asserting=20that=20std=20really=20does=20inherit=20the=20?= =?UTF-8?q?mask=20(the=20reason=20the=20helper=20exists);=20`handler=5Fwri?= =?UTF-8?q?tes=5Fsignal=5Fto=5Fpipe`=20exercises=20the=20handler=20directl?= =?UTF-8?q?y=20and=20through=20a=20real=20`sigaction`=20+=20`raise`,=20the?= =?UTF-8?q?n=20restores=20`SIG=5FDFL`.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Cargo.toml | 3 +- src/app.rs | 7 +- src/event_loop/run.rs | 2 - src/event_loop/session.rs | 132 ++++++++++++++++++++++++++++++++--- src/event_loop/text_scale.rs | 71 ++++++++++++++++--- 5 files changed, 192 insertions(+), 23 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bb500b0..79d0539 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,7 @@ chrono = { version = "0.4", features = ["clock"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" smithay-client-toolkit = { version = "0.20", features = ["calloop", "calloop-wayland-source", "xkbcommon"] } -calloop = { version = "0.14", features = ["signals"] } +calloop = "0.14" calloop-wayland-source = "0.4" tiny-skia = "0.12" fontdue = "=0.9.3" @@ -46,6 +46,7 @@ rust-i18n = "3" # Transitive dep of rust-i18n (globwalk → ignore); 0.4.24+ needs a rustc # newer than Debian's 1.85, and its manifest does not declare that MSRV. ignore = "=0.4.23" +libc = "0.2" wayland-protocols = { version = "0.32", features = ["client", "unstable", "staging"] } wayland-egl = "0.32" wayland-scanner = "0.31" diff --git a/src/app.rs b/src/app.rs index 6bc2103..bce75e5 100644 --- a/src/app.rs +++ b/src/app.rs @@ -419,10 +419,9 @@ pub trait App: 'static /// The file is plain on disk: never put secrets in it (passwords /// belong to the secure-mode `text_edit` and its wiped buffers), /// and keep large blobs (images, caches) elsewhere. The signal - /// handler only covers threads started after [`run`](crate::run) - /// was entered; a thread the app spawned earlier that receives - /// `SIGTERM` still terminates the process — block the signal in - /// such threads or start them from inside the app. + /// handler is installed process-wide when [`run`](crate::run) is + /// entered, so the clean exit holds no matter which thread receives + /// the signal or when it was started. /// /// [`restore_state`](Self::restore_state) explains when — and when /// not — the bytes come back. diff --git a/src/event_loop/run.rs b/src/event_loop/run.rs index 0703716..e883c91 100644 --- a/src/event_loop/run.rs +++ b/src/event_loop/run.rs @@ -66,8 +66,6 @@ pub( crate ) fn try_run( mut app: A ) -> Result<(), RunError> .insert( event_loop.handle() ) .map_err( |e| RunError::EventLoop( format!( "WaylandSource::insert: {e:?}" ) ) )?; - // Before any thread exists: signalfd only sees signals blocked in the - // thread that created it, and the mask is inherited by later threads. super::session::install_signal_source( &event_loop.handle() )?; let compositor = CompositorState::bind( &globals, &qh ) diff --git a/src/event_loop/session.rs b/src/event_loop/session.rs index 71a8a78..2535d8b 100644 --- a/src/event_loop/session.rs +++ b/src/event_loop/session.rs @@ -4,6 +4,8 @@ //! `xdg-session-management-v1` client glue plus the runtime hooks that //! persist `App::save_state` and turn signals into a clean exit. +use std::os::fd::{ AsRawFd, FromRawFd, OwnedFd }; +use std::sync::atomic::{ AtomicI32, Ordering }; use std::time::Duration; use calloop::timer::{ TimeoutAction, Timer }; @@ -144,18 +146,81 @@ impl SessionRuntime } } +static SIGNAL_PIPE_WR: AtomicI32 = AtomicI32::new( -1 ); + +/// Async-signal-safe: an atomic load and a `write`, errno preserved. +extern "C" fn on_termination_signal( sig: libc::c_int ) +{ + unsafe + { + let errno = *libc::__errno_location(); + let fd = SIGNAL_PIPE_WR.load( Ordering::Acquire ); + if fd >= 0 + { + let byte = sig as u8; + let _ = libc::write( fd, ( &byte as *const u8 ).cast(), 1 ); + } + *libc::__errno_location() = errno; + } +} + +/// Self-pipe rather than a signalfd: a `sigaction` handler is process- +/// wide, so the clean exit works no matter which thread the kernel +/// picks, without blocking the signal in every thread — a mask that +/// children would inherit across exec. pub( crate ) fn install_signal_source( handle: &LoopHandle<'static, AppData> ) -> Result<(), RunError> { - use calloop::signals::{ Signal, Signals }; - let signals = Signals::new( &[ Signal::SIGTERM, Signal::SIGINT ] ) - .map_err( |e| RunError::EventLoop( format!( "Signals::new: {e}" ) ) )?; - handle - .insert_source( signals, |event, _, data: &mut AppData| + use calloop::generic::Generic; + use calloop::{ Interest, Mode, PostAction }; + + let mut fds = [ -1i32; 2 ]; + if unsafe { libc::pipe2( fds.as_mut_ptr(), libc::O_CLOEXEC | libc::O_NONBLOCK ) } != 0 + { + return Err( RunError::EventLoop( format!( "signal pipe2: {}", std::io::Error::last_os_error() ) ) ); + } + SIGNAL_PIPE_WR.store( fds[ 1 ], Ordering::Release ); + + unsafe + { + let mut action: libc::sigaction = std::mem::zeroed(); + action.sa_sigaction = on_termination_signal as usize; + action.sa_flags = libc::SA_RESTART; + libc::sigemptyset( &mut action.sa_mask ); + for sig in [ libc::SIGTERM, libc::SIGINT ] { - eprintln!( "ltk: {:?} received, exiting cleanly", event.signal() ); - data.exit_requested = true; + if libc::sigaction( sig, &action, std::ptr::null_mut() ) != 0 + { + return Err( RunError::EventLoop( format!( "sigaction: {}", std::io::Error::last_os_error() ) ) ); + } + } + } + + let read_end = unsafe { OwnedFd::from_raw_fd( fds[ 0 ] ) }; + handle + .insert_source( Generic::new( read_end, Interest::READ, Mode::Level ), |_, fd, data: &mut AppData| + { + let mut sig = None; + let mut buf = [ 0u8; 16 ]; + loop + { + let n = unsafe { libc::read( fd.as_raw_fd(), buf.as_mut_ptr().cast(), buf.len() ) }; + if n <= 0 { break; } + sig = Some( buf[ 0 ] as i32 ); + } + if let Some( sig ) = sig + { + let name = match sig + { + libc::SIGTERM => "SIGTERM", + libc::SIGINT => "SIGINT", + _ => "signal", + }; + eprintln!( "ltk: {name} received, exiting cleanly" ); + data.exit_requested = true; + } + Ok( PostAction::Continue ) } ) - .map_err( |e| RunError::EventLoop( format!( "signals insert_source: {e:?}" ) ) )?; + .map_err( |e| RunError::EventLoop( format!( "signal pipe insert_source: {e:?}" ) ) )?; Ok( () ) } @@ -224,3 +289,54 @@ impl Dispatch for AppData { } } + +#[ cfg( test ) ] +mod tests +{ + use super::*; + + fn read_one( fd: i32 ) -> Option + { + let mut buf = [ 0u8; 4 ]; + let n = unsafe { libc::read( fd, buf.as_mut_ptr().cast(), buf.len() ) }; + ( n == 1 ).then( || buf[ 0 ] ) + } + + #[ test ] + fn handler_writes_signal_to_pipe() + { + let mut fds = [ -1i32; 2 ]; + assert_eq!( unsafe { libc::pipe2( fds.as_mut_ptr(), libc::O_CLOEXEC | libc::O_NONBLOCK ) }, 0 ); + SIGNAL_PIPE_WR.store( fds[ 1 ], Ordering::Release ); + + on_termination_signal( libc::SIGTERM ); + assert_eq!( read_one( fds[ 0 ] ), Some( libc::SIGTERM as u8 ) ); + + // The real wiring: raise() delivers synchronously to this + // thread, so the handler has run before it returns. + unsafe + { + let mut action: libc::sigaction = std::mem::zeroed(); + action.sa_sigaction = on_termination_signal as usize; + action.sa_flags = libc::SA_RESTART; + libc::sigemptyset( &mut action.sa_mask ); + assert_eq!( libc::sigaction( libc::SIGTERM, &action, std::ptr::null_mut() ), 0 ); + libc::raise( libc::SIGTERM ); + } + assert_eq!( read_one( fds[ 0 ] ), Some( libc::SIGTERM as u8 ) ); + + unsafe + { + let mut dfl: libc::sigaction = std::mem::zeroed(); + dfl.sa_sigaction = libc::SIG_DFL; + libc::sigemptyset( &mut dfl.sa_mask ); + libc::sigaction( libc::SIGTERM, &dfl, std::ptr::null_mut() ); + } + SIGNAL_PIPE_WR.store( -1, Ordering::Release ); + unsafe + { + libc::close( fds[ 0 ] ); + libc::close( fds[ 1 ] ); + } + } +} diff --git a/src/event_loop/text_scale.rs b/src/event_loop/text_scale.rs index 3a1830c..328e8f8 100644 --- a/src/event_loop/text_scale.rs +++ b/src/event_loop/text_scale.rs @@ -12,11 +12,28 @@ //! `gsettings` is not installed. use std::io::{ BufRead, BufReader }; +use std::os::unix::process::CommandExt; use std::process::{ Command, Stdio }; const SCHEMA: &str = "org.gnome.desktop.interface"; const KEY: &str = "text-scaling-factor"; +/// A blocked signal mask survives exec, so clear it: a child that +/// inherits SIGTERM blocked outlives the session's shutdown. +fn unblock_signals( cmd: &mut Command ) -> &mut Command +{ + unsafe + { + cmd.pre_exec( || + { + let mut set: libc::sigset_t = std::mem::zeroed(); + libc::sigemptyset( &mut set ); + libc::sigprocmask( libc::SIG_SETMASK, &set, std::ptr::null_mut() ); + Ok( () ) + } ) + } +} + /// Spawn the watcher thread. `tx` delivers each observed factor to the /// run loop; the initial `gsettings get` value is sent first. pub( super ) fn spawn_watcher( tx: calloop::channel::Sender ) @@ -30,10 +47,11 @@ pub( super ) fn spawn_watcher( tx: calloop::channel::Sender ) let _ = tx.send( v ); } - let Ok( mut child ) = Command::new( "gsettings" ) - .args( [ "monitor", SCHEMA, KEY ] ) - .stdout( Stdio::piped() ) - .stderr( Stdio::null() ) + let Ok( mut child ) = unblock_signals( + Command::new( "gsettings" ) + .args( [ "monitor", SCHEMA, KEY ] ) + .stdout( Stdio::piped() ) + .stderr( Stdio::null() ) ) .spawn() else { return }; let Some( stdout ) = child.stdout.take() else { return }; @@ -52,9 +70,10 @@ pub( super ) fn spawn_watcher( tx: calloop::channel::Sender ) fn read_current() -> Option { - let out = Command::new( "gsettings" ) - .args( [ "get", SCHEMA, KEY ] ) - .stderr( Stdio::null() ) + let out = unblock_signals( + Command::new( "gsettings" ) + .args( [ "get", SCHEMA, KEY ] ) + .stderr( Stdio::null() ) ) .output() .ok()?; parse_factor( std::str::from_utf8( &out.stdout ).ok()? ) @@ -73,7 +92,7 @@ fn parse_factor( s: &str ) -> Option #[ cfg( test ) ] mod tests { - use super::parse_factor; + use super::{ parse_factor, unblock_signals }; #[ test ] fn parses_monitor_line_and_bare_value() @@ -83,4 +102,40 @@ mod tests assert_eq!( parse_factor( "" ), None ); assert_eq!( parse_factor( "text-scaling-factor: nope" ), None ); } + + fn spawn_and_read_sigblk( clear: bool ) -> u64 + { + let mut cmd = std::process::Command::new( "sleep" ); + cmd.arg( "30" ); + if clear + { + unblock_signals( &mut cmd ); + } + let mut child = cmd.spawn().unwrap(); + let status = std::fs::read_to_string( format!( "/proc/{}/status", child.id() ) ).unwrap(); + let _ = child.kill(); + let _ = child.wait(); + status.lines() + .find_map( |l| l.strip_prefix( "SigBlk:" ) ) + .and_then( |v| u64::from_str_radix( v.trim(), 16 ).ok() ) + .unwrap() + } + + #[ test ] + fn child_starts_with_clear_signal_mask() + { + unsafe + { + let mut set: libc::sigset_t = std::mem::zeroed(); + libc::sigemptyset( &mut set ); + libc::sigaddset( &mut set, libc::SIGTERM ); + libc::pthread_sigmask( libc::SIG_BLOCK, &set, std::ptr::null_mut() ); + } + let sigterm_bit = 1u64 << ( libc::SIGTERM - 1 ); + + // std deliberately lets the child inherit the mask — the very + // reason unblock_signals exists. + assert_ne!( spawn_and_read_sigblk( false ) & sigterm_bit, 0 ); + assert_eq!( spawn_and_read_sigblk( true ) & sigterm_bit, 0 ); + } }