asusd: anime: don't cause async deadlocks damnit

Same old song, an async mutex lock being held for a wide scope.
In particular being held for a scope that includes a function call which
also tries to get the lock.

Fix it by copy/clone of the config interior values required.

Fixes #588
This commit is contained in:
Luke D. Jones
2025-01-21 12:24:24 +13:00
parent cba8e1a473
commit 25823dc6b7
2 changed files with 51 additions and 44 deletions

View File

@@ -16,7 +16,7 @@ use rog_anime::usb::{
use rog_anime::{ActionData, AnimeDataBuffer, AnimePacketType}; use rog_anime::{ActionData, AnimeDataBuffer, AnimePacketType};
use rog_platform::hid_raw::HidRaw; use rog_platform::hid_raw::HidRaw;
use rog_platform::usb_raw::USBRaw; use rog_platform::usb_raw::USBRaw;
use tokio::sync::{Mutex, MutexGuard}; use tokio::sync::Mutex;
use self::config::{AniMeConfig, AniMeConfigCached}; use self::config::{AniMeConfig, AniMeConfigCached};
use crate::error::RogError; use crate::error::RogError;
@@ -77,10 +77,6 @@ impl AniMe {
Ok(()) Ok(())
} }
pub async fn lock_config(&self) -> MutexGuard<AniMeConfig> {
self.config.lock().await
}
pub async fn write_bytes(&self, message: &[u8]) -> Result<(), RogError> { pub async fn write_bytes(&self, message: &[u8]) -> Result<(), RogError> {
if let Some(hid) = &self.hid { if let Some(hid) = &self.hid {
hid.lock().await.write_bytes(message)?; hid.lock().await.write_bytes(message)?;
@@ -231,10 +227,11 @@ impl AniMe {
}) })
.ok(); .ok();
} }
// A write can block for many milliseconds so lets not hold the config lock for
// the same period
let enabled = inner.config.lock().await.builtin_anims_enabled;
inner inner
.write_bytes(&pkt_set_enable_powersave_anim( .write_bytes(&pkt_set_enable_powersave_anim(enabled))
inner.config.lock().await.builtin_anims_enabled
))
.await .await
.map_err(|err| { .map_err(|err| {
warn!("rog_anime::run_animation:callback {}", err); warn!("rog_anime::run_animation:callback {}", err);

View File

@@ -443,50 +443,60 @@ impl crate::CtrlTask for AniMeZbus {
impl crate::Reloadable for AniMeZbus { impl crate::Reloadable for AniMeZbus {
async fn reload(&mut self) -> Result<(), RogError> { async fn reload(&mut self) -> Result<(), RogError> {
if let Ok(config) = self.0.config.try_lock() { let AniMeConfig {
let anim = &config.builtin_anims; builtin_anims_enabled,
// Set builtins builtin_anims,
if config.builtin_anims_enabled { display_enabled,
self.0 display_brightness,
.write_bytes(&pkt_set_builtin_animations( off_when_lid_closed,
anim.boot, anim.awake, anim.sleep, anim.shutdown off_when_unplugged,
)) ..
.await?; } = *self.0.config.lock().await;
}
// Builtins enabled or na? // Set builtins
if builtin_anims_enabled {
self.0 self.0
.set_builtins_enabled(config.builtin_anims_enabled, config.display_brightness) .write_bytes(&pkt_set_builtin_animations(
builtin_anims.boot,
builtin_anims.awake,
builtin_anims.sleep,
builtin_anims.shutdown
))
.await?; .await?;
}
// Builtins enabled or na?
self.0
.set_builtins_enabled(builtin_anims_enabled, display_brightness)
.await?;
let manager = get_logind_manager().await; let manager = get_logind_manager().await;
let lid_closed = manager.lid_closed().await.unwrap_or_default(); let lid_closed = manager.lid_closed().await.unwrap_or_default();
let power_plugged = manager.on_external_power().await.unwrap_or_default(); let power_plugged = manager.on_external_power().await.unwrap_or_default();
let turn_off = (lid_closed && config.off_when_lid_closed) let turn_off =
|| (!power_plugged && config.off_when_unplugged); (lid_closed && off_when_lid_closed) || (!power_plugged && off_when_unplugged);
self.0
.write_bytes(&pkt_set_enable_display(!turn_off))
.await
.map_err(|err| {
warn!("create_sys_event_tasks::reload {}", err);
})
.ok();
if turn_off || !display_enabled {
self.0.write_bytes(&pkt_set_enable_display(false)).await?;
// early return so we don't run animation thread
return Ok(());
}
if !builtin_anims_enabled && !self.0.cache.boot.is_empty() {
self.0 self.0
.write_bytes(&pkt_set_enable_display(!turn_off)) .write_bytes(&pkt_set_enable_powersave_anim(false))
.await .await
.map_err(|err| {
warn!("create_sys_event_tasks::reload {}", err);
})
.ok(); .ok();
if turn_off || !config.display_enabled { let action = self.0.cache.boot.clone();
self.0.write_bytes(&pkt_set_enable_display(false)).await?; self.0.run_thread(action, true).await;
// early return so we don't run animation thread
return Ok(());
}
if !config.builtin_anims_enabled && !self.0.cache.boot.is_empty() {
self.0
.write_bytes(&pkt_set_enable_powersave_anim(false))
.await
.ok();
let action = self.0.cache.boot.clone();
self.0.run_thread(action, true).await;
}
} }
Ok(()) Ok(())
} }