Skip to content

Commit 55a4484

Browse files
Abderahmane BENALIclaude
authored andcommitted
docs: document the listener thread; narrow the no-threads guarantee
"No background threads" was asserted across DESIGN.md and README.md and is no longer unconditional: none for an executor that waits, one per-context listener thread for an application that registers a listener callback. Each assertion now says what it actually guarantees. DESIGN.md gains "The listener thread: delivery without a wait": why EventsExecutor deadlocks against wait-only delivery, the lazy start, the reordering and lost-wakeup hazards and what closes them (drain_mutex, the per-wait-set delivery fd), the listener_mutex lifetime rule and the restrictions it implies, that a callback may take from its own endpoint because callback_mutex is recursive, which context-wide locks exist, and what the thread deliberately is not. README, test/README and CHANGELOG follow. Squashed from: - docs: document the listener thread; narrow the no-threads guarantee - docs: fix "edge-driven", describe the per-wait-set delivery fd, drop #64's entry - docs: correct the drain-safety claim, and log the two blocker fixes - docs: sweep the claims this stack invalidated but the prose pass missed - docs: a callback may now take from its own endpoint - docs: callback_mutex is recursive; listener_mutex is not the only context lock Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NUNQNo26cKRPrVXcaHZnje
1 parent c3a739b commit 55a4484

4 files changed

Lines changed: 169 additions & 19 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,91 @@ All notable changes to `rmw_unix_socket_cpp` are documented here.
55
The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [Unreleased]
9+
10+
The EventsExecutor release. `rclcpp`'s `EventsExecutor` — the default in
11+
`performance_test` and `ros2-benchmark-container` — crashed on every pub/sub
12+
topology, and fixing the crash only turned it into a silent hang: that executor
13+
never calls `rmw_wait`, and delivery happened exclusively inside `rmw_wait`, so
14+
nothing ever drained its sockets. Callback-driven delivery now works, and the
15+
four copies of the receive loop that hid the gap are one.
16+
17+
### Added
18+
19+
- **Listener thread for callback-driven delivery.** An endpoint that registers
20+
a listener callback is watched by one per-context thread that drains its
21+
socket and fires the callback with no application thread involved. It is
22+
started **lazily**, by the first callback registration in the context, so an
23+
executor that waits registers nothing and the process still starts no
24+
background thread — the zero-thread property holds for
25+
`SingleThreadedExecutor` and `MultiThreadedExecutor` exactly as before.
26+
Joined in `rmw_shutdown`.
27+
- **`delivery_fd`, so a watched endpoint can still sit in a wait set.** The
28+
listener signals this eventfd strictly *after* enqueueing and `rmw_wait`
29+
drains it strictly *before* scanning its queues, which is what stops a wait
30+
from sleeping on a socket the listener already emptied. Same ordering pair as
31+
the registry doorbell. **One per wait set**, created in
32+
`rmw_create_wait_set`: reading an eventfd drains its whole counter, so a
33+
single shared fd is a single credit that whichever wait set wakes first
34+
consumes — including one that gained no work and discards it. Creating it
35+
with the wait set also means a callback registered while a wait is already
36+
blocked still has an armed fd to signal.
37+
38+
### Changed
39+
40+
- **One socket drain instead of four.** `drain_endpoint` replaces
41+
`drain_subscription`, `drain_socket`, and the inline copies in
42+
`rmw_take_request` and `rmw_take_response`. Each copy had been deciding its
43+
own policy, which is where the delivery gaps lived. `DrainTarget` now carries
44+
that policy per endpoint.
45+
- **Listener callbacks report a batch count.** One notification per drain
46+
carrying the number of entries the queue *gained*, rather than one call of
47+
`1` per datagram. `rmw/event_callback_type.h` defines `number_of_events` as
48+
the count since the callback was last called and allows `> 1`. The count is
49+
growth rather than datagrams read, because it is a take credit: an overflow
50+
that pushes 100 and pops 90 reports 10, not 100.
51+
- Request and response queues are capped at `SERVICE_QUEUE_DEPTH` (100)
52+
wherever they are filled. `rmw_wait` always applied that bound; the take-path
53+
drains had none.
54+
- `DESIGN.md` documents the listener thread and the `delivery_fd` ordering, and
55+
narrows the no-background-threads claim to what it now guarantees.
56+
57+
### Fixed
58+
59+
- **Registering a callback while the context shut down could abort or hang.**
60+
`listener_watch` read `is_shutdown` before taking `listener_mutex`, which
61+
`listener_stop` must release in order to join. A registration landing in that
62+
window restarted the listener: it move-assigned over a still-joinable
63+
`std::thread` (`std::terminate`) and replaced the epoll and wake descriptors,
64+
so the stop's wake-up went to an fd nobody polled and `rmw_shutdown` never
65+
returned. Both are closed by a `listener_stopping` flag checked under the
66+
mutex — `is_shutdown` alone does not cover it, because `rmw_context_fini`
67+
reaches `listener_stop` without setting it.
68+
- **Concurrent drains of one endpoint could reorder a publisher's messages.**
69+
`drain_endpoint` `recv`s outside `queue_mutex` and pushes inside it, so the
70+
listener thread and an `rmw_wait`/`rmw_take` could interleave and break the
71+
per-publisher FIFO ordering ROS 2 guarantees. A per-endpoint `drain_mutex`
72+
held for the whole drain serialises them.
73+
- **Services and clients were callback-dead after registration** (#61
74+
follow-up). `on_new_request_cb` and `on_new_response_cb` were stored and
75+
flushed once against an existing backlog, then never fired again by any drain.
76+
- **The `rmw_wait` drain notified nobody.** Only the `rmw_take` path fired a
77+
subscription's `on_new_message` callback, so a callback registered by an
78+
executor was silent for every message that arrived while a thread sat in
79+
`rmw_wait`. It also trimmed to the QoS depth without reporting the overflow.
80+
- **A backlog was flushed twice per registration.** `rclcpp` calls the rmw
81+
setter twice in a row on purpose — a stack temporary, then its permanent
82+
storage — so the flush in the three `set_on_new_*_callback` functions paid the
83+
same queued messages out for both calls, and a TRANSIENT_LOCAL subscription
84+
with ten replayed samples handed the executor twenty events. It now flushes
85+
only when a callback takes over from none.
86+
- **The three callback setters left the callback installed after
87+
`listener_watch` failed**, contradicting the error they returned. A failure
88+
now means the callback is not set.
89+
- **`listener_start` could throw out of an `extern "C"` entry point.**
90+
`std::thread` construction reports thread exhaustion as `std::system_error`;
91+
it is now caught, and the running flag it had already set is cleared.
92+
893
## [0.5.0] - 2026-08-27
994

1095
The wait/wakeup release. The 200 ms `rmw_wait` poll is gone, replaced by an

‎README.md‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@ A lightweight, **localhost-only** RMW (ROS Middleware) implementation for ROS 2,
44
built on Unix domain sockets and POSIX shared memory instead of DDS.
55

66
It targets single-machine deployments with many nodes (150+ in production) and a
7-
small, deterministic resource footprint — no DDS, no daemon, no background
8-
threads. Discovery is a shared-memory registry; data moves over `AF_UNIX`
7+
small, deterministic resource footprint — no DDS, no daemon, and no background
8+
thread unless your executor asks to be called back. Discovery is a shared-memory
9+
registry; data moves over `AF_UNIX`
910
datagram sockets; the wait set is `epoll` + `eventfd`.
1011

1112
## When to use it (and when not)
@@ -55,7 +56,8 @@ nodes that need to communicate must select the same RMW — there is no DDS brid
5556
| **Transport** | `SOCK_DGRAM` Unix domain sockets in `/tmp/ros2_uds/<domain_id>/`. Message boundaries are free; no connection management. |
5657
| **Discovery** | Single shared-memory file `/dev/shm/ros2_uds_<domain_id>`. No daemon, no multicast. |
5758
| **Locking** | Lock-free registry (per-slot seqlock + atomic state machine); process-local caches use a plain mutex. |
58-
| **Wait** | `epoll` over socket fds + `eventfd` guard conditions. **No background threads** — all I/O happens inside `rmw_wait`. |
59+
| **Wait** | `epoll` over socket fds + `eventfd` guard conditions. I/O happens inside `rmw_wait`, so a waiting executor gets **no background threads**. |
60+
| **Callbacks** | An endpoint that registers a listener callback is drained by one per-context listener thread, started on that first registration — what `EventsExecutor` needs, since it never calls `rmw_wait`. |
5961
| **Serialization** | CDR via `fastcdr` + `rosidl_typesupport_fastrtps` (the same path as the DDS RMWs). |
6062
| **Stale cleanup** | Dead processes are detected via `/proc/<pid>`; their registry entries and socket files are reclaimed on init and on every discovery query. |
6163

@@ -140,7 +142,11 @@ for the full rationale.
140142
Localhost only; Linux only; no DDS interoperability. Loaned (zero-copy) messages,
141143
dynamic message types, network flow endpoints, and content filtering return
142144
`RMW_RET_UNSUPPORTED`. `deadline`, `lifespan`, and `liveliness` QoS are accepted
143-
but not enforced. The full list and the reasoning behind each is in
145+
but not enforced, and no QoS status event (`matched`, incompatible-QoS,
146+
incompatible-type, `message_lost`) is generated — `rmw_event_type_is_supported`
147+
returns `false` for every type, so `rclcpp` refuses those event handlers rather
148+
than holding a dead one. The listener callbacks an `EventsExecutor` needs are
149+
supported. The full list and the reasoning behind each is in
144150
[DESIGN.md → Limitations and Unsupported Features](rmw_unix_socket_cpp/DESIGN.md#limitations-and-unsupported-features).
145151

146152
## Documentation

0 commit comments

Comments
 (0)