bread-ecosystem/bread-utils/src/singleton.rs
Breadway 49c63c8ccf bread-utils: fix flaky test isolation around process-global env vars
singleton.rs's tests used to call the real kill() on the current test
process itself (the recorded "other instance" PID was our own, since
tests run single-process) — split try_acquire (side-effect-free) out from
toggle_or_kill (sends the signal) so the concurrency assertions no longer
risk SIGTERM-ing the test binary.

Separately, hypr.rs's env-var test mutates HYPRLAND_INSTANCE_SIGNATURE/
XDG_RUNTIME_DIR process-globally; cargo runs tests in parallel threads by
default, so it could race a concurrently-running singleton or xdg test
expecting the real XDG_RUNTIME_DIR, intermittently failing them with
ENOENT. Added a shared env_test_lock() all env-var-touching tests now
acquire for their duration.

Verified via 5 repeated full test runs with zero flakes (28/28 passing
each time).
2026-07-17 10:02:42 +08:00

209 lines
8.3 KiB
Rust

//! Correct single-instance / PID-toggle, replacing the TOCTOU-prone pattern
//! duplicated in `breadbox/src/main.rs` (`toggle_or_continue`/`pid_file`,
//! ~30 lines) and `breadclip/src/main.rs` (same function names, whose own
//! comment reads `// ---- PID file toggle (single-instance, matches breadbox
//! pattern) ----`).
//!
//! The old pattern: read the PID file, `/proc/<pid>/comm`-check whether it's
//! still this app, `kill` it if so, otherwise `fs::write` our own PID over
//! it. That's three separate, non-atomic steps — two instances launched at
//! once can both read "no valid PID" and both proceed as the "first"
//! instance; a stale PID file left by a crash can also collide with an
//! unrelated process that was later assigned the same PID by the kernel,
//! sending it a `kill` it never asked for.
//!
//! This module instead holds an exclusive, kernel-atomic advisory lock
//! (`std::fs::File::try_lock`, i.e. `flock(2)`) on the PID file for the
//! entire lifetime of the process that acquires it. Lock ownership itself
//! *is* the liveness check — there is no window where two processes can
//! both believe they're the sole instance, and a crashed process's lock is
//! released by the kernel the instant it dies, so there's no stale-lock
//! case to reason about at all.
//!
//! [`try_acquire`] is the side-effect-free primitive (no signals sent);
//! [`toggle_or_kill`] layers breadbox/breadclip's actual desired behavior
//! (kill whoever's running, then exit) on top of it.
use std::fs::{File, OpenOptions};
use std::io::{Read, Seek, SeekFrom, Write};
use std::path::PathBuf;
/// Held for the lifetime of the running instance. Dropping it releases the
/// flock and removes the PID file. Keep this alive (e.g. in a `let _guard =
/// ...` bound in `main`) for as long as the app should be considered "the"
/// running instance.
pub struct Guard {
_file: File,
path: PathBuf,
}
impl Drop for Guard {
fn drop(&mut self) {
let _ = std::fs::remove_file(&self.path);
}
}
pub enum Acquire {
/// No other instance was running; we now hold the lock.
Acquired(Guard),
/// Another instance already holds the lock and is therefore alive right
/// now. Carries whatever PID it last recorded, if the file contents
/// parsed as one.
HeldByOther(Option<u32>),
}
pub enum Toggle {
/// No other instance was running; we now hold the lock. Keep the guard
/// alive for the process's lifetime.
Started(Guard),
/// Another instance was already running and (if a PID could be read
/// from the file) has been sent `SIGTERM`. The caller should exit
/// immediately without starting.
KilledExisting,
}
/// `$XDG_RUNTIME_DIR/<app>.pid` (falling back to `/tmp`, matching every
/// existing consumer's own fallback) — same location `breadbox`/`breadclip`
/// already used.
pub fn pid_file_path(app: &str) -> PathBuf {
crate::xdg::runtime_dir().join(format!("{app}.pid"))
}
/// Try to become the single instance of `app`, with no side effects beyond
/// the lock/file itself — in particular, unlike [`toggle_or_kill`], this
/// never signals another process. Prefer this if your app wants different
/// behavior than "kill the existing instance" (e.g. just refuse to start a
/// second copy).
pub fn try_acquire(app: &str) -> std::io::Result<Acquire> {
let path = pid_file_path(app);
let mut file = OpenOptions::new()
.read(true)
.write(true)
.create(true)
.open(&path)?;
match file.try_lock() {
Ok(()) => {
file.set_len(0)?;
file.seek(SeekFrom::Start(0))?;
write!(file, "{}", std::process::id())?;
file.sync_all()?;
Ok(Acquire::Acquired(Guard { _file: file, path }))
}
Err(_) => {
let mut contents = String::new();
let _ = file.read_to_string(&mut contents);
Ok(Acquire::HeldByOther(contents.trim().parse::<u32>().ok()))
}
}
}
/// Toggle behavior: acquire the single-instance lock for `app`. If already
/// held by another live process, signal it to quit (`SIGTERM` via `kill`)
/// and return [`Toggle::KilledExisting`] — the caller should exit. Otherwise
/// take the lock and return [`Toggle::Started`] — the caller should proceed
/// and keep the guard alive.
pub fn toggle_or_kill(app: &str) -> std::io::Result<Toggle> {
Ok(match try_acquire(app)? {
Acquire::Acquired(guard) => Toggle::Started(guard),
Acquire::HeldByOther(Some(pid)) => {
kill(pid);
Toggle::KilledExisting
}
Acquire::HeldByOther(None) => Toggle::KilledExisting,
})
}
#[cfg(unix)]
fn kill(pid: u32) {
// Shells out rather than binding libc directly, matching how every
// existing consumer already did this (`Command::new("kill")`) — no new
// dependency for a one-shot signal.
let _ = std::process::Command::new("kill")
.arg(pid.to_string())
.status();
}
#[cfg(not(unix))]
fn kill(_pid: u32) {}
#[cfg(test)]
mod tests {
use super::*;
use std::time::Duration;
fn unique_app(name: &str) -> String {
format!("bread-utils-singleton-test-{name}-{}", std::process::id())
}
#[test]
fn first_acquire_succeeds_and_releases_on_drop() {
// Guards against `hypr`'s env-var test concurrently pointing
// XDG_RUNTIME_DIR at a nonexistent path mid-test — see `env_test_lock`.
let _lock = crate::env_test_lock().lock().unwrap_or_else(|e| e.into_inner());
let app = unique_app("first");
match try_acquire(&app).unwrap() {
Acquire::Acquired(_guard) => {}
Acquire::HeldByOther(_) => panic!("expected to be the first instance"),
}
// Guard dropped at end of scope; pid file should be gone.
std::thread::sleep(Duration::from_millis(10));
assert!(!pid_file_path(&app).exists());
}
#[test]
fn second_acquire_while_first_is_held_reports_held_by_other_with_our_pid() {
let _lock = crate::env_test_lock().lock().unwrap_or_else(|e| e.into_inner());
let app = unique_app("second");
let guard = match try_acquire(&app).unwrap() {
Acquire::Acquired(g) => g,
Acquire::HeldByOther(_) => panic!("expected to be the first instance"),
};
// A second attempt while the first guard is still held must not be
// able to acquire the lock too — that's the whole point. No signal
// is sent by `try_acquire` itself (that's `toggle_or_kill`'s job),
// so this is safe to assert without affecting the test process.
match try_acquire(&app).unwrap() {
Acquire::HeldByOther(pid) => assert_eq!(pid, Some(std::process::id())),
Acquire::Acquired(_) => panic!("second acquire succeeded while the first still holds the lock"),
}
drop(guard);
}
#[test]
fn lock_is_released_after_guard_drop_so_a_later_instance_can_acquire() {
let _lock = crate::env_test_lock().lock().unwrap_or_else(|e| e.into_inner());
let app = unique_app("release");
let guard = match try_acquire(&app).unwrap() {
Acquire::Acquired(g) => g,
Acquire::HeldByOther(_) => panic!("expected to be the first instance"),
};
drop(guard);
match try_acquire(&app).unwrap() {
Acquire::Acquired(_g) => {}
Acquire::HeldByOther(_) => panic!("lock should have been released when the guard was dropped"),
}
}
#[test]
fn toggle_or_kill_starts_when_nothing_else_is_running() {
let _lock = crate::env_test_lock().lock().unwrap_or_else(|e| e.into_inner());
let app = unique_app("toggle-start");
match toggle_or_kill(&app).unwrap() {
Toggle::Started(_guard) => {}
Toggle::KilledExisting => panic!("expected to start as the first instance"),
}
}
// Deliberately not unit-tested: `toggle_or_kill`'s kill-the-existing-
// instance branch. Exercising it for real means sending a real SIGTERM
// to a real process; the only PID a test process can safely target is
// its own (as a stand-in "other instance" via a shared PID file), and
// doing that would SIGTERM the test binary itself. The branch is a
// two-line, directly-inspectable call to `kill()` gated on
// `HeldByOther(Some(pid))`, which the `second_acquire_...` test above
// already exercises up to (and excluding) the signal send.
}