fix(core): guard runtime use during version queries

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),

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<Box<dyn Callback>>,
+    _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<String> {
-        // 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<Self> {
-        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::<C>), raw) };
         if handle.is_null() {
             return Err(Error::NullHandle);
         }
 
-        registration.commit();
-
         Ok(Self {
             handle,
             _callback: Some(boxed as Box<dyn Callback>),
+            _guard: guard,
         })
     }
 

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<Self> {
+impl Guard {
+    pub(crate) fn acquire() -> Result<Self> {
         Ok(Self)
     }
-
-    pub(crate) fn commit(self) {}
 }
-
-pub(crate) fn unregister_assistant() {}

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<Path>) -> 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<Self> {
+impl Guard {
+    pub(crate) fn acquire() -> Result<Self> {
         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)
+        ));
+    }
 }