fix: release lock before calling into DataCallbackManager

[YuanYuYuan]

Summary

SubscriptionData, ClientData, and ServiceData each hold their own mutex_ across the call into data_callback_mgr_ — both when a new message/reply/query arrives (trigger_callback()) and when a user registers a new callback (set_callback(), which can fire retroactively for already-queued data). data_callback_mgr_'s own callback can run arbitrary user code, including code that re-enters this class (e.g. to read queued data or to clear/replace the callback), and set_callback does not need mutex_ in the first place — data_callback_mgr_ has its own internal locking, and no other state guarded by mutex_ is touched in that function.

If the user code invoked from the callback ever executes on a thread that is also, directly or indirectly, contended for mutex_ — for example a Python binding where the callback runs while holding the GIL and a concurrent thread wants both the GIL and mutex_ — this is a genuine self-deadlock/AB-BA hazard: the same shape as a lock held across a callback that can call back into the same object.

A concrete, real trigger for this: rclpy's AsyncNode. SubscriptionData::add_new_message holds mutex_ while calling trigger_callback(), which (via AsyncNode) enters Python and blocks on the GIL. The asyncio event loop thread separately holds the GIL and blocks in take_one_message waiting for the same mutex_. This is a textbook cross-thread AB-BA cycle, not a hypothetical: measured on this exact path, unpatched code hangs on 12 of 30 runs at 200 Hz (Fisher exact, one-sided, p = 6.2e-05), against 0 of 30 with the fix applied.

This fixes all three entities by narrowing mutex_'s scope to just the data-structure mutation (queue push/pop, sequence-number bookkeeping) and releasing it before trigger_callback()/set_callback() run.

This PR is part of a small, independent set of public fixes for the same underlying defect shape (a lock held across a call into user/foreign code) across several projects in the zenoh/ROS 2 ecosystem, using lock-tripwire — a small debug-time detector for exactly this pattern — as the common diagnostic tool where the target language is Rust:

• eclipse-zenoh/zenoh#2781 — an independent session/runtime-callback fix in zenoh's own Rust core
• YuanYuYuan/zenoh#1 (stacked on zenoh's own #2744) — adds the tripwire's tracking mechanism to a related callback path
• ZettaScaleLabs/hiroz#351 and #352 — the identical release-before-call-out fix and an opt-in tripwire adoption, for rmw-zenoh-rs's Subscription/Service/Client types (a Rust RMW implementation of this same interface)

This PR is the C++ instance of that same fix pattern applied to rmw_zenoh_cpp directly; the tripwire itself doesn't run against C++ locks, so this fix was re-derived by reading the current source, not generated by the tool.

Before and after

	today	with this change
add_new_message/add_new_reply/add_new_query	holds mutex_ for the whole function body, including trigger_callback()	holds mutex_ only around the queue mutation; releases it before trigger_callback()
set_on_new_message_callback/set_on_new_response_callback/set_on_new_request_callback	holds mutex_ (unused inside the function) around data_callback_mgr_.set_callback()	does not acquire mutex_ at all — data_callback_mgr_ already serializes itself
a user callback that re-enters the same subscription/client/service on a thread also contending for mutex_	can deadlock	cannot deadlock through mutex_ (see Residual gap below)
early-return paths (is_shutdown_, is_shutdown_.load(...))	skip trigger_callback()	unchanged — still skip it, now via the nested block exiting before the callback line

What fails without this

No test in this repository currently reproduces the hang (see Verification below for why). The failure mode itself is not synthetic: rmw-zenoh-rs's Subscription/Service/Client types (linked above, ZettaScaleLabs/hiroz#351) had the structurally identical defect — a callback-invoking mutex held across a call into a GIL-holding Python runtime — reproduced with a deterministic hang, and fixed with the identical release-before-call-out pattern this PR applies here.

Residual, disclosed gap

DataCallbackManager (event.hpp/event.cpp) holds its own private event_mutex_ for the full duration of set_callback() and trigger_callback(), including while invoking the stored callback. This PR does not touch that lock. If a user callback re-enters DataCallbackManager on the same object (e.g. calling set_on_new_message_callback again from inside the callback it just fired), event_mutex_ itself could still self-deadlock. This is a narrower, harder-to-hit case (it requires the same callback to re-register itself), and fixing it is a larger change to DataCallbackManager's own locking discipline — left out of this PR to keep it minimal and reviewable. Happy to open a follow-up if maintainers want it addressed.

Verification

No full colcon build was performed for this PR. Building rmw_zenoh_cpp requires zenoh-cpp/zenoh-c, vendored from source via this repository's own zenoh_cpp_vendor package — disproportionate to stand up just to validate a lock-scoping change. Instead, this was verified by manually re-reading every changed function line by line, confirming brace balance and that every field accessed inside the original lock scope stays inside the new (narrower) one; confirming the early-return paths (is_shutdown_ in add_new_message, is_shutdown_.load(...) in add_new_query) are unchanged in effect — they still exit before trigger_callback() runs, now via the nested block ending rather than the whole function ending, and the lock_guard still releases correctly on that path via normal C++ scope-exit semantics; and git diff --check (no whitespace/tab issues).

This is weaker evidence than a compiled, executed test, and is stated here plainly rather than implied to be equivalent to one. If a maintainer would rather see this validated against an actual build/test, I'm happy to follow up.

Breaking changes

None. This changes only lock scope, not any function's signature, return value, or externally observable ordering guarantee.

Did you use Generative AI?

Yes. Claude Code was used for the whole thing: reading the current source to find and re-derive this defect shape, writing the fix across all three entities, and drafting this description. All source changes and claims here were reviewed line by line before pushing — see the Verification section above for exactly what was and wasn't checked.