Handle SIGTERM/SIGINT with a sigaction self-pipe instead of a signalfd, and clear the signal mask of spawned children
The signalfd-based source (calloop's `signals` feature) needs SIGTERM and SIGINT blocked in every thread of the process, since the kernel hands a process-directed signal to any thread that does not block it. calloop only blocks them in the thread that creates the source, so the runtime depended on `ltk::run` being entered before the app spawned any thread — a constraint apps do not honour: crustace-notifier builds its tokio runtime and its D-Bus thread first, and in that configuration a SIGTERM lands on a tokio worker and kills the process outright, skipping the clean exit (`save_state`, `mark_clean_exit`) entirely. The blocked mask had a second consequence: it is inherited across fork and exec, and Rust's std deliberately does not reset it in the child. Every process an ltk app spawns from a thread created after `ltk::run` therefore started with SIGTERM blocked. For the `gsettings monitor` child of the text-scale watcher this meant it never died on systemd's stop, so the unit's cgroup stayed populated until `TimeoutStopSec` (90 s) expired and systemd resorted to SIGKILL — visible as a stop job holding up every session shutdown. `install_signal_source` now installs a `sigaction` handler for SIGTERM and SIGINT that writes the signal number to a non-blocking `O_CLOEXEC` pipe (an atomic load and a `write`, errno preserved); the read end is a calloop `Generic` source that drains the pipe and sets `exit_requested`. A handler is process-wide, so the clean exit works no matter which thread receives the signal or when it was created, and no thread blocks anything, so children inherit a clean mask. `SA_RESTART` keeps the handler from injecting spurious EINTRs into the app's own threads; the pipe wakes the event loop regardless. `text_scale` additionally clears the child's mask in `pre_exec` before running `gsettings`, as a guard against a mask already dirty when the process was launched. The `save_state` docs drop the caveat about threads started before `run`, the ordering comment at the call site goes with the constraint it described, and the `signals` feature is dropped from calloop. `libc` becomes a direct dependency for `sigaction`, `pipe2` and `sigprocmask`. Tests: `child_starts_with_clear_signal_mask` blocks SIGTERM in the test thread, spawns a child through the helper and checks its `SigBlk` in `/proc`, with the inverse control asserting that std really does inherit the mask (the reason the helper exists); `handler_writes_signal_to_pipe` exercises the handler directly and through a real `sigaction` + `raise`, then restores `SIG_DFL`.
This commit is contained in:
@@ -34,7 +34,7 @@ chrono = { version = "0.4", features = ["clock"] }
|
|||||||
serde = { version = "1.0", features = ["derive"] }
|
serde = { version = "1.0", features = ["derive"] }
|
||||||
serde_json = "1.0"
|
serde_json = "1.0"
|
||||||
smithay-client-toolkit = { version = "0.20", features = ["calloop", "calloop-wayland-source", "xkbcommon"] }
|
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"
|
calloop-wayland-source = "0.4"
|
||||||
tiny-skia = "0.12"
|
tiny-skia = "0.12"
|
||||||
fontdue = "=0.9.3"
|
fontdue = "=0.9.3"
|
||||||
@@ -46,6 +46,7 @@ rust-i18n = "3"
|
|||||||
# Transitive dep of rust-i18n (globwalk → ignore); 0.4.24+ needs a rustc
|
# 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.
|
# newer than Debian's 1.85, and its manifest does not declare that MSRV.
|
||||||
ignore = "=0.4.23"
|
ignore = "=0.4.23"
|
||||||
|
libc = "0.2"
|
||||||
wayland-protocols = { version = "0.32", features = ["client", "unstable", "staging"] }
|
wayland-protocols = { version = "0.32", features = ["client", "unstable", "staging"] }
|
||||||
wayland-egl = "0.32"
|
wayland-egl = "0.32"
|
||||||
wayland-scanner = "0.31"
|
wayland-scanner = "0.31"
|
||||||
|
|||||||
@@ -419,10 +419,9 @@ pub trait App: 'static
|
|||||||
/// The file is plain on disk: never put secrets in it (passwords
|
/// The file is plain on disk: never put secrets in it (passwords
|
||||||
/// belong to the secure-mode `text_edit` and its wiped buffers),
|
/// belong to the secure-mode `text_edit` and its wiped buffers),
|
||||||
/// and keep large blobs (images, caches) elsewhere. The signal
|
/// and keep large blobs (images, caches) elsewhere. The signal
|
||||||
/// handler only covers threads started after [`run`](crate::run)
|
/// handler is installed process-wide when [`run`](crate::run) is
|
||||||
/// was entered; a thread the app spawned earlier that receives
|
/// entered, so the clean exit holds no matter which thread receives
|
||||||
/// `SIGTERM` still terminates the process — block the signal in
|
/// the signal or when it was started.
|
||||||
/// such threads or start them from inside the app.
|
|
||||||
///
|
///
|
||||||
/// [`restore_state`](Self::restore_state) explains when — and when
|
/// [`restore_state`](Self::restore_state) explains when — and when
|
||||||
/// not — the bytes come back.
|
/// not — the bytes come back.
|
||||||
|
|||||||
@@ -66,8 +66,6 @@ pub( crate ) fn try_run<A: App>( mut app: A ) -> Result<(), RunError>
|
|||||||
.insert( event_loop.handle() )
|
.insert( event_loop.handle() )
|
||||||
.map_err( |e| RunError::EventLoop( format!( "WaylandSource::insert: {e:?}" ) ) )?;
|
.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() )?;
|
super::session::install_signal_source( &event_loop.handle() )?;
|
||||||
|
|
||||||
let compositor = CompositorState::bind( &globals, &qh )
|
let compositor = CompositorState::bind( &globals, &qh )
|
||||||
|
|||||||
@@ -4,6 +4,8 @@
|
|||||||
//! `xdg-session-management-v1` client glue plus the runtime hooks that
|
//! `xdg-session-management-v1` client glue plus the runtime hooks that
|
||||||
//! persist `App::save_state` and turn signals into a clean exit.
|
//! 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 std::time::Duration;
|
||||||
|
|
||||||
use calloop::timer::{ TimeoutAction, Timer };
|
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<A: App>( handle: &LoopHandle<'static, AppData<A>> ) -> Result<(), RunError>
|
pub( crate ) fn install_signal_source<A: App>( handle: &LoopHandle<'static, AppData<A>> ) -> Result<(), RunError>
|
||||||
{
|
{
|
||||||
use calloop::signals::{ Signal, Signals };
|
use calloop::generic::Generic;
|
||||||
let signals = Signals::new( &[ Signal::SIGTERM, Signal::SIGINT ] )
|
use calloop::{ Interest, Mode, PostAction };
|
||||||
.map_err( |e| RunError::EventLoop( format!( "Signals::new: {e}" ) ) )?;
|
|
||||||
handle
|
let mut fds = [ -1i32; 2 ];
|
||||||
.insert_source( signals, |event, _, data: &mut AppData<A>|
|
if unsafe { libc::pipe2( fds.as_mut_ptr(), libc::O_CLOEXEC | libc::O_NONBLOCK ) } != 0
|
||||||
{
|
{
|
||||||
eprintln!( "ltk: {:?} received, exiting cleanly", event.signal() );
|
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 ]
|
||||||
|
{
|
||||||
|
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<A>|
|
||||||
|
{
|
||||||
|
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;
|
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( () )
|
Ok( () )
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -224,3 +289,54 @@ impl<A: App> Dispatch<XdgToplevelSessionV1, ()> for AppData<A>
|
|||||||
{
|
{
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[ cfg( test ) ]
|
||||||
|
mod tests
|
||||||
|
{
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
fn read_one( fd: i32 ) -> Option<u8>
|
||||||
|
{
|
||||||
|
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 ] );
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -12,11 +12,28 @@
|
|||||||
//! `gsettings` is not installed.
|
//! `gsettings` is not installed.
|
||||||
|
|
||||||
use std::io::{ BufRead, BufReader };
|
use std::io::{ BufRead, BufReader };
|
||||||
|
use std::os::unix::process::CommandExt;
|
||||||
use std::process::{ Command, Stdio };
|
use std::process::{ Command, Stdio };
|
||||||
|
|
||||||
const SCHEMA: &str = "org.gnome.desktop.interface";
|
const SCHEMA: &str = "org.gnome.desktop.interface";
|
||||||
const KEY: &str = "text-scaling-factor";
|
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
|
/// Spawn the watcher thread. `tx` delivers each observed factor to the
|
||||||
/// run loop; the initial `gsettings get` value is sent first.
|
/// run loop; the initial `gsettings get` value is sent first.
|
||||||
pub( super ) fn spawn_watcher( tx: calloop::channel::Sender<f32> )
|
pub( super ) fn spawn_watcher( tx: calloop::channel::Sender<f32> )
|
||||||
@@ -30,10 +47,11 @@ pub( super ) fn spawn_watcher( tx: calloop::channel::Sender<f32> )
|
|||||||
let _ = tx.send( v );
|
let _ = tx.send( v );
|
||||||
}
|
}
|
||||||
|
|
||||||
let Ok( mut child ) = Command::new( "gsettings" )
|
let Ok( mut child ) = unblock_signals(
|
||||||
|
Command::new( "gsettings" )
|
||||||
.args( [ "monitor", SCHEMA, KEY ] )
|
.args( [ "monitor", SCHEMA, KEY ] )
|
||||||
.stdout( Stdio::piped() )
|
.stdout( Stdio::piped() )
|
||||||
.stderr( Stdio::null() )
|
.stderr( Stdio::null() ) )
|
||||||
.spawn()
|
.spawn()
|
||||||
else { return };
|
else { return };
|
||||||
let Some( stdout ) = child.stdout.take() else { return };
|
let Some( stdout ) = child.stdout.take() else { return };
|
||||||
@@ -52,9 +70,10 @@ pub( super ) fn spawn_watcher( tx: calloop::channel::Sender<f32> )
|
|||||||
|
|
||||||
fn read_current() -> Option<f32>
|
fn read_current() -> Option<f32>
|
||||||
{
|
{
|
||||||
let out = Command::new( "gsettings" )
|
let out = unblock_signals(
|
||||||
|
Command::new( "gsettings" )
|
||||||
.args( [ "get", SCHEMA, KEY ] )
|
.args( [ "get", SCHEMA, KEY ] )
|
||||||
.stderr( Stdio::null() )
|
.stderr( Stdio::null() ) )
|
||||||
.output()
|
.output()
|
||||||
.ok()?;
|
.ok()?;
|
||||||
parse_factor( std::str::from_utf8( &out.stdout ).ok()? )
|
parse_factor( std::str::from_utf8( &out.stdout ).ok()? )
|
||||||
@@ -73,7 +92,7 @@ fn parse_factor( s: &str ) -> Option<f32>
|
|||||||
#[ cfg( test ) ]
|
#[ cfg( test ) ]
|
||||||
mod tests
|
mod tests
|
||||||
{
|
{
|
||||||
use super::parse_factor;
|
use super::{ parse_factor, unblock_signals };
|
||||||
|
|
||||||
#[ test ]
|
#[ test ]
|
||||||
fn parses_monitor_line_and_bare_value()
|
fn parses_monitor_line_and_bare_value()
|
||||||
@@ -83,4 +102,40 @@ mod tests
|
|||||||
assert_eq!( parse_factor( "" ), None );
|
assert_eq!( parse_factor( "" ), None );
|
||||||
assert_eq!( parse_factor( "text-scaling-factor: nope" ), 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 );
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user