[benaliabderrahmane]

Full set of fixes from a multi-agent code audit of rmw_unix_socket_cpp (method + findings in AUDIT_FINAL.md). Each fix was drafted by three independent agents and reconciled by a selector. The complete stack builds cleanly: from-scratch colcon build --packages-select rmw_unix_socket_cpp → 0 errors, 0 warnings.

This PR is the integration of the whole stack (base main). The three P1 + two P2 shortlist bugs also have dedicated focused PRs for individual review/CI:

• P1-1 serialize() exception boundary — fix(serialization): contain fastcdr/std exceptions in serialize() [P1] #4
• P1-2 registry seqlock publication race — fix(registry): publish slot state only after payload commits under the seqlock [P1] #5
• P1-3 security_options double-free — fix(init): deep-copy security_options in rmw_init_options_copy [P1] #6
• P2-2 rmw_take_sequence write cursor — fix(subscription): write rmw_take_sequence output at the taken cursor [P2] #7
• P2-10 rmw_wait timeout overflow — fix(wait): compute rmw_wait timeout in int64 to preserve infinite blocking [P2] #8

Additional fixes in this PR (24)

P2

• P2-1 broaden deserialize() catch to std::exception (oversize-length input can't cross extern "C")
• P2-4 deep-copy discovery_options to stop aliasing/leaks (incl. build-fix: rmw_discovery_options_copy needs a non-const allocator)
• P2-5 enforce RMW_CHECK_TYPE_IDENTIFIERS_MATCH on all service/client entry points
• P2-6 error (not silent RMW_RET_OK) when rmw_send_response can't find the client
• P2-8 roll back rmw_create_node on strdup failure
• P2-11 re-arm epoll every wait (fixes the recycled-fd "permanently deaf entity" hang)
• P2-12 free node-name arrays on enclave-init failure
• P2-14 static_assert the on-wire/shm layout (regression guard)

P3

• P3-1 ABA re-check before cleanup_stale reclaim · P3-3 identifier checks (subscription/graph/node) · P3-4 retry epoll_wait on EINTR · P3-6 atomic last_registry_generation · P3-8 null-check guard condition · P3-9 single eventfd read per wait · P3-11 skip a corrupt request/response datagram · P3-12 pre-scan generation cache · P3-13 graph strdup checks · P3-15 init identifier/double-init guards · P3-17 guard node->context in destroy paths · P3-18 size_t serialize length · P3-23 append QoS-incompatibility reasons

perf / cleanup

• PERF-7 move (not copy) the TRANSIENT_LOCAL payload
• dead-code cleanup: verified-dead includes/statements, RMW_GID_STORAGE_SIZE

Deferred (need a build-iteration loop or a design decision — not in this PR)

PERF-1/2/3/4/5/6/8, rcpputils dependency removal, write-only struct fields / dead generation block, the decision-required findings (TRANSIENT_LOCAL wait-path replay, service/client queue-cap policy, serialization_format string, is_shutdown enforcement, …), and the missing-test additions (P2-15..18).

[benaliabderrahmane]

Review: blocking correctness bug + a systemic test gap

Thanks for the thorough audit-fix stack -- it builds clean (0 warnings) on jazzy/kilted/rolling and most fixes are well-reasoned. One change must be corrected before merge, plus a process gap worth addressing.

Blocking

rmw_send_response returns RMW_RET_ERROR on a legitimate client-gone miss -- rmw_unix_socket_cpp/src/rmw_service.cpp:299-300

After the new authoritative re-query (287-298) fails, a true miss does RMW_SET_ERROR_MSG + return RMW_RET_ERROR. That ret (RMW_RET_ERROR=1 -> RCL_RET_ERROR) makes rclcpp Service::send_response throw an unhandled C++ exception from inside the executor callback. This is reachable in normal operation: a client issues a request, times out, and shuts down before a slow handler responds -> its registry entry is removed -> true miss -> the service node crashes. On a 150-node vessel an impatient client over a slow service can destabilize the server.

The reference rmw_cyclonedds handles this exact case deliberately (rmw_node.cpp:4872-4873): RMW_RET_OK on client_present_t::GONE, RMW_RET_TIMEOUT on MAYBE -- never RMW_RET_ERROR for a client that left. The re-query you added only rescues the stale-but-present case; the common "client legitimately left" case is still a true miss. Please return RMW_RET_OK (or at most RMW_RET_TIMEOUT) on a true miss to match DDS semantics.

Strongly recommended (not blocking the merge by itself)

No tests added across the whole bundle (rmw_unix_socket_cpp/test/) -- git log origin/main..audit-fixes and gh pr diff 9 --name-only show zero test files touched, despite 3 P1 + many concurrency/correctness fixes. A full gtest suite already exists, so a reproducer per bug is cheap and idiomatic (CLAUDE.md S4 / failing-test-first). The blocker above is the perfect example: a 4-line test (request, drop client, send_response -> expect non-throwing OK) would have caught it. CI green only re-runs pre-existing tests that don't exercise the new paths.

Non-blocking, worth a look

• ABA window in registry_cleanup_stale (registry.cpp:375-386): the new seq re-check is a big improvement but closes the window only up to, not through, the state CAS -- a remove+live-re-add between the seq load (375) and the CAS (381) can still tear down a live entry, and the comment at 371-373 overclaims ("not been touched since the snapshot"). Suggest tightening the comment.
• PERF-7 now fans out under cache_mutex (rmw_publisher.cpp:244-252): the new current-message send loop + early return sit inside the lock scope, so the per-destination sendmsg fanout runs while holding cache_mutex for TRANSIENT_LOCAL pubs (non-TRANSIENT stays lock-free). No deadlock risk, but the body's "move (not copy)" one-liner doesn't mention the restructure.
• P3-17 incomplete (rmw_subscription.cpp:215, rmw_client.cpp:150, rmw_service.cpp:150): the node->context guard landed in only 2 of 5 destroy paths; these three still deref node->context->options.allocator directly. Body says "destroy paths" (plural).
• P2-6/P3-11/P3-23 ship untested behavior changes, and rmw_features.cpp:73,83 flips two stale comments in-area (minor S3). The QoS append arithmetic itself checks out (no off-by-one), but test_rmw_qos.cpp asserts only the compat enum, never reason[].
• Credit: the P2-14 static_assert layout guards are the one fix that ships with its own (compile-time) verification -- nice.

Happy to re-review once the send_response ret is corrected; ideally with the small reproducer.