Serialise the tests that install process-global host seams

crates/tinymemory-module/src/host_test.rs

@@ -255,6 +255,9 @@ async fn runtime_callbacks_and_spacy_cross_the_bus_with_their_full_payloads() {
 /// one's `HostSeamsRestore` had already put the globals back.
 #[tokio::test]
 async fn install_wires_every_seam_this_module_can_supply() {
+    // Taken before the capture, so the state this restores on drop is the
+    // state no other test can be moving underneath it. See `seam_lock`.
+    let _seams = crate::seam_lock::hold_global_seams_async().await;
     let _restore = HostSeamsRestore::capture();
     let (connection, _callbacks) = bus_with_runtime_host().await;
 
@@ -389,6 +392,7 @@ async fn store_policy_wakes_sleepers_only_on_resume() {
 fn manual_override_outranks_a_paused_gate_and_is_bounded() {
     use tinymemory_core::scheduler_gate as core_gate;
     use tinymemory_core::scheduler_gate::{PauseReason, Policy};
+    let _seams = crate::seam_lock::hold_global_seams();
     core_gate::clear_manual_override();
     let gate = gate_for_test();
     gate.store_policy(Policy::Paused {

crates/tinymemory-module/src/lib.rs

@@ -69,6 +69,8 @@ pub mod config_loader;
 pub mod embedding;
 mod host;
 mod provider;
+#[cfg(test)]
+mod seam_lock;
 mod service;
 
 pub use chat::{CHAT_HOST_BUS_NAME, CHAT_HOST_INTERFACE, CHAT_HOST_OBJECT_PATH};

crates/tinymemory-module/src/seam_lock.rs

@@ -0,0 +1,54 @@
+//! Serialises the tests that reach `tinymemory_core`'s process-global seams.
+//!
+//! The seams — event sink, error reporter, NLP host, scheduler gate and
+//! shutdown host — are `static`s owned by the process, not by whoever installs
+//! them. `libtest` runs a binary's tests on parallel threads, so two tests that
+//! each install a seam are racing: one's `set_scheduler_gate` or
+//! `set_manual_override` lands between the other's write and its assertion, and
+//! the assertion then fails for reasons that have nothing to do with the test
+//! that reported it.
+//!
+//! [`crate::host_test::HostSeamsRestore`] does not solve this on its own, and it
+//! is worth saying why, because it looks as though it should. It restores what a
+//! test found, which fixes *ordering* — a later test cannot inherit an earlier
+//! one's gate. It says nothing about two tests running at the same time.
+//!
+//! A test that installs or reads a global seam must hold this for its whole
+//! body, taken before it captures anything. See issue #130.
+//!
+//! The lock is `tokio`'s rather than the standard library's for one reason: two
+//! of the three callers are `#[tokio::test]` and hold it across `.await`, which
+//! is exactly what `clippy::await_holding_lock` exists to stop you doing with a
+//! `std` guard. Hence the pair of accessors below — the sync one is not an
+//! alternative to the async one, it is for the caller that has no runtime.
+
+use std::sync::OnceLock;
+
+use tokio::sync::{Mutex, MutexGuard};
+
+static SEAMS: OnceLock<Mutex<()>> = OnceLock::new();
+
+fn seams() -> &'static Mutex<()> {
+    SEAMS.get_or_init(|| Mutex::new(()))
+}
+
+/// Blocks the current thread until no other test holds the global seams.
+///
+/// # Panics
+///
+/// Panics if called from inside a tokio runtime — use [`hold_global_seams_async`]
+/// there. That is `blocking_lock`'s own rule, and it is the right failure: a
+/// blocking wait on a runtime thread is a deadlock waiting for the right
+/// scheduling.
+pub(crate) fn hold_global_seams() -> MutexGuard<'static, ()> {
+    seams().blocking_lock()
+}
+
+/// The same lock, awaited, for a test that runs on a tokio runtime.
+pub(crate) async fn hold_global_seams_async() -> MutexGuard<'static, ()> {
+    seams().lock().await
+}
+
+#[cfg(test)]
+#[path = "seam_lock_test.rs"]
+mod test;

crates/tinymemory-module/src/seam_lock_test.rs

@@ -0,0 +1,40 @@
+//! Tests for the global-seam test lock.
+//!
+//! One test, and it is about the lock rather than about any seam: everything
+//! [`super`] claims rests on the lock actually being exclusive.
+
+/// The lock is actually exclusive.
+///
+/// Worth pinning, because everything in [`super`] only helps if this holds, and a
+/// refactor could quietly make it a no-op — a second `OnceLock`, a guard
+/// dropped at the end of its own statement rather than the test body — without
+/// any test noticing. The failure it guards against is invisible by nature:
+/// the suite would simply go back to being flaky somewhere else.
+#[test]
+fn only_one_thread_holds_the_seams_at_a_time() {
+    use std::sync::atomic::{AtomicUsize, Ordering};
+
+    static INSIDE: AtomicUsize = AtomicUsize::new(0);
+    static PEAK: AtomicUsize = AtomicUsize::new(0);
+
+    std::thread::scope(|scope| {
+        for _ in 0..8 {
+            scope.spawn(|| {
+                for _ in 0..200 {
+                    let _seams = super::hold_global_seams();
+                    let now = INSIDE.fetch_add(1, Ordering::SeqCst) + 1;
+                    PEAK.fetch_max(now, Ordering::SeqCst);
+                    std::thread::yield_now();
+                    INSIDE.fetch_sub(1, Ordering::SeqCst);
+                }
+            });
+        }
+    });
+
+    assert_eq!(
+        PEAK.load(Ordering::SeqCst),
+        1,
+        "two threads held the global seams at once, so serialising the tests \
+         that install them buys nothing"
+    );
+}

crates/tinymemory-module/src/service/test.rs

@@ -1017,6 +1017,7 @@ async fn override_member_opens_a_window_that_outranks_a_paused_gate() {
         }
     }
 
+    let _seams = crate::seam_lock::hold_global_seams_async().await;
     gate::clear_manual_override();
     gate::set_scheduler_gate(std::sync::Arc::new(PausedGate));
     assert!(matches!(gate::current_policy(), Policy::Paused { .. }));