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) + )); + } }