From a42824ff400454669e5a9ce24bcfdfe7924eaa16 Mon Sep 17 00:00:00 2001
From: yuanyuyuan <az6980522@gmail.com>
Date: Tue, 28 Jul 2026 02:16:17 +0800
Subject: [PATCH 1/7] feat(hiroz): debug-time barrier against callbacks under a
 lock
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

hiroz has repeatedly shipped the same defect: a user callback invoked while
a non-reentrant hiroz lock guard is still live on the calling thread. Any
callback that re-enters hiroz there blocks on a lock its own thread already
holds. It needs no race and is reachable from ordinary public API.

Add `reentrancy`, a debug-only tripwire for the invariant "a user callback
is never invoked while a hiroz lock guard is live". `TrackedMutex` and
`TrackedRwLock` wrap the `std::sync` types and count live guards in a
thread-local; `invoke_user_callback!(site, call)` asserts the count is zero
and names the site before dispatching. Field order is fixed so the inner
guard releases before the counter decrements. Everything is behind
`debug_assertions` and compiles to nothing in release; tests and CI run in
debug, which is where the assertion is wanted.

A cheaper option was tried first and does not work.
`clippy::significant_drop_in_scrutinee` reports zero hits on code known to
contain the defect, because the shape is `if let Ok(cb) = holder.lock()`,
which *binds* the guard rather than leaving it a scrutinee temporary. The
defect is a dynamic guard lifetime spanning a dynamic dispatch through
`Arc<dyn Fn>` or an `extern "C" fn` pointer; static analysis sees through
neither, a counter sees both.

The module carries a unit test that asserts the tripwire panics while a
guard is live, so the detector cannot silently rot into a no-op.

Two adjacent fixes are needed for this barrier to actually run:

- `hiroz-tests` was linted under interop features but tested under none.
  Several of its suites are `#![cfg(feature = "ros-msgs")]`, and a
  crate-level cfg that is not satisfied compiles to an empty test binary
  reporting `0 passed` — indistinguishable from green. `run-tests` now runs
  the workspace excluding `hiroz-tests`, then `hiroz-tests` with
  `ros-msgs,jazzy`; coverage gets the same features for the same reason.
  `feature_gate.rs` is not gated, so a featureless build cannot compile it
  away, and it fails with a message naming the cause.

- `check-local.nu` (the local gate) was missing four checks that
  `check-local.sh` (what CI runs) has: check-hu, test-shm,
  check-distro-features and the rustdoc intra-doc link check, and neither
  passed `--all` to `cargo fmt`, which skips `hiroz-py` and `rmw-zenoh-rs`.
  The rustdoc detector was validated by injecting a broken link: rc=0 clean,
  rc=1 broken, rc=0 restored. The five new intra-doc links in
  `reentrancy.rs` are fully qualified so they resolve under that gate. The
  three pre-existing unresolved links in `error.rs` are a separate defect on
  main and are fixed in their own change, not buried here.
---
 .github/workflows/ci.yml                 |   4 +
 crates/hiroz-tests/tests/feature_gate.rs |  51 ++++
 crates/hiroz/src/lib.rs                  |   3 +
 crates/hiroz/src/reentrancy.rs           | 303 +++++++++++++++++++++++
 scripts/check-local.nu                   |  10 +-
 scripts/check-local.sh                   |   4 +-
 scripts/test-pure-rust.nu                |   9 +-
 7 files changed, 380 insertions(+), 4 deletions(-)
 create mode 100644 crates/hiroz-tests/tests/feature_gate.rs
 create mode 100644 crates/hiroz/src/reentrancy.rs

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 9e1710c2c..0d74a8f24 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -276,9 +276,13 @@ jobs:
       - name: Run coverage
         run: |
           source /tmp/nix-dev-env.sh
+          # hiroz-tests needs its own features: several suites are
+          # `#![cfg(feature = "ros-msgs")]` and compile to empty binaries without
+          # them, so coverage taken blind under-reports the paths they exercise.
           cargo llvm-cov \
             -p hiroz -p hiroz-codegen -p hiroz-cdr -p hiroz-protocol -p hiroz-schema \
             -p hiroz-tests \
+            --features hiroz-tests/ros-msgs,hiroz-tests/jazzy \
             -j4 \
             --lcov --output-path lcov.info
         shell: bash
diff --git a/crates/hiroz-tests/tests/feature_gate.rs b/crates/hiroz-tests/tests/feature_gate.rs
new file mode 100644
index 000000000..17ae621c1
--- /dev/null
+++ b/crates/hiroz-tests/tests/feature_gate.rs
@@ -0,0 +1,51 @@
+//! Fails loudly when `hiroz-tests` is built without the features its suites need.
+//!
+//! Several suites in this crate are `#![cfg(feature = "ros-msgs")]` — most
+//! importantly `reentrant_service.rs`, which is the only coverage the
+//! callback-under-lock deadlocks in the service and parameter paths have. A
+//! crate-level `cfg` that is not satisfied does not error and does not warn: the
+//! file compiles to an empty test binary that reports `0 passed`, which is
+//! indistinguishable from green in every runner and every CI log.
+//!
+//! That is exactly how those four tests stopped running without anyone noticing.
+//! `scripts/test-pure-rust.nu` linted `hiroz-tests` under interop features but
+//! *tested* it under none, so the clippy step saw the code and the test step did
+//! not.
+//!
+//! This file is deliberately **not** gated, so it is the one thing in the crate
+//! that a featureless build cannot compile away. If the features go missing
+//! again, the run fails with a message naming the cause instead of quietly
+//! shrinking.
+//!
+//! Consequence, stated so it is a decision rather than a surprise: `hiroz-tests`
+//! has no supported featureless configuration. Run it as
+//! `cargo test -p hiroz-tests --features ros-msgs,jazzy` (or with `ros-interop`
+//! for the suites that also need a ROS installation).
+
+/// Without `ros-msgs`, the gated suites are silently absent — fail instead.
+#[test]
+#[cfg(not(feature = "ros-msgs"))]
+fn ros_msgs_gated_suites_must_not_be_silently_skipped() {
+    panic!(
+        "hiroz-tests was built without the `ros-msgs` feature.\n\
+         \n\
+         The suites gated on it — `reentrant_service.rs` among them — have been \
+         compiled to empty test binaries and will report `0 passed`, which reads \
+         as green. That is not a pass; it is no coverage.\n\
+         \n\
+         Build the crate with its features:\n\
+         \n\
+             cargo test -p hiroz-tests --features ros-msgs,jazzy\n\
+         \n\
+         Suites that additionally drive a real ROS 2 installation need \
+         `--features ros-interop,<distro>` instead."
+    );
+}
+
+/// With `ros-msgs`, record that the gate was satisfied.
+///
+/// Present so the guard is visible in the test list in *both* configurations —
+/// a check whose only evidence is the absence of a failure is not a check.
+#[test]
+#[cfg(feature = "ros-msgs")]
+fn ros_msgs_gated_suites_are_compiled_in() {}
diff --git a/crates/hiroz/src/lib.rs b/crates/hiroz/src/lib.rs
index 71d671916..516604464 100644
--- a/crates/hiroz/src/lib.rs
+++ b/crates/hiroz/src/lib.rs
@@ -79,6 +79,9 @@ pub mod python_bridge;
 pub mod qos;
 /// Internal message queues.
 pub mod queue;
+
+/// Debug-time enforcement of "no user callback under a hiroz lock guard".
+pub mod reentrancy;
 /// Message type metadata traits (`WithTypeInfo`, etc.).
 pub mod ros_msg;
 /// ROS 2 service client and server.
diff --git a/crates/hiroz/src/reentrancy.rs b/crates/hiroz/src/reentrancy.rs
new file mode 100644
index 000000000..4bd77057b
--- /dev/null
+++ b/crates/hiroz/src/reentrancy.rs
@@ -0,0 +1,303 @@
+//! Debug-time enforcement of hiroz's one hard rule about user callbacks:
+//!
+//! > **A user callback is never invoked while a hiroz lock guard is live.**
+//!
+//! Every re-entrancy deadlock hiroz has had was one violation of that sentence.
+//! A callback invoked under a guard is user code running inside hiroz's own
+//! critical section, and the first thing such code usually does is call back
+//! into hiroz — re-acquiring a non-reentrant `Mutex`/`RwLock` on the thread that
+//! already holds it. There is no race to lose; it is a deterministic hang.
+//!
+//! The rule was previously enforced by review. That does not scale, and it
+//! demonstrably leaked: a mechanical sweep of this workspace found further
+//! instances in `rmw-zenoh-rs` after five had already been fixed by hand.
+//!
+//! # Why a runtime tripwire and not a lint
+//!
+//! The obvious candidate, `clippy::significant_drop_in_scrutinee`, was tried and
+//! **does not fire on this code at all**. It targets guards that are unnamed
+//! temporaries in a `match`/`if let` scrutinee; hiroz's (and rmw's) shape is
+//! `if let Ok(cb) = holder.lock()`, where the guard is *bound* to a name and so
+//! is not a scrutinee temporary. Verified: `cargo clippy -p rmw-zenoh-rs -W
+//! clippy::significant_drop_in_scrutinee` reports zero hits on code containing
+//! five genuine instances of the defect.
+//!
+//! A bespoke lint fares no better. The defect is a *dynamic* guard lifetime
+//! spanning a *dynamic* dispatch through `Arc<dyn Fn>` or an `extern "C" fn`
+//! pointer. Static analysis cannot see through either, and the FFI pointers are
+//! opaque by construction.
+//!
+//! A counter can see both, trivially.
+//!
+//! # Cost
+//!
+//! Zero in release. [`GuardCount`](crate::reentrancy::GuardCount) is a unit
+//! struct whose constructor and `Drop` compile to nothing without
+//! `debug_assertions`, and
+//! [`assert_no_guards_held`](crate::reentrancy::assert_no_guards_held)
+//! expands to nothing. Tests and CI run in debug, which is where the assertion
+//! is wanted.
+//!
+//! # Using it
+//!
+//! Lock declarations on a user-callback-reachable path use
+//! [`TrackedMutex`](crate::reentrancy::TrackedMutex) /
+//! [`TrackedRwLock`](crate::reentrancy::TrackedRwLock) instead of the
+//! `std::sync` originals. Every site that invokes user code calls
+//! [`assert_no_guards_held`](crate::reentrancy::assert_no_guards_held) first — in practice via
+//! [`invoke_user_callback!`], which does both in one line and names the site in
+//! the panic message.
+
+use std::sync::{LockResult, Mutex, MutexGuard, RwLock, RwLockReadGuard, RwLockWriteGuard};
+
+#[cfg(debug_assertions)]
+thread_local! {
+    /// How many tracked hiroz guards are live on this thread right now.
+    static LIVE_GUARDS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
+}
+
+/// RAII counter embedded in every tracked guard.
+#[derive(Debug)]
+pub struct GuardCount;
+
+impl GuardCount {
+    #[inline(always)]
+    fn new() -> Self {
+        #[cfg(debug_assertions)]
+        LIVE_GUARDS.with(|n| n.set(n.get() + 1));
+        Self
+    }
+}
+
+impl Drop for GuardCount {
+    #[inline(always)]
+    fn drop(&mut self) {
+        #[cfg(debug_assertions)]
+        LIVE_GUARDS.with(|n| n.set(n.get().saturating_sub(1)));
+    }
+}
+
+/// Number of tracked hiroz guards live on this thread. Always 0 in release.
+#[inline(always)]
+pub fn live_guards() -> usize {
+    #[cfg(debug_assertions)]
+    {
+        LIVE_GUARDS.with(|n| n.get())
+    }
+    #[cfg(not(debug_assertions))]
+    {
+        0
+    }
+}
+
+/// Panics (debug builds only) if any tracked hiroz guard is live on this thread.
+///
+/// Call immediately before invoking user code. `site` names the call site and is
+/// reproduced in the panic message, because the useful information when this
+/// fires is *which* callback was about to run, not the backtrace of the counter.
+#[inline(always)]
+pub fn assert_no_guards_held(site: &str) {
+    #[cfg(debug_assertions)]
+    {
+        let live = live_guards();
+        assert!(
+            live == 0,
+            "hiroz re-entrancy rule violated at `{site}`: about to invoke a user \
+             callback with {live} hiroz lock guard(s) still live on this thread. \
+             A callback that re-enters hiroz here will deadlock. Collect what you \
+             need under the lock, drop the guard, then call — see \
+             `GraphEventManager::trigger_graph_change` for the pattern."
+        );
+    }
+    #[cfg(not(debug_assertions))]
+    let _ = site;
+}
+
+/// Assert the re-entrancy rule, then invoke user code.
+///
+/// ```ignore
+/// invoke_user_callback!("EventsManager::set_callback backlog", callback(count));
+/// ```
+#[macro_export]
+macro_rules! invoke_user_callback {
+    ($site:expr, $call:expr) => {{
+        $crate::reentrancy::assert_no_guards_held($site);
+        $call
+    }};
+}
+
+// ---------------------------------------------------------------------------
+// Tracked lock types
+// ---------------------------------------------------------------------------
+
+/// A `std::sync::Mutex` whose guards are counted by [`live_guards`].
+#[derive(Debug, Default)]
+pub struct TrackedMutex<T>(Mutex<T>);
+
+impl<T> TrackedMutex<T> {
+    pub fn new(value: T) -> Self {
+        Self(Mutex::new(value))
+    }
+
+    pub fn lock(&self) -> LockResult<TrackedMutexGuard<'_, T>> {
+        match self.0.lock() {
+            Ok(inner) => Ok(TrackedMutexGuard {
+                inner,
+                _count: GuardCount::new(),
+            }),
+            Err(poisoned) => Err(std::sync::PoisonError::new(TrackedMutexGuard {
+                inner: poisoned.into_inner(),
+                _count: GuardCount::new(),
+            })),
+        }
+    }
+}
+
+/// Guard for [`TrackedMutex`]. Field order matters: `inner` is declared first so
+/// it is released before the counter decrements, never the other way round.
+#[derive(Debug)]
+pub struct TrackedMutexGuard<'a, T> {
+    inner: MutexGuard<'a, T>,
+    _count: GuardCount,
+}
+
+impl<T> std::ops::Deref for TrackedMutexGuard<'_, T> {
+    type Target = T;
+    fn deref(&self) -> &T {
+        &self.inner
+    }
+}
+
+impl<T> std::ops::DerefMut for TrackedMutexGuard<'_, T> {
+    fn deref_mut(&mut self) -> &mut T {
+        &mut self.inner
+    }
+}
+
+/// A `std::sync::RwLock` whose guards are counted by [`live_guards`].
+#[derive(Debug, Default)]
+pub struct TrackedRwLock<T>(RwLock<T>);
+
+impl<T> TrackedRwLock<T> {
+    pub fn new(value: T) -> Self {
+        Self(RwLock::new(value))
+    }
+
+    pub fn read(&self) -> LockResult<TrackedReadGuard<'_, T>> {
+        match self.0.read() {
+            Ok(inner) => Ok(TrackedReadGuard {
+                inner,
+                _count: GuardCount::new(),
+            }),
+            Err(p) => Err(std::sync::PoisonError::new(TrackedReadGuard {
+                inner: p.into_inner(),
+                _count: GuardCount::new(),
+            })),
+        }
+    }
+
+    pub fn write(&self) -> LockResult<TrackedWriteGuard<'_, T>> {
+        match self.0.write() {
+            Ok(inner) => Ok(TrackedWriteGuard {
+                inner,
+                _count: GuardCount::new(),
+            }),
+            Err(p) => Err(std::sync::PoisonError::new(TrackedWriteGuard {
+                inner: p.into_inner(),
+                _count: GuardCount::new(),
+            })),
+        }
+    }
+}
+
+#[derive(Debug)]
+pub struct TrackedReadGuard<'a, T> {
+    inner: RwLockReadGuard<'a, T>,
+    _count: GuardCount,
+}
+
+impl<T> std::ops::Deref for TrackedReadGuard<'_, T> {
+    type Target = T;
+    fn deref(&self) -> &T {
+        &self.inner
+    }
+}
+
+#[derive(Debug)]
+pub struct TrackedWriteGuard<'a, T> {
+    inner: RwLockWriteGuard<'a, T>,
+    _count: GuardCount,
+}
+
+impl<T> std::ops::Deref for TrackedWriteGuard<'_, T> {
+    type Target = T;
+    fn deref(&self) -> &T {
+        &self.inner
+    }
+}
+
+impl<T> std::ops::DerefMut for TrackedWriteGuard<'_, T> {
+    fn deref_mut(&mut self) -> &mut T {
+        &mut self.inner
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn guards_are_counted_and_released() {
+        let m = TrackedMutex::new(1u32);
+        assert_eq!(live_guards(), 0);
+        {
+            let g = m.lock().unwrap();
+            assert_eq!(*g, 1);
+            assert_eq!(live_guards(), 1);
+            {
+                let rw = TrackedRwLock::new(2u32);
+                let _r = rw.read().unwrap();
+                assert_eq!(live_guards(), 2);
+            }
+            assert_eq!(live_guards(), 1);
+        }
+        assert_eq!(live_guards(), 0);
+    }
+
+    #[test]
+    fn assert_passes_with_no_guards() {
+        assert_no_guards_held("test");
+    }
+
+    /// The detector must actually detect. Without this, a tripwire that never
+    /// fires is indistinguishable from a codebase with no defects.
+    #[test]
+    #[cfg_attr(debug_assertions, should_panic(expected = "re-entrancy rule violated"))]
+    fn assert_fires_while_a_guard_is_live() {
+        let m = TrackedMutex::new(0u32);
+        let _g = m.lock().unwrap();
+        assert_no_guards_held("deliberate violation");
+        // In release the assertion is compiled out, so the test must not be
+        // expected to panic there.
+        #[cfg(not(debug_assertions))]
+        assert_eq!(live_guards(), 0);
+    }
+
+    #[test]
+    fn a_poisoned_guard_is_still_counted() {
+        let m = std::sync::Arc::new(TrackedMutex::new(0u32));
+        let m2 = m.clone();
+        let _ = std::thread::spawn(move || {
+            let _g = m2.lock().unwrap();
+            panic!("poison it");
+        })
+        .join();
+
+        let guard = m.lock();
+        assert!(guard.is_err(), "expected the mutex to be poisoned");
+        let recovered = guard.unwrap_or_else(|e| e.into_inner());
+        assert_eq!(live_guards(), 1, "a recovered poisoned guard must count");
+        drop(recovered);
+        assert_eq!(live_guards(), 0);
+    }
+}
diff --git a/scripts/check-local.nu b/scripts/check-local.nu
index 7aa68984e..87cf50446 100755
--- a/scripts/check-local.nu
+++ b/scripts/check-local.nu
@@ -56,7 +56,7 @@ def main [--suite: string = "full"] {
     if $suite == "lint" {
         log-header "Running hiroz Lint Checks"
         let checks = [
-            {name: "Formatting (cargo fmt)", cmd: "cargo fmt --check"},
+            {name: "Formatting (cargo fmt)", cmd: "cargo fmt --all --check"},
             {name: "Clippy (all targets)", cmd: "cargo clippy --all-targets -- -D warnings"},
         ]
         let results = $checks | enumerate | each {|item|
@@ -70,7 +70,7 @@ def main [--suite: string = "full"] {
     log-header "Running hiroz Pre-Submission Checks"
 
     let checks = [
-        {name: "Formatting (cargo fmt)", cmd: "cargo fmt --check"},
+        {name: "Formatting (cargo fmt)", cmd: "cargo fmt --all --check"},
         {name: "Clippy (all targets)", cmd: "cargo clippy --all-targets -- -D warnings"},
         {name: "Build (cargo build)", cmd: "cargo build --examples"},
         {name: "Tests", cmd: (if (which cargo-nextest | is-not-empty) {
@@ -78,6 +78,12 @@ def main [--suite: string = "full"] {
         } else {
             "cargo test --lib --tests"
         })},
+        # The four checks below exist in scripts/check-local.sh (which remote CI
+        # runs) but were absent here, so they could only ever fail remotely.
+        {name: "hu clippy (check-hu)", cmd: "nu scripts/test-pure-rust.nu check-hu"},
+        {name: "SHM tests (test-shm)", cmd: "nu scripts/test-pure-rust.nu test-shm"},
+        {name: "Distro feature flags (check-distro-features)", cmd: "nu scripts/test-pure-rust.nu check-distro-features"},
+        {name: "Rustdoc links (cargo doc)", cmd: "let r = (^cargo doc --no-deps -p hiroz --quiet | complete); let w = ($r.stderr | lines | where $it =~ 'unresolved link'); if ($w | is-not-empty) { print ($w | str join (char newline)); error make {msg: 'rustdoc: unresolved intra-doc links'} }"},
     ]
 
     let results = $checks | enumerate | each {|item|
diff --git a/scripts/check-local.sh b/scripts/check-local.sh
index a580f1b0f..d4b72d1b1 100755
--- a/scripts/check-local.sh
+++ b/scripts/check-local.sh
@@ -35,7 +35,9 @@ run_check() {
 }
 
 # 1. Check formatting
-run_check "Formatting (cargo fmt)" "cargo fmt --check"
+# --all is required: without it cargo fmt only covers the default workspace
+# members, silently skipping hiroz-py and rmw-zenoh-rs.
+run_check "Formatting (cargo fmt)" "cargo fmt --all --check"
 
 # 2. Clippy lints
 run_check "Clippy (all targets)" "cargo clippy --all-targets -- -D warnings"
diff --git a/scripts/test-pure-rust.nu b/scripts/test-pure-rust.nu
index 6e85db857..2bd1e202d 100755
--- a/scripts/test-pure-rust.nu
+++ b/scripts/test-pure-rust.nu
@@ -19,7 +19,14 @@ def run-tests [] {
     $env.RUSTFLAGS = "-D warnings"
 
     log-step "Run tests"
-    run-cmd "cargo nextest run --no-fail-fast"
+    # `hiroz-tests` is excluded here and run separately below *with* its features.
+    # Several of its suites — `reentrant_service.rs` most importantly — are
+    # `#![cfg(feature = "ros-msgs")]`, and a crate-level cfg that is not satisfied
+    # compiles to an empty test binary reporting `0 passed`. That reads as green.
+    # None of those features need a ROS installation (hiroz-msgs bundles the
+    # definitions), so there is no reason for this job to run the crate blind.
+    run-cmd "cargo nextest run --no-fail-fast --workspace --exclude hiroz-tests"
+    run-cmd "cargo nextest run --no-fail-fast -p hiroz-tests --features ros-msgs,jazzy"
 }
 
 def check-bundled-msgs [] {

From 0bd010a8dc2e9461882d0bf678363063c1dac281 Mon Sep 17 00:00:00 2001
From: yuanyuyuan <az6980522@gmail.com>
Date: Tue, 28 Jul 2026 17:48:13 +0800
Subject: [PATCH 2/7] fix(reentrancy): make GuardCount unforgeable, widen the
 local rustdoc gate

`GuardCount` was a fieldless unit struct, so any code able to name it could
construct one -- and therefore drop one. `Drop` decrements the thread-local,
so a stray `drop(GuardCount)` while a tracked guard was live took the count to
zero, `assert_no_guards_held` then passed, and a genuine callback-under-lock
went unreported. `saturating_sub` made that desync permanently silent. A
private field means only this module can mint one, so the count moves only by
acquiring and releasing a real guard. The module docs called it a unit struct;
they now say zero-sized newtype.

`check-local.nu`'s rustdoc gate matched only `unresolved link`, while
`check-local.sh:95` -- what CI runs -- also matches `broken_intra_doc_links`.
A diagnostic carrying the lint name but not that phrase passed locally and
failed remotely, which is the specific way this gate has already wasted CI
cycles on this branch series. Both patterns now, as the shell gate does.

No behaviour change: the four `reentrancy::tests` still pass, including
`assert_fires_while_a_guard_is_live`, which is the one that proves the counter
still detects.
---
 crates/hiroz/src/reentrancy.rs | 17 +++++++++++++----
 scripts/check-local.nu         |  2 +-
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/crates/hiroz/src/reentrancy.rs b/crates/hiroz/src/reentrancy.rs
index 4bd77057b..b8c71a0de 100644
--- a/crates/hiroz/src/reentrancy.rs
+++ b/crates/hiroz/src/reentrancy.rs
@@ -31,8 +31,8 @@
 //!
 //! # Cost
 //!
-//! Zero in release. [`GuardCount`](crate::reentrancy::GuardCount) is a unit
-//! struct whose constructor and `Drop` compile to nothing without
+//! Zero in release. [`GuardCount`](crate::reentrancy::GuardCount) is a
+//! zero-sized newtype whose constructor and `Drop` compile to nothing without
 //! `debug_assertions`, and
 //! [`assert_no_guards_held`](crate::reentrancy::assert_no_guards_held)
 //! expands to nothing. Tests and CI run in debug, which is where the assertion
@@ -57,15 +57,24 @@ thread_local! {
 }
 
 /// RAII counter embedded in every tracked guard.
+///
+/// The private field is what makes the counter trustworthy. As a fieldless unit
+/// struct this was constructible — and therefore *droppable* — by any code that
+/// could name it, and `Drop` decrements the thread-local. A stray
+/// `drop(GuardCount)` while a tracked guard was live would take the count to
+/// zero, `assert_no_guards_held` would pass, and a genuine callback-under-lock
+/// would go unreported. `saturating_sub` guaranteed that desync was silent.
+/// Only this module can mint one now, so the count can only be moved by
+/// acquiring and releasing a real guard.
 #[derive(Debug)]
-pub struct GuardCount;
+pub struct GuardCount(());
 
 impl GuardCount {
     #[inline(always)]
     fn new() -> Self {
         #[cfg(debug_assertions)]
         LIVE_GUARDS.with(|n| n.set(n.get() + 1));
-        Self
+        Self(())
     }
 }
 
diff --git a/scripts/check-local.nu b/scripts/check-local.nu
index 87cf50446..1d80e0da8 100755
--- a/scripts/check-local.nu
+++ b/scripts/check-local.nu
@@ -83,7 +83,7 @@ def main [--suite: string = "full"] {
         {name: "hu clippy (check-hu)", cmd: "nu scripts/test-pure-rust.nu check-hu"},
         {name: "SHM tests (test-shm)", cmd: "nu scripts/test-pure-rust.nu test-shm"},
         {name: "Distro feature flags (check-distro-features)", cmd: "nu scripts/test-pure-rust.nu check-distro-features"},
-        {name: "Rustdoc links (cargo doc)", cmd: "let r = (^cargo doc --no-deps -p hiroz --quiet | complete); let w = ($r.stderr | lines | where $it =~ 'unresolved link'); if ($w | is-not-empty) { print ($w | str join (char newline)); error make {msg: 'rustdoc: unresolved intra-doc links'} }"},
+        {name: "Rustdoc links (cargo doc)", cmd: "let r = (^cargo doc --no-deps -p hiroz --quiet | complete); let w = ($r.stderr | lines | where {|it| ($it =~ 'unresolved link') or ($it =~ 'broken_intra_doc_links')}); if ($w | is-not-empty) { print ($w | str join (char newline)); error make {msg: 'rustdoc: unresolved intra-doc links'} }"},
     ]
 
     let results = $checks | enumerate | each {|item|

From dd615e091c5f5ad951b65a51bc294a20f156e477 Mon Sep 17 00:00:00 2001
From: yuanyuyuan <az6980522@gmail.com>
Date: Tue, 28 Jul 2026 19:01:42 +0800
Subject: [PATCH 3/7] fix(tests): enforce the feature requirement package-wide

`tests/feature_gate.rs` only fails when Cargo happens to select that target.
`cargo test -p hiroz-tests --test reentrant_service` with no features builds
only that target, the crate-level `cfg` compiles it to an empty binary, `0
passed` is reported, and the guard never runs -- so the silent-skip mode it
exists to close stayed reachable through the narrower invocation.

A build script runs for every build of the package whatever targets are
selected, so that is the one place the requirement holds everywhere. Verified
both ways: `cargo check -p hiroz-tests --tests` exits 101 with the message,
and with `--features ros-msgs,jazzy` exits 0.

Note `cargo check -p hiroz-tests` *without* `--tests` proves nothing here --
the crate has no lib target, so cargo builds nothing and never runs the build
script at all. That invocation reports success on a crate it did not compile.

`scripts/test-pure-rust.nu`'s `shm_example` step named no features; it now
does. `shm_example` itself is ungated, but the crate no longer has a supported
featureless configuration, which `feature_gate.rs` already declared.
---
 crates/hiroz-tests/build.rs | 29 +++++++++++++++++++++++++++++
 scripts/test-pure-rust.nu   |  5 ++++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/crates/hiroz-tests/build.rs b/crates/hiroz-tests/build.rs
index 159e0b9fb..edc8cca37 100644
--- a/crates/hiroz-tests/build.rs
+++ b/crates/hiroz-tests/build.rs
@@ -1,6 +1,35 @@
 use std::{env, path::PathBuf};
 
 fn main() {
+
+    // Package-wide enforcement of the feature requirement.
+    //
+    // `tests/feature_gate.rs` fails loudly when the crate is built without
+    // `ros-msgs`, but only if Cargo happens to select *that* target. Ask for one
+    // gated suite on its own -- `cargo test -p hiroz-tests --test
+    // reentrant_service` with no features -- and Cargo builds only that target,
+    // the crate-level `cfg` compiles it to an empty binary, `0 passed` is
+    // reported, and the guard never runs. The silent-skip mode the guard exists
+    // to close is still reachable through the narrower invocation.
+    //
+    // A build script runs for every build of this package regardless of which
+    // targets are selected, so this is the only place the requirement can be
+    // stated once and hold everywhere.
+    //
+    // Consequence, already declared in `feature_gate.rs`: `hiroz-tests` has no
+    // supported featureless configuration.
+    if std::env::var_os("CARGO_FEATURE_ROS_MSGS").is_none() {
+        panic!(
+            "\n\nhiroz-tests requires the `ros-msgs` feature.\n\n\
+             Without it the suites gated on it compile to empty test binaries \n\
+             that report `0 passed`, which reads as green but is no coverage.\n\n\
+             Build it as:\n\n    \
+             cargo test -p hiroz-tests --features ros-msgs,jazzy\n\n\
+             Suites that drive a real ROS 2 installation need \n    \
+             --features ros-interop,<distro> instead.\n"
+        );
+    }
+
     // Declare custom cfg for ROS version detection
     println!("cargo::rustc-check-cfg=cfg(ros_humble)");
 
diff --git a/scripts/test-pure-rust.nu b/scripts/test-pure-rust.nu
index 2bd1e202d..f08fddcde 100755
--- a/scripts/test-pure-rust.nu
+++ b/scripts/test-pure-rust.nu
@@ -87,7 +87,10 @@ def test-shm [] {
     # Integration-style unit tests (pub/sub with SHM)
     run-cmd "cargo test --package hiroz --test shm"
     # Integration tests (validate shm_pointcloud2 example)
-    run-cmd "cargo test --package hiroz-tests --test shm_example"
+    # `hiroz-tests` has no featureless configuration -- its build script now
+    # enforces that -- so this names the features even though shm_example
+    # itself is ungated.
+    run-cmd "cargo test --package hiroz-tests --test shm_example --features ros-msgs,jazzy"
 }
 
 # ============================================================================

From 8a8651a7fb790b55039f8e20e7e6a8aa34297e56 Mon Sep 17 00:00:00 2001
From: yuanyuyuan <az6980522@gmail.com>
Date: Tue, 28 Jul 2026 20:14:46 +0800
Subject: [PATCH 4/7] fix(ci): lint hiroz-tests with its features, satisfy
 pre-commit rustfmt

Two things the GitHub pre-commit gate caught that the local runner does not.

`scripts/test-ros.nu` ran `cargo clippy --all-targets --workspace -F rmw`,
which selects `hiroz-tests` with no features. Its suites are
`#![cfg(feature = "ros-msgs")]`, so clippy was linting a set of empty files
and reporting success -- the same hollow result the new build-script guard
exists to refuse. That guard turned this from a silent no-op into a hard
failure, which is the point. Exclude the crate from the workspace pass and
lint it separately with `ros-interop,<distro>`, matching how the same script
already builds and tests it.

Also drops a stray blank line in `crates/hiroz-tests/build.rs` that the
pinned stable rustfmt in the pre-commit hook rejects. Worth noting the local
`cargo fmt --all --check` does not catch it: CI runs
`nix build .#checks...pre-commit-check`, a different and stricter gate.
---
 crates/hiroz-tests/build.rs | 1 -
 scripts/test-ros.nu         | 8 +++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/crates/hiroz-tests/build.rs b/crates/hiroz-tests/build.rs
index edc8cca37..15c24f9ae 100644
--- a/crates/hiroz-tests/build.rs
+++ b/crates/hiroz-tests/build.rs
@@ -1,7 +1,6 @@
 use std::{env, path::PathBuf};
 
 fn main() {
-
     // Package-wide enforcement of the feature requirement.
     //
     // `tests/feature_gate.rs` fails loudly when the crate is built without
diff --git a/scripts/test-ros.nu b/scripts/test-ros.nu
index a0ff8c265..2b1af6ed5 100755
--- a/scripts/test-ros.nu
+++ b/scripts/test-ros.nu
@@ -22,7 +22,13 @@ def clippy-rmw [] {
     }
 
     log-step "Clippy (rmw feature)"
-    run-cmd "cargo clippy --all-targets --workspace -F rmw -- -D warnings"
+    # `hiroz-tests` is excluded and linted separately with its features. Under
+    # plain `--workspace` it is selected with none, and its suites are
+    # `#![cfg(feature = "ros-msgs")]` — so clippy would lint a set of empty
+    # files and report success. Its build script now refuses that configuration
+    # outright rather than let it look clean.
+    run-cmd "cargo clippy --all-targets --workspace --exclude hiroz-tests -F rmw -- -D warnings"
+    run-cmd $"cargo clippy --all-targets -p hiroz-tests --features ros-interop,($distro) -- -D warnings"
 }
 
 def run-ros-interop [] {

From 10b1b58991d3fea3374919f057552381f9e70d11 Mon Sep 17 00:00:00 2001
From: yuanyuyuan <az6980522@gmail.com>
Date: Tue, 28 Jul 2026 20:43:38 +0800
Subject: [PATCH 5/7] fix(ci): keep rmw-zenoh-rs out of the ROS-independent
 test run

`run-tests` gained `--workspace --exclude hiroz-tests` so the gated suites
would actually run. That also pulled in `rmw-zenoh-rs`, whose build script
generates bindings from ROS C headers -- so the ROS-*independent* job now
fails with `'rcutils/strdup.h' file not found` before running a single test.

`rmw-zenoh-rs` is not in `default-members`, so it was never built by this job
before `--workspace` was introduced. Excluding it restores the previous
selection rather than dropping coverage; the ROS jobs build and lint it via
`-F rmw`.

This did not show up on the local runner because its dev shell patches the
workspace member list to drop that crate. GitHub's runner does not, which is
why the wider matrix caught it.
---
 scripts/test-pure-rust.nu | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/test-pure-rust.nu b/scripts/test-pure-rust.nu
index f08fddcde..2503f50ec 100755
--- a/scripts/test-pure-rust.nu
+++ b/scripts/test-pure-rust.nu
@@ -25,7 +25,12 @@ def run-tests [] {
     # compiles to an empty test binary reporting `0 passed`. That reads as green.
     # None of those features need a ROS installation (hiroz-msgs bundles the
     # definitions), so there is no reason for this job to run the crate blind.
-    run-cmd "cargo nextest run --no-fail-fast --workspace --exclude hiroz-tests"
+    # `rmw-zenoh-rs` is excluded too: its build script generates bindings from
+    # ROS C headers (`rcutils/strdup.h`), which this job by definition does not
+    # have. It is not in `default-members`, so it was never built here before
+    # `--workspace` was introduced -- excluding it restores that, rather than
+    # dropping coverage. The ROS jobs build and lint it via `-F rmw`.
+    run-cmd "cargo nextest run --no-fail-fast --workspace --exclude hiroz-tests --exclude rmw-zenoh-rs"
     run-cmd "cargo nextest run --no-fail-fast -p hiroz-tests --features ros-msgs,jazzy"
 }
 

From 69326f00a161b7609ce2e3d65bc71291af4858b2 Mon Sep 17 00:00:00 2001
From: yuanyuyuan <az6980522@gmail.com>
Date: Tue, 28 Jul 2026 21:09:11 +0800
Subject: [PATCH 6/7] fix(ci): skip the SHM-size suite in the ROS-independent
 test run

`--workspace` also pulled in `hiroz-msgs`, whose `shm_size_estimation` suite
asks the OS for a POSIX shared-memory segment big enough for a PointCloud2.
A GitHub runner's /dev/shm cannot provide one: the failure is `OS error 12`
(ENOMEM) raised inside `zenoh-shm` before any hiroz code runs. Three tests
fail for a property of the machine, not of the change under test.

Skip that binary rather than exclude the whole crate, so the rest of
`hiroz-msgs` keeps the coverage `--workspace` was added to gain. SHM itself
is still covered by the dedicated `test-shm` step, which allocates segments
a runner can serve.

Like `rmw-zenoh-rs`, `hiroz-msgs` is not in `default-members`, so none of
this ran in this job before `--workspace` was introduced -- these are
newly-surfaced, not newly-broken.
---
 scripts/test-pure-rust.nu | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/scripts/test-pure-rust.nu b/scripts/test-pure-rust.nu
index 2503f50ec..ac7c1a1c0 100755
--- a/scripts/test-pure-rust.nu
+++ b/scripts/test-pure-rust.nu
@@ -30,7 +30,15 @@ def run-tests [] {
     # have. It is not in `default-members`, so it was never built here before
     # `--workspace` was introduced -- excluding it restores that, rather than
     # dropping coverage. The ROS jobs build and lint it via `-F rmw`.
-    run-cmd "cargo nextest run --no-fail-fast --workspace --exclude hiroz-tests --exclude rmw-zenoh-rs"
+    # `shm_size_estimation` is skipped, not excluded for convenience: it asks the
+    # OS for a POSIX shared-memory segment large enough to hold a PointCloud2,
+    # and a GitHub runner's `/dev/shm` cannot provide it -- the failure is
+    # `OS error 12` (ENOMEM) from `zenoh-shm`, before any hiroz code runs. It is
+    # a property of the machine, not of the change under test. SHM is covered by
+    # the dedicated `test-shm` step, which uses segments a runner can allocate.
+    # `hiroz-msgs` is not in `default-members`, so these never ran in this job
+    # until `--workspace` was introduced here.
+    run-cmd "cargo nextest run --no-fail-fast --workspace --exclude hiroz-tests --exclude rmw-zenoh-rs -E 'not binary(shm_size_estimation)'"
     run-cmd "cargo nextest run --no-fail-fast -p hiroz-tests --features ros-msgs,jazzy"
 }
 

From a70f7336acf3d0f38909fa0ed40b20a419846705 Mon Sep 17 00:00:00 2001
From: yuanyuyuan <az6980522@gmail.com>
Date: Thu, 30 Jul 2026 02:20:44 +0800
Subject: [PATCH 7/7] fix(reentrancy): stop citing a defect as the exemplar,
 drop invented history

Review found three false claims shipped in code, not just prose.

1. The panic message told developers to "see GraphEventManager::
   trigger_graph_change for the pattern". On this branch that function holds
   trigger_guard_condition AND graph_guard_conditions across trigger(gc), an
   opaque extern "C" pointer -- it is a live instance of the defect the
   tripwire just caught. It only becomes the pattern after pr/4b rewrites it.
   The pattern is now stated inline instead of cross-referenced.

2. feature_gate.rs, build.rs and test-pure-rust.nu all cited
   reentrant_service.rs as the most important gated suite, and asserted "that
   is exactly how those four tests stopped running without anyone noticing".
   That file does not exist on main or on this branch -- it arrives with
   pr/3. Those tests never ran, so they cannot have stopped running. Now
   cites the ten suites that actually are gated here.

3. The "0 passed" story conflated two mechanisms. default-members excludes
   hiroz-tests, so a bare `cargo nextest run` never built the crate at all --
   it was not built empty. The empty-compilation mode is real but reachable
   only when the crate IS selected without features. Both are now stated, and
   kept distinct.
---
 crates/hiroz-tests/build.rs              | 10 ++++----
 crates/hiroz-tests/tests/feature_gate.rs | 29 +++++++++++++++---------
 crates/hiroz/src/reentrancy.rs           |  7 +++---
 scripts/test-pure-rust.nu                |  6 ++++-
 4 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/crates/hiroz-tests/build.rs b/crates/hiroz-tests/build.rs
index 15c24f9ae..c1b0188ac 100644
--- a/crates/hiroz-tests/build.rs
+++ b/crates/hiroz-tests/build.rs
@@ -5,11 +5,11 @@ fn main() {
     //
     // `tests/feature_gate.rs` fails loudly when the crate is built without
     // `ros-msgs`, but only if Cargo happens to select *that* target. Ask for one
-    // gated suite on its own -- `cargo test -p hiroz-tests --test
-    // reentrant_service` with no features -- and Cargo builds only that target,
-    // the crate-level `cfg` compiles it to an empty binary, `0 passed` is
-    // reported, and the guard never runs. The silent-skip mode the guard exists
-    // to close is still reachable through the narrower invocation.
+    // gated suite on its own -- `cargo test -p hiroz-tests --test cache` with no
+    // features -- and Cargo builds only that target, the crate-level `cfg`
+    // compiles it to an empty binary, `0 passed` is reported, and the guard
+    // never runs. The silent-skip mode the guard exists to close is still
+    // reachable through the narrower invocation.
     //
     // A build script runs for every build of this package regardless of which
     // targets are selected, so this is the only place the requirement can be
diff --git a/crates/hiroz-tests/tests/feature_gate.rs b/crates/hiroz-tests/tests/feature_gate.rs
index 17ae621c1..c3d7c2c3e 100644
--- a/crates/hiroz-tests/tests/feature_gate.rs
+++ b/crates/hiroz-tests/tests/feature_gate.rs
@@ -1,16 +1,22 @@
 //! Fails loudly when `hiroz-tests` is built without the features its suites need.
 //!
-//! Several suites in this crate are `#![cfg(feature = "ros-msgs")]` — most
-//! importantly `reentrant_service.rs`, which is the only coverage the
-//! callback-under-lock deadlocks in the service and parameter paths have. A
-//! crate-level `cfg` that is not satisfied does not error and does not warn: the
-//! file compiles to an empty test binary that reports `0 passed`, which is
-//! indistinguishable from green in every runner and every CI log.
+//! Ten suites in this crate are `#![cfg(feature = "ros-msgs")]` — `cache.rs`,
+//! `lifecycle.rs`, `parameter_tests.rs`, `service_schema_discovery.rs`,
+//! `subscriber_timeout.rs`, `type_description_integration.rs`, the `z_*_example`
+//! suites, and others. A crate-level `cfg` that is not satisfied does not error
+//! and does not warn: the file compiles to an empty test binary reporting
+//! `0 passed`, which is indistinguishable from green in every runner and log.
 //!
-//! That is exactly how those four tests stopped running without anyone noticing.
-//! `scripts/test-pure-rust.nu` linted `hiroz-tests` under interop features but
-//! *tested* it under none, so the clippy step saw the code and the test step did
-//! not.
+//! Two separate mechanisms could hide that, and they are worth keeping distinct:
+//!
+//! * **Selection.** `Cargo.toml` sets
+//!   `default-members = ["crates/hiroz", "crates/hiroz-codegen"]`, so a bare
+//!   `cargo nextest run` never selected `hiroz-tests` at all — the crate was not
+//!   built, rather than built empty. `scripts/test-pure-rust.nu` now names the
+//!   crate explicitly.
+//! * **Empty compilation.** Ask for the crate without features and the gated
+//!   suites really do compile away to `0 passed`. That is what this file and the
+//!   build script guard against.
 //!
 //! This file is deliberately **not** gated, so it is the one thing in the crate
 //! that a featureless build cannot compile away. If the features go missing
@@ -29,7 +35,8 @@ fn ros_msgs_gated_suites_must_not_be_silently_skipped() {
     panic!(
         "hiroz-tests was built without the `ros-msgs` feature.\n\
          \n\
-         The suites gated on it — `reentrant_service.rs` among them — have been \
+         The suites gated on it — `cache.rs`, `lifecycle.rs`, \
+         `parameter_tests.rs` and others — have been \
          compiled to empty test binaries and will report `0 passed`, which reads \
          as green. That is not a pass; it is no coverage.\n\
          \n\
diff --git a/crates/hiroz/src/reentrancy.rs b/crates/hiroz/src/reentrancy.rs
index b8c71a0de..0a7084e52 100644
--- a/crates/hiroz/src/reentrancy.rs
+++ b/crates/hiroz/src/reentrancy.rs
@@ -113,9 +113,10 @@ pub fn assert_no_guards_held(site: &str) {
             live == 0,
             "hiroz re-entrancy rule violated at `{site}`: about to invoke a user \
              callback with {live} hiroz lock guard(s) still live on this thread. \
-             A callback that re-enters hiroz here will deadlock. Collect what you \
-             need under the lock, drop the guard, then call — see \
-             `GraphEventManager::trigger_graph_change` for the pattern."
+             A callback that re-enters hiroz here may deadlock, and will if it \
+             touches a lock this thread already holds. The fix is always the \
+             same shape: collect what you need into an owned value, drop every \
+             guard, then invoke the callback."
         );
     }
     #[cfg(not(debug_assertions))]
diff --git a/scripts/test-pure-rust.nu b/scripts/test-pure-rust.nu
index ac7c1a1c0..62998bcce 100755
--- a/scripts/test-pure-rust.nu
+++ b/scripts/test-pure-rust.nu
@@ -20,9 +20,13 @@ def run-tests [] {
 
     log-step "Run tests"
     # `hiroz-tests` is excluded here and run separately below *with* its features.
-    # Several of its suites — `reentrant_service.rs` most importantly — are
+    # Ten of its suites (`cache.rs`, `lifecycle.rs`, `parameter_tests.rs`,
+    # `service_schema_discovery.rs`, `subscriber_timeout.rs` and others) are
     # `#![cfg(feature = "ros-msgs")]`, and a crate-level cfg that is not satisfied
     # compiles to an empty test binary reporting `0 passed`. That reads as green.
+    # Separately, the crate is not in `default-members`, so before this change a
+    # bare `cargo nextest run` did not build it at all -- naming it explicitly is
+    # what makes the features load-bearing rather than moot.
     # None of those features need a ROS installation (hiroz-msgs bundles the
     # definitions), so there is no reason for this job to run the crate blind.
     # `rmw-zenoh-rs` is excluded too: its build script generates bindings from