From 4e46604cfdbe0cebe67492e8bf2018a298ed3f59 Mon Sep 17 00:00:00 2001 From: S4NKALP Date: Mon, 25 May 2026 13:36:16 +0545 Subject: [PATCH] =?UTF-8?q?feat(src/commands):=20implement=20atomic=20inte?= =?UTF-8?q?rrupt=20handling,=20display=E2=80=91manager=20guard,=20lock=20f?= =?UTF-8?q?ile=20for=20operation=20serialization,=20safe=20cache=20writes,?= =?UTF-8?q?=20and=20improve=20daemon=20enable=20messaging?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The changes add robust interruption support, exclusive‑lock enforcement, and safer file handling to the GPU switching commands, while clarifying installation output. - New Features - Atomic `INTERRUPTED` flag with Ctrl‑C signal handler - `DmGuard` struct to manage display‑manager start/stop lifecycle - `LockGuard` with exclusive lock file (`/var/lock/lapctl.lock`) to serialize GPU switches - Safe temporary‑file write + rename for cache updates - Enhancements - Unified `prepare_no_reboot` returns `DmGuard` instead of `Option` - Consistent early‑exit handling with lock acquisition checks - Improved debug/info logging for DM actions - Restructured `finalize_no_reboot` to return a guard value - Clarified installation success/failure messages for `lapctld` - Bug Fixes - Prevented race conditions on concurrent GPU switches by locking before operation - Ensured lock is released on early return paths - Fixed incomplete `finalize_no_reboot` handling - Corrected daemon enable detection and user feedback logic --- src/commands/gpu.rs | 143 +++++++++++++++++++++++++--------- src/commands/install_rules.rs | 15 +++- 2 files changed, 120 insertions(+), 38 deletions(-) diff --git a/src/commands/gpu.rs b/src/commands/gpu.rs index e2f656b..8836e5c 100644 --- a/src/commands/gpu.rs +++ b/src/commands/gpu.rs @@ -7,14 +7,59 @@ use crate::utils::system::{ assert_root, create_file, get_active_graphical_sessions, get_display_manager, is_service_active, manage_service, rebuild_initramfs, terminate_session, }; +use fs2::FileExt; use log::{error, info}; use serde::{Deserialize, Serialize}; use std::fs; use std::path::Path; use std::process::Command; +use std::sync::atomic::{AtomicBool, Ordering}; use zbus::Connection; use zbus::proxy; +static INTERRUPTED: AtomicBool = AtomicBool::new(false); + +fn setup_signal_handler() { + let _ = ctrlc::set_handler(|| { + INTERRUPTED.store(true, Ordering::SeqCst); + }); +} + +struct DmGuard { + name: Option, +} + +impl Drop for DmGuard { + fn drop(&mut self) { + if let Some(ref name) = self.name { + info!("Restarting display manager: {}", name); + let _ = manage_service(name, "start"); + } + } +} + +const LOCK_PATH: &str = "/var/lock/lapctl.lock"; + +struct LockGuard; + +impl LockGuard { + fn acquire() -> Result { + let file = fs::File::create(LOCK_PATH) + .map_err(|e| format!("Failed to create lock file: {}", e))?; + file.try_lock_exclusive() + .map_err(|e| format!("Another GPU switch operation is already in progress: {}", e))?; + info!("Acquired exclusive lock"); + Ok(LockGuard) + } +} + +impl Drop for LockGuard { + fn drop(&mut self) { + let _ = fs::remove_file(LOCK_PATH); + info!("Released lock"); + } +} + #[proxy( interface = "org.lapctl1", default_service = "org.lapctl", @@ -414,10 +459,15 @@ fn create_cache_file() { let _ = fs::create_dir_all(parent); } - if let Ok(json) = serde_json::to_string_pretty(&cache) - && fs::write(CACHE_FILE_PATH, json).is_ok() - { - log::debug!("Created/Updated cache file {}", CACHE_FILE_PATH); + if let Ok(json) = serde_json::to_string_pretty(&cache) { + let tmp_path = format!("{}.tmp", CACHE_FILE_PATH); + if fs::write(&tmp_path, &json).is_ok() { + if fs::rename(&tmp_path, CACHE_FILE_PATH).is_ok() { + log::debug!("Created/Updated cache file {}", CACHE_FILE_PATH); + } else { + let _ = fs::remove_file(&tmp_path); + } + } } } @@ -435,7 +485,8 @@ fn read_cache_file() -> Result { } } -fn prepare_no_reboot(dm_flag: Option) -> Result, String> { +fn prepare_no_reboot(dm_flag: Option) -> Result { + setup_signal_handler(); let dm_name = dm_flag.or_else(get_display_manager); if let Some(ref name) = dm_name @@ -443,7 +494,9 @@ fn prepare_no_reboot(dm_flag: Option) -> Result, String> { info!("Stopping display manager: {}", name); manage_service(name, "stop")?; - return Ok(Some(name.clone())); + return Ok(DmGuard { + name: Some(name.clone()), + }); } // If no active DM found, check for graphical sessions @@ -460,18 +513,18 @@ fn prepare_no_reboot(dm_flag: Option) -> Result, String> std::thread::sleep(std::time::Duration::from_secs(2)); } - Ok(None) -} - -fn finalize_no_reboot(stopped_dm: Option) { - if let Some(name) = stopped_dm { - info!("Starting display manager: {}", name); - let _ = manage_service(&name, "start"); - } + Ok(DmGuard { name: None }) } fn switch_integrated(no_reboot: bool) { assert_root(); + let _lock = match LockGuard::acquire() { + Ok(l) => l, + Err(e) => { + error!("{}", e); + return; + } + }; println!("Switching to integrated mode"); if !Path::new(CACHE_FILE_PATH).exists() { @@ -483,15 +536,14 @@ fn switch_integrated(no_reboot: bool) { create_file(BLACKLIST_PATH, BLACKLIST_CONTENT, false); create_file(UDEV_INTEGRATED_PATH, UDEV_INTEGRATED, false); - let mut stopped_dm = None; - if no_reboot { - match prepare_no_reboot(None) { - Ok(dm) => stopped_dm = dm, + let _guard = if no_reboot { + let guard = match prepare_no_reboot(None) { + Ok(g) => g, Err(e) => { error!("{}", e); return; } - } + }; if let Err(e) = kill_gpu_processes() { error!("Failed to kill GPU processes: {}", e); @@ -511,7 +563,11 @@ fn switch_integrated(no_reboot: bool) { error!("Failed to remove GPU: {}", e); } } - } + + guard + } else { + DmGuard { name: None } + }; let is_debug = log::log_enabled!(log::Level::Debug); let mut dis_cmd = Command::new("systemctl"); @@ -527,7 +583,6 @@ fn switch_integrated(no_reboot: bool) { if no_reboot { println!("Operation completed successfully"); - finalize_no_reboot(stopped_dm); } else { rebuild_initramfs(); println!("Operation completed successfully"); @@ -537,6 +592,13 @@ fn switch_integrated(no_reboot: bool) { fn switch_hybrid(rtd3: Option, use_nvidia_current: bool, no_reboot: bool) { assert_root(); + let _lock = match LockGuard::acquire() { + Ok(l) => l, + Err(e) => { + error!("{}", e); + return; + } + }; println!("Switching to hybrid mode"); println!( "Enable PCI-Express Runtime D3 (RTD3) Power Management: {}", @@ -553,20 +615,23 @@ fn switch_hybrid(rtd3: Option, use_nvidia_current: bool, no_reboot: bool) { cleanup(); - let mut stopped_dm = None; - if no_reboot { - match prepare_no_reboot(None) { - Ok(dm) => stopped_dm = dm, + let _guard = if no_reboot { + let guard = match prepare_no_reboot(None) { + Ok(g) => g, Err(e) => { error!("{}", e); return; } - } + }; if let Err(e) = kill_gpu_processes() { error!("Failed to kill GPU processes: {}", e); } - } + + guard + } else { + DmGuard { name: None } + }; // persistenced enable let is_debug = log::log_enabled!(log::Level::Debug); @@ -606,7 +671,6 @@ fn switch_hybrid(rtd3: Option, use_nvidia_current: bool, no_reboot: bool) { if no_reboot { println!("Operation completed successfully"); - finalize_no_reboot(stopped_dm); } else { rebuild_initramfs(); println!("Operation completed successfully"); @@ -623,6 +687,13 @@ fn switch_nvidia( no_reboot: bool, ) { assert_root(); + let _lock = match LockGuard::acquire() { + Ok(l) => l, + Err(e) => { + error!("{}", e); + return; + } + }; let wayland = wayland || is_wayland_session(); @@ -716,20 +787,23 @@ fn switch_nvidia( } } - let mut stopped_dm = None; - if no_reboot { - match prepare_no_reboot(dm.clone()) { - Ok(dm_name) => stopped_dm = dm_name, + let _guard = if no_reboot { + let guard = match prepare_no_reboot(dm.clone()) { + Ok(g) => g, Err(e) => { error!("{}", e); return; } - } + }; if let Err(e) = kill_gpu_processes() { error!("Failed to kill GPU processes: {}", e); } - } + + guard + } else { + DmGuard { name: None } + }; // persistenced enable let is_debug = log::log_enabled!(log::Level::Debug); @@ -746,7 +820,6 @@ fn switch_nvidia( if no_reboot { println!("Operation completed successfully"); - finalize_no_reboot(stopped_dm); } else { rebuild_initramfs(); println!("Operation completed successfully"); diff --git a/src/commands/install_rules.rs b/src/commands/install_rules.rs index d319bd3..fb6929c 100644 --- a/src/commands/install_rules.rs +++ b/src/commands/install_rules.rs @@ -112,14 +112,23 @@ pub fn execute() { .args(["enable", "--now", "lapctld"]) .status(); - if let Ok(s) = status { + let daemon_ok = if let Ok(s) = status { if s.success() { info!("lapctld daemon enabled and started."); + true } else { log::error!("Failed to enable/start lapctld daemon."); + false } - } + } else { + false + }; - println!("Installation successful! The lapctld daemon is now running."); + if daemon_ok { + println!("Installation successful! The lapctld daemon is now running."); + } else { + println!("Installation completed (but daemon may not be running)."); + println!("Try starting it manually with: systemctl start lapctld"); + } println!("You can now use all lapctl commands from any location."); }