From 245c035dc9032f617799fbb8e689d1e4cb885a60 Mon Sep 17 00:00:00 2001 From: "Luke D. Jones" Date: Thu, 8 Dec 2022 20:12:55 +1300 Subject: [PATCH] Fix tasks not always running correctly on boot/sleep/wake/shutdown --- CHANGELOG.md | 3 ++ Cargo.lock | 18 +++++----- Cargo.toml | 2 +- daemon/src/ctrl_anime/trait_impls.rs | 32 +++++++++-------- daemon/src/ctrl_aura/trait_impls.rs | 34 ++++++++++-------- daemon/src/ctrl_platform.rs | 16 +++++---- daemon/src/ctrl_power.rs | 54 +++++++++++++++------------- daemon/src/lib.rs | 46 ++++++++++++++++++------ 8 files changed, 125 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6f03434..c4a76949 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [v4.5.6-RC1] +- Fix tasks not always running correctly on boot/sleep/wake/shutdown by finishing the move to async + ## [v4.5.5] - remove an unwrap() causing panic on main ROGCC thread diff --git a/Cargo.lock b/Cargo.lock index 7778ccd9..7277920f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -149,7 +149,7 @@ checksum = "8da52d66c7071e2e3fa2a1e5c6d088fec47b593032b254f5e980de8ea54454d6" [[package]] name = "asusctl" -version = "4.5.5" +version = "4.5.6-RC1" dependencies = [ "daemon", "gif", @@ -727,7 +727,7 @@ checksum = "b365fabc795046672053e29c954733ec3b05e4be654ab130fe8f1f94d7051f35" [[package]] name = "daemon" -version = "4.5.5" +version = "4.5.6-RC1" dependencies = [ "async-trait", "concat-idents", @@ -750,7 +750,7 @@ dependencies = [ [[package]] name = "daemon-user" -version = "4.5.5" +version = "4.5.6-RC1" dependencies = [ "dirs", "rog_anime", @@ -2593,7 +2593,7 @@ dependencies = [ [[package]] name = "rog-control-center" -version = "4.5.5" +version = "4.5.6-RC1" dependencies = [ "daemon", "dirs", @@ -2623,7 +2623,7 @@ dependencies = [ [[package]] name = "rog_anime" -version = "4.5.5" +version = "4.5.6-RC1" dependencies = [ "gif", "glam", @@ -2638,7 +2638,7 @@ dependencies = [ [[package]] name = "rog_aura" -version = "4.5.5" +version = "4.5.6-RC1" dependencies = [ "serde", "serde_derive", @@ -2649,7 +2649,7 @@ dependencies = [ [[package]] name = "rog_dbus" -version = "4.5.5" +version = "4.5.6-RC1" dependencies = [ "rog_anime", "rog_aura", @@ -2660,7 +2660,7 @@ dependencies = [ [[package]] name = "rog_platform" -version = "4.5.5" +version = "4.5.6-RC1" dependencies = [ "concat-idents", "inotify", @@ -2676,7 +2676,7 @@ dependencies = [ [[package]] name = "rog_profiles" -version = "4.5.5" +version = "4.5.6-RC1" dependencies = [ "serde", "serde_derive", diff --git a/Cargo.toml b/Cargo.toml index 0d526527..9ff25330 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ members = ["asusctl", "daemon", "daemon-user", "rog-platform", "rog-dbus", "rog-anime", "rog-aura", "rog-profiles", "rog-control-center"] [workspace.package] -version = "4.5.5" +version = "4.5.6-RC1" [workspace.dependencies] async-trait = "^0.1" diff --git a/daemon/src/ctrl_anime/trait_impls.rs b/daemon/src/ctrl_anime/trait_impls.rs index 095ca300..64fb7f9c 100644 --- a/daemon/src/ctrl_anime/trait_impls.rs +++ b/daemon/src/ctrl_anime/trait_impls.rs @@ -166,28 +166,32 @@ impl crate::CtrlTask for CtrlAnimeZbus { self.create_sys_event_tasks( // Loop is required to try an attempt to get the mutex *without* blocking // other threads - it is possible to end up with deadlocks otherwise. - move || loop { - if let Some(lock) = inner1.try_lock() { + move || { + let inner1 = inner1.clone(); + async move { + let lock = inner1.lock().await; run_action(true, lock, inner1.clone()); - break; } }, - move || loop { - if let Some(lock) = inner2.try_lock() { - run_action(false, lock, inner2.clone()); - break; + move || { + let inner2 = inner2.clone(); + async move { + let lock = inner2.lock().await; + run_action(true, lock, inner2.clone()); } }, - move || loop { - if let Some(lock) = inner3.try_lock() { + move || { + let inner3 = inner3.clone(); + async move { + let lock = inner3.lock().await; run_action(true, lock, inner3.clone()); - break; } }, - move || loop { - if let Some(lock) = inner4.try_lock() { - run_action(false, lock, inner4.clone()); - break; + move || { + let inner4 = inner4.clone(); + async move { + let lock = inner4.lock().await; + run_action(true, lock, inner4.clone()); } }, ) diff --git a/daemon/src/ctrl_aura/trait_impls.rs b/daemon/src/ctrl_aura/trait_impls.rs index 5219f747..0bc2d4ba 100644 --- a/daemon/src/ctrl_aura/trait_impls.rs +++ b/daemon/src/ctrl_aura/trait_impls.rs @@ -262,28 +262,32 @@ impl CtrlTask for CtrlKbdLedZbus { self.create_sys_event_tasks( // Loop so that we do aquire the lock but also don't block other // threads (prevents potential deadlocks) - move || loop { - if let Some(lock) = inner1.try_lock() { + move || { + let inner1 = inner1.clone(); + async move { + let lock = inner1.lock().await; load_save(true, lock); - break; } }, - move || loop { - if let Some(lock) = inner2.try_lock() { + move || { + let inner2 = inner2.clone(); + async move { + let lock = inner2.lock().await; load_save(false, lock); - break; } }, - move || loop { - if let Some(lock) = inner3.try_lock() { - load_save(true, lock); - break; - } - }, - move || loop { - if let Some(lock) = inner4.try_lock() { + move || { + let inner3 = inner3.clone(); + async move { + let lock = inner3.lock().await; + load_save(false, lock); + } + }, + move || { + let inner4 = inner4.clone(); + async move { + let lock = inner4.lock().await; load_save(false, lock); - break; } }, ) diff --git a/daemon/src/ctrl_platform.rs b/daemon/src/ctrl_platform.rs index fc56cbe7..d1e16f5d 100644 --- a/daemon/src/ctrl_platform.rs +++ b/daemon/src/ctrl_platform.rs @@ -332,10 +332,12 @@ impl CtrlTask for CtrlPlatform { let platform1 = self.clone(); let platform2 = self.clone(); self.create_sys_event_tasks( - move || {}, + move || async { {} }, move || { - info!("CtrlRogBios reloading panel_od"); - if let Some(lock) = platform1.config.try_lock() { + let platform1 = platform1.clone(); + async move { + info!("CtrlRogBios reloading panel_od"); + let lock = platform1.config.lock().await; if platform1.platform.has_panel_od() { platform1 .set_panel_overdrive(lock.panel_od) @@ -347,10 +349,12 @@ impl CtrlTask for CtrlPlatform { } } }, - move || {}, + move || async { {} }, move || { - info!("CtrlRogBios reloading panel_od"); - if let Some(lock) = platform2.config.try_lock() { + let platform2 = platform2.clone(); + async move { + info!("CtrlRogBios reloading panel_od"); + let lock = platform2.config.lock().await; if platform2.platform.has_panel_od() { platform2 .set_panel_overdrive(lock.panel_od) diff --git a/daemon/src/ctrl_power.rs b/daemon/src/ctrl_power.rs index 2983d1fc..2e58816b 100644 --- a/daemon/src/ctrl_power.rs +++ b/daemon/src/ctrl_power.rs @@ -155,10 +155,12 @@ impl CtrlTask for CtrlPower { let power1 = self.clone(); let power2 = self.clone(); self.create_sys_event_tasks( - move || {}, + move || async {}, move || { - info!("CtrlCharge reloading charge limit"); - if let Some(lock) = power1.config.try_lock() { + let power1 = power1.clone(); + async move { + info!("CtrlCharge reloading charge limit"); + let lock = power1.config.lock().await; power1 .set(lock.bat_charge_limit) .map_err(|err| { @@ -166,22 +168,25 @@ impl CtrlTask for CtrlPower { err }) .ok(); - } - if let Ok(value) = power1.power.get_online() { - let action = if value == 1 { - SystemdUnitAction::Restart - } else { - SystemdUnitAction::Stop - }; - if do_systemd_unit_action(action, NVIDIA_POWERD).is_ok() { - info!("CtrlPower task: did {action:?} on {NVIDIA_POWERD}"); + + if let Ok(value) = power1.power.get_online() { + let action = if value == 1 { + SystemdUnitAction::Restart + } else { + SystemdUnitAction::Stop + }; + if do_systemd_unit_action(action, NVIDIA_POWERD).is_ok() { + info!("CtrlPower task: did {action:?} on {NVIDIA_POWERD}"); + } } } }, - move || {}, + move || async {}, move || { - info!("CtrlCharge reloading charge limit"); - if let Some(lock) = power2.config.try_lock() { + let power2 = power2.clone(); + async move { + info!("CtrlCharge reloading charge limit"); + let lock = power2.config.lock().await; power2 .set(lock.bat_charge_limit) .map_err(|err| { @@ -189,15 +194,16 @@ impl CtrlTask for CtrlPower { err }) .ok(); - } - if let Ok(value) = power2.power.get_online() { - let action = if value == 1 { - SystemdUnitAction::Restart - } else { - SystemdUnitAction::Stop - }; - if do_systemd_unit_action(action, NVIDIA_POWERD).is_ok() { - info!("CtrlPower task: did {action:?} on {NVIDIA_POWERD}"); + + if let Ok(value) = power2.power.get_online() { + let action = if value == 1 { + SystemdUnitAction::Restart + } else { + SystemdUnitAction::Stop + }; + if do_systemd_unit_action(action, NVIDIA_POWERD).is_ok() { + info!("CtrlPower task: did {action:?} on {NVIDIA_POWERD}"); + } } } }, diff --git a/daemon/src/lib.rs b/daemon/src/lib.rs index 5154e630..5c708b4e 100644 --- a/daemon/src/lib.rs +++ b/daemon/src/lib.rs @@ -21,9 +21,11 @@ pub mod ctrl_supported; pub mod error; +use std::future::Future; + use crate::error::RogError; use async_trait::async_trait; -use log::warn; +use log::{debug, warn}; use logind_zbus::manager::ManagerProxy; use zbus::{export::futures_util::StreamExt, zvariant::ObjectPath, Connection, SignalContext}; @@ -134,13 +136,31 @@ pub trait CtrlTask { /// /// The closures can potentially block, so execution time should be the minimal possible /// such as save a variable. - async fn create_sys_event_tasks( + async fn create_sys_event_tasks< + Fut1, + Fut2, + Fut3, + Fut4, + F1: Send + 'static, + F2: Send + 'static, + F3: Send + 'static, + F4: Send + 'static, + >( &self, - mut on_sleep: impl FnMut() + Send + 'static, - mut on_wake: impl FnMut() + Send + 'static, - mut on_shutdown: impl FnMut() + Send + 'static, - mut on_boot: impl FnMut() + Send + 'static, - ) { + mut on_sleep: F1, + mut on_wake: F2, + mut on_shutdown: F3, + mut on_boot: F4, + ) where + F1: FnMut() -> Fut1, + F2: FnMut() -> Fut2, + F3: FnMut() -> Fut3, + F4: FnMut() -> Fut4, + Fut1: Future + Send, + Fut2: Future + Send, + Fut3: Future + Send, + Fut4: Future + Send, + { let connection = Connection::system() .await .expect("Controller could not create dbus connection"); @@ -154,9 +174,11 @@ pub trait CtrlTask { while let Some(event) = notif.next().await { if let Ok(args) = event.args() { if args.start { - on_sleep(); + debug!("Doing on_sleep()"); + on_sleep().await; } else if !args.start() { - on_wake(); + debug!("Doing on_wake()"); + on_wake().await; } } } @@ -172,9 +194,11 @@ pub trait CtrlTask { while let Some(event) = notif.next().await { if let Ok(args) = event.args() { if args.start { - on_shutdown(); + debug!("Doing on_shutdown()"); + on_shutdown().await; } else if !args.start() { - on_boot(); + debug!("Doing on_boot()"); + on_boot().await; } } }