From 093fe1f1e272213e9f2e61ba10edcbd829419023 Mon Sep 17 00:00:00 2001 From: Loong Date: Thu, 22 Oct 2026 23:51:50 +0100 Subject: [PATCH] fix(core): guard runtime use during version queries Hold runtime use in RAII guards owned by Assistant instances and temporary version queries. Keep runtime changes blocked until destruction or version string copying finishes. --- crates/maa-core/src/error.rs | 6 +- crates/maa-core/src/lib.rs | 25 ++-- .../maa-core/src/runtime_lifecycle/linked.rs | 10 +- .../maa-core/src/runtime_lifecycle/runtime.rs | 141 ++++++++++-------- 4 files changed, 98 insertions(+), 84 deletions(-) diff --git a/crates/maa-core/src/error.rs b/crates/maa-core/src/error.rs index 471299be..704a6dd6 100644 --- a/crates/maa-core/src/error.rs +++ b/crates/maa-core/src/error.rs @@ -11,16 +11,16 @@ pub enum Error { #[error("The content returned by MaaCore is too large (length > {0})")] ContentTooLarge(usize), #[cfg(feature = "runtime")] - #[error("Cannot load or unload MaaCore runtime while Assistant instances are alive")] + #[error("Cannot load or unload MaaCore runtime while it is in use")] ActiveAssistants, #[cfg(feature = "runtime")] #[error("Cannot perform this operation while MaaCore runtime is loading or unloading")] RuntimeChanging, #[cfg(feature = "runtime")] - #[error("Cannot create Assistant before MaaCore runtime is loaded")] + #[error("Cannot use MaaCore before its runtime is loaded")] RuntimeNotLoaded, #[cfg(feature = "runtime")] - #[error("Cannot create Assistant because too many Assistant instances are alive")] + #[error("Cannot acquire MaaCore runtime because too many uses are active")] TooManyAssistants, #[error("Input argument contains invalid bytes")] InvalidArgument(#[from] maa_ffi_string::Error), diff --git a/crates/maa-core/src/lib.rs b/crates/maa-core/src/lib.rs index e0639845..84410cf2 100644 --- a/crates/maa-core/src/lib.rs +++ b/crates/maa-core/src/lib.rs @@ -39,16 +39,15 @@ pub(crate) fn get_log_path() -> std::path::PathBuf { pub struct Assistant { handle: maa_sys::binding::AsstHandle, _callback: Option>, + _guard: runtime_lifecycle::Guard, } impl Drop for Assistant { fn drop(&mut self) { // Destroy the handle before the callback is dropped to make sure the callback is not used - // after the Box is freed. The active-assistant count is decremented after this call so a - // concurrent runtime load or unload still sees this instance as alive during destruction. + // after the Box is freed. The runtime guard is released after this call, + // keeping concurrent runtime changes blocked during destruction. unsafe { maa_sys::binding::AsstDestroy(self.handle) }; - - runtime_lifecycle::unregister_assistant(); } } @@ -85,10 +84,12 @@ impl Assistant { } /// Get the version string of MaaCore. + /// + /// In runtime mode, returns an error if MaaCore is not loaded or is changing. pub fn get_version() -> Result { - // Safety: AsstGetVersion returns a pointer into the loaded library's - // read-only data segment, valid only while the library remains loaded. - // `to_owned` copies the bytes before any unload can occur. + let _guard = runtime_lifecycle::Guard::acquire()?; + // Safety: MaaCore returns a valid, NUL-terminated string. The runtime + // guard prevents load or unload until the string has been copied. let version = unsafe { std::ffi::CStr::from_ptr(maa_sys::binding::AsstGetVersion()).to_owned() }; Ok(String::from_utf8(version.into_bytes())?) @@ -115,18 +116,17 @@ impl Assistant { impl Assistant { /// Create a new assistant instance without a callback. pub fn new() -> Result { - let registration = runtime_lifecycle::PendingAssistantRegistration::begin()?; + let guard = runtime_lifecycle::Guard::acquire()?; let handle = unsafe { maa_sys::binding::AsstCreate() }; if handle.is_null() { return Err(Error::NullHandle); } - registration.commit(); - Ok(Self { handle, _callback: None, + _guard: guard, }) } @@ -138,18 +138,17 @@ impl Assistant { let boxed = Box::new(callback); let raw = &raw const *boxed as *mut c_void; - let registration = runtime_lifecycle::PendingAssistantRegistration::begin()?; + let guard = runtime_lifecycle::Guard::acquire()?; let handle = unsafe { maa_sys::binding::AsstCreateEx(Some(trampoline::), raw) }; if handle.is_null() { return Err(Error::NullHandle); } - registration.commit(); - Ok(Self { handle, _callback: Some(boxed as Box), + _guard: guard, }) } diff --git a/crates/maa-core/src/runtime_lifecycle/linked.rs b/crates/maa-core/src/runtime_lifecycle/linked.rs index f228f851..40b9b417 100644 --- a/crates/maa-core/src/runtime_lifecycle/linked.rs +++ b/crates/maa-core/src/runtime_lifecycle/linked.rs @@ -19,14 +19,10 @@ impl Assistant { } } -pub(crate) struct PendingAssistantRegistration; +pub(crate) struct Guard; -impl PendingAssistantRegistration { - pub(crate) fn begin() -> Result { +impl Guard { + pub(crate) fn acquire() -> Result { Ok(Self) } - - pub(crate) fn commit(self) {} } - -pub(crate) fn unregister_assistant() {} diff --git a/crates/maa-core/src/runtime_lifecycle/runtime.rs b/crates/maa-core/src/runtime_lifecycle/runtime.rs index 1c4fc3e5..accacee7 100644 --- a/crates/maa-core/src/runtime_lifecycle/runtime.rs +++ b/crates/maa-core/src/runtime_lifecycle/runtime.rs @@ -7,7 +7,7 @@ use crate::{Assistant, Error, Result}; const RUNTIME_LOCKED: usize = 1usize << (usize::BITS - 1); const RUNTIME_LOADED: usize = 1usize << (usize::BITS - 2); -const ASSISTANT_COUNT_MASK: usize = RUNTIME_LOADED - 1; +const USE_COUNT_MASK: usize = RUNTIME_LOADED - 1; static RUNTIME_STATE: AtomicUsize = AtomicUsize::new(0); @@ -19,8 +19,8 @@ fn is_loaded(state: usize) -> bool { state & RUNTIME_LOADED != 0 } -fn assistant_count(state: usize) -> usize { - state & ASSISTANT_COUNT_MASK +fn use_count(state: usize) -> usize { + state & USE_COUNT_MASK } enum StableState { @@ -62,7 +62,7 @@ impl RuntimeChange { } let locked_state = RUNTIME_STATE.load(Ordering::Acquire); - if assistant_count(locked_state) > 0 { + if use_count(locked_state) > 0 { RUNTIME_STATE.fetch_and(!RUNTIME_LOCKED, Ordering::AcqRel); return Err(Error::ActiveAssistants); } @@ -89,7 +89,7 @@ impl Assistant { /// Load the shared library of the MaaCore /// /// Must be called before any other method. - /// Fails if any assistant instances are alive. + /// Fails if any assistant instances are alive or a version query is in progress. pub fn load(path: impl AsRef) -> Result<()> { let path = path.as_ref(); let runtime_change = RuntimeChange::begin()?; @@ -117,7 +117,7 @@ impl Assistant { /// Unload the shared library of the MaaCore. /// /// Must be called after all assistant instances are destroyed. - /// Fails if any assistant instances are alive. + /// Fails if any assistant instances are alive or a version query is in progress. pub fn unload() -> Result<()> { let runtime_change = RuntimeChange::begin()?; maa_sys::binding::unload(); @@ -131,12 +131,11 @@ impl Assistant { } } -pub(crate) struct PendingAssistantRegistration { - committed: bool, -} +/// Prevents runtime changes until the guarded operation or instance is finished. +pub(crate) struct Guard; -impl PendingAssistantRegistration { - pub(crate) fn begin() -> Result { +impl Guard { + pub(crate) fn acquire() -> Result { loop { let state = RUNTIME_STATE.load(Ordering::Acquire); if is_locked(state) { @@ -147,40 +146,30 @@ impl PendingAssistantRegistration { return Err(Error::RuntimeNotLoaded); } - let count = assistant_count(state); + let count = use_count(state); let next_count = count .checked_add(1) - .filter(|count| *count <= ASSISTANT_COUNT_MASK) + .filter(|count| *count <= USE_COUNT_MASK) .ok_or(Error::TooManyAssistants)?; - let next = (state & !ASSISTANT_COUNT_MASK) | next_count; + let next = (state & !USE_COUNT_MASK) | next_count; if RUNTIME_STATE .compare_exchange(state, next, Ordering::AcqRel, Ordering::Acquire) .is_ok() { - return Ok(Self { committed: false }); + return Ok(Self); } } } - - pub(crate) fn commit(mut self) { - self.committed = true; - } } -impl Drop for PendingAssistantRegistration { +impl Drop for Guard { fn drop(&mut self) { - if !self.committed { - unregister_assistant(); - } + let previous = RUNTIME_STATE.fetch_sub(1, Ordering::AcqRel); + debug_assert!(is_loaded(previous)); + debug_assert!(use_count(previous) > 0); } } -pub(crate) fn unregister_assistant() { - let previous = RUNTIME_STATE.fetch_sub(1, Ordering::AcqRel); - debug_assert!(is_loaded(previous)); - debug_assert!(assistant_count(previous) > 0); -} - #[cfg(test)] pub(crate) mod tests { use super::*; @@ -192,29 +181,35 @@ pub(crate) mod tests { } #[test] - fn runtime_change_rejects_active_assistants() { + fn guard_blocks_load_and_unload_on_other_threads() { let _state_lock = RUNTIME_STATE_TEST_LOCK.lock().unwrap(); reset_runtime_state_for_test(RUNTIME_LOADED); - let _registration = PendingAssistantRegistration::begin().unwrap(); - - assert!(matches!( - RuntimeChange::begin(), - Err(Error::ActiveAssistants) - )); + let guard = Guard::acquire().unwrap(); + std::thread::scope(|scope| { + scope + .spawn(|| { + assert!(matches!( + Assistant::load("/this/library/does_not_exist.so"), + Err(Error::ActiveAssistants) + )); + assert!(matches!(Assistant::unload(), Err(Error::ActiveAssistants))); + }) + .join() + .unwrap(); + }); + drop(guard); + assert!(RuntimeChange::begin().is_ok()); } #[test] - fn assistant_registration_rejects_runtime_changes() { + fn guard_rejects_runtime_changes() { let _state_lock = RUNTIME_STATE_TEST_LOCK.lock().unwrap(); reset_runtime_state_for_test(0); let _runtime_change = RuntimeChange::begin().unwrap(); - assert!(matches!( - PendingAssistantRegistration::begin(), - Err(Error::RuntimeChanging) - )); + assert!(matches!(Guard::acquire(), Err(Error::RuntimeChanging))); } #[test] @@ -242,28 +237,22 @@ pub(crate) mod tests { } #[test] - fn assistant_registration_rejects_unloaded_runtime() { + fn guard_rejects_unloaded_runtime() { let _state_lock = RUNTIME_STATE_TEST_LOCK.lock().unwrap(); reset_runtime_state_for_test(0); - assert!(matches!( - PendingAssistantRegistration::begin(), - Err(Error::RuntimeNotLoaded) - )); + assert!(matches!(Guard::acquire(), Err(Error::RuntimeNotLoaded))); } #[test] - fn assistant_registration_rejects_count_overflow() { + fn guard_rejects_count_overflow() { let _state_lock = RUNTIME_STATE_TEST_LOCK.lock().unwrap(); - reset_runtime_state_for_test(RUNTIME_LOADED | ASSISTANT_COUNT_MASK); + reset_runtime_state_for_test(RUNTIME_LOADED | USE_COUNT_MASK); - assert!(matches!( - PendingAssistantRegistration::begin(), - Err(Error::TooManyAssistants) - )); + assert!(matches!(Guard::acquire(), Err(Error::TooManyAssistants))); assert_eq!( RUNTIME_STATE.load(Ordering::Acquire), - RUNTIME_LOADED | ASSISTANT_COUNT_MASK + RUNTIME_LOADED | USE_COUNT_MASK ); } @@ -300,35 +289,49 @@ pub(crate) mod tests { } #[test] - fn runtime_lock_preserves_assistant_count_changes() { + fn runtime_lock_preserves_use_count_changes() { let _state_lock = RUNTIME_STATE_TEST_LOCK.lock().unwrap(); - reset_runtime_state_for_test(RUNTIME_LOCKED | RUNTIME_LOADED | 1); + reset_runtime_state_for_test(RUNTIME_LOADED); + let guard = Guard::acquire().unwrap(); - unregister_assistant(); + RUNTIME_STATE.fetch_or(RUNTIME_LOCKED, Ordering::AcqRel); + drop(guard); + assert_eq!( + RUNTIME_STATE.load(Ordering::Acquire), + RUNTIME_LOCKED | RUNTIME_LOADED + ); RUNTIME_STATE.fetch_and(!RUNTIME_LOCKED, Ordering::AcqRel); assert_eq!(RUNTIME_STATE.load(Ordering::Acquire), RUNTIME_LOADED); } #[test] - fn assistant_registration_rolls_back_without_commit() { + fn guard_releases_count_on_drop() { let _state_lock = RUNTIME_STATE_TEST_LOCK.lock().unwrap(); reset_runtime_state_for_test(RUNTIME_LOADED); - drop(PendingAssistantRegistration::begin().unwrap()); + drop(Guard::acquire().unwrap()); assert_eq!(RUNTIME_STATE.load(Ordering::Acquire), RUNTIME_LOADED); } #[test] - fn assistant_registration_commit_keeps_count() { + fn runtime_guards_keep_count_until_last_drop() { let _state_lock = RUNTIME_STATE_TEST_LOCK.lock().unwrap(); reset_runtime_state_for_test(RUNTIME_LOADED); - PendingAssistantRegistration::begin().unwrap().commit(); + let first = Guard::acquire().unwrap(); + let second = Guard::acquire().unwrap(); + assert_eq!(RUNTIME_STATE.load(Ordering::Acquire), RUNTIME_LOADED | 2); + drop(first); assert_eq!(RUNTIME_STATE.load(Ordering::Acquire), RUNTIME_LOADED | 1); - unregister_assistant(); + assert!(matches!( + RuntimeChange::begin(), + Err(Error::ActiveAssistants) + )); + drop(second); + assert_eq!(RUNTIME_STATE.load(Ordering::Acquire), RUNTIME_LOADED); } #[test] @@ -340,4 +343,20 @@ pub(crate) mod tests { assert_eq!(RUNTIME_STATE.load(Ordering::Acquire), RUNTIME_LOADED); } + + #[test] + fn version_query_rejects_unloaded_or_changing_runtime_before_ffi() { + let _state_lock = RUNTIME_STATE_TEST_LOCK.lock().unwrap(); + reset_runtime_state_for_test(0); + assert!(matches!( + Assistant::get_version(), + Err(Error::RuntimeNotLoaded) + )); + + reset_runtime_state_for_test(RUNTIME_LOCKED); + assert!(matches!( + Assistant::get_version(), + Err(Error::RuntimeChanging) + )); + } }