Fix review findings: pause ref-count invariant, QoS adaptation, and more

[facontidavide]

Summary

Fixes the app-core and ROS2 findings from a full-application code review. All changes are covered by new unit tests written before the fixes (TDD): 174 tests pass (159 baseline + 15 new), TSAN and ASAN clean — including a 2000-iteration two-thread stress test for the pause/disconnect race.

Correctness fixes

• Pause/subscription ref-count invariant (3 related bugs) — sessions now track per-topic middleware refs (ref_held), and every subscription state transition (subscribe, unsubscribe, pause, resume, disconnect cleanup) runs under cleanup_mutex_. Fixes:
  • unsubscribe-while-paused double-decrementing shared ref counts (destroying subscriptions other clients still use)
  • subscribe-while-paused acquiring a ref that resume acquired again → permanent leak after disconnect
  • the pause vs disconnect race where both paths released the same refs
• Resume preserves subscriptions — topics missing from discovery at resume time (publisher briefly down) stay in the session and are re-acquired on a later resume, per the documented pause contract; reported via a new unavailable_topics response field.
• QoS adaptation (GenericSubscriptionManager) — subscriptions now match publisher QoS (BEST_EFFORT if any publisher offers it, TRANSIENT_LOCAL if all do). Previously the fixed RELIABLE/VOLATILE QoS(100) silently received zero messages from SensorDataQoS publishers (cameras, lidars, IMUs) and never got latched samples (/tf_static, /map).
• executor.spin() instead of a spin_some() busy-loop — the ROS2 bridge no longer pins a CPU core at 100% while idle.
• Rate-limit hardening — client-supplied max_rate_hz clamped to [0.001, 1e6] Hz (values above ~2.1e6 Hz previously hit UB in an int cast; values below 1 mHz silently meant unlimited). Re-subscribing with a bare topic string now preserves an existing rate limit instead of resetting it to unlimited.
• Request queue drain — process_requests() drains pending requests (bounded at 256/call) instead of handling one per 10 ms tick, removing the ~100 req/s throughput cap and the heartbeat starvation it enabled; receive_request() is now truly non-blocking (was parking the executor up to 10 ms per empty poll).
• WebSocket middleware teardown — client callbacks copy the server shared_ptr under state_mutex_ (fixes a shutdown TOCTOU null dereference); the stop thread is joined in the destructor instead of detached (a detached thread running IXWebSocket teardown past main() is UB).
• MessageBuffer TTL — a backwards wall-clock step (NTP) no longer wraps the unsigned age computation and purges every buffered message; the clock is injectable for tests.
• Schema extractor — bounded strings (string<=256) no longer make whole-schema extraction fail; new try_get_message_definition() distinguishes extraction failure from legitimately empty definitions, so std_msgs/msg/Empty topics can finally be subscribed.

Docs

• docs/API.md: rate clamping, bare-string semantics, actual rate-limit selection behavior (first eligible message), resume unavailable_topics.
• CLAUDE.md: wire-format section now documents the 16-byte frame header (previously said "no header", contradicting the code and API.md); stale test count fixed.

Test plan

• 174/174 unit tests pass (RoboStack humble env)
• TSAN build: 174/174, no new reports (suppressions unchanged)
• ASAN build: 174/174
• pre-commit run -a clean
• Every bug fix has a test that was watched failing before the fix

🤖 Generated with Claude Code