Do not count sequence number 0 against the reliable send window

[victorpaleologue]

Summary

Until a matched reliable reader sends its first ACKNACK, the reliable send window counts one sample that does not exist. The window holds one sample fewer than its limit, and a writer whose window is 1 refuses its first write. This makes the acknowledgement frontier never go below 1, which is what the TODO in Writer::refresh_acked_frontier asks for.

The bug

RtpsReaderProxy starts with all_acked_before: SequenceNumber::zero() in all three constructors (new, from_reader, from_discovered_reader_data), and keeps it until the reader's first ACKNACK. handle_ack_nack never stores anything below 1 — it clamps readerSNState.base with max(base, SequenceNumber::from(1)), and its comment explains that base 1 is how a reader says "nothing received".

Writer::update_reader_proxy calls refresh_acked_frontier as soon as a reader matches. That passes the minimum acked_up_to_before() over the reliable readers — 0 for a reader that has not acknowledged yet — to WriterSendBuffer::set_acked_frontier, which stores it as acked_before, replacing the buffer's own initial value of 1.

has_room then computes

let unacked = i64::from(inner.last_seq) - i64::from(inner.acked_before) + 1;
unacked < inner.window_limit as i64

On a fresh writer that is 0 - 0 + 1 = 1: one sample in flight before any has been written. So, until every reliable reader has sent an ACKNACK:

• the window admits window_limit - 1 samples, and
• a writer with a window of 1 (ResourceLimits { max_samples: 1, .. }) refuses its first write: it blocks for max_blocking_time, then returns WouldBlock ("reliable send window full").

Since 0.14.0 the window is ResourceLimits::max_samples (8192 when unset), so with default QoS this costs one slot out of 8192. In 0.13, where the window was the KeepLast depth, it caused visible failures in ROS 2. ros2-client publishes with reliable KeepLast(1) QoS in several places. Its action status writer (transient-local, KeepLast(1)) timed out when a goal was accepted, and the ros_discovery_info writer returned WouldBlock regularly.

The fix

set_acked_frontier reads a frontier below 1 as 1 (max(sn, SequenceNumber::new(1))). That is the same floor handle_ack_nack applies to an ACKNACK, so it is the value the frontier takes after the reader's first ACKNACK anyway.

The fix is in the send buffer rather than in the reader proxy on purpose. The heartbeat tick and the repair path compare last_change < rp.all_acked_before directly, and starting proxies at 1 would change their behaviour on a writer that has not written yet — for example, the periodic heartbeat tick would stop sending to a fresh reader. Clamping where the window arithmetic lives changes flow control only. The TODO in refresh_acked_frontier keeps its other point, slow first-match repair.

Tests

The new unit test writer_send_buffer::tests::unacknowledged_reader_frontier_leaves_full_window creates a reliable buffer with a window of 1 and sets the frontier to Some(SequenceNumber::zero()), as a matched reader that has not ACKNACKed yet does. It then checks that the first write is admitted, the second is refused, and an ACKNACK frontier of 2 makes room again. On master (c076919) the first assertion fails. With this change the test passes.

• cargo test -- --test-threads=1: 694 + 2 + 2 + 58 pass
• cargo test --features=security -- --test-threads=1: 704 + 2 + 2 + 58 pass
• These are all clean: cargo +nightly fmt -- --check, cargo +nightly clippy --all --lib --bins --examples, cargo +nightly clippy --tests --examples (also with --all-features), cargo doc, and cargo +1.88.0 check

I did not add a two-participant test. The window is wrong only between a match and the reader's first ACKNACK, which comes back within a round trip of the HEARTBEAT the writer sends on match. A test that writes inside that interval depends on timing, so it would not be deterministic.

Questions for reviewers

These are related, but this PR does not change them:

1. Late-joining readers. The frontier is the minimum over the readers, and a newly matched reader starts at 0 (1 with this change). For a volatile writer, matched_reader_update marks every existing sequence number as pending GAP for the new reader, because the reader is not owed those samples. They still count against the window until that reader's first ACKNACK. Take a writer that has written window_limit samples or more, even with all of them acknowledged by the current readers. When a new reliable reader matches, the writer refuses writes until that ACKNACK arrives. Should a new volatile reader's flow-control frontier start at last_seq + 1, or the send buffer's first_seq, instead?
2. Back-pressure under KEEP_LAST. Should a KEEP_LAST writer block on unacknowledged samples at all? DDS 1.4 §2.2.3.14 lets a reliable write block if the modification would cause data to be lost or a RESOURCE_LIMITS limit to be exceeded. Under KEEP_LAST, replacing the oldest sample of an instance is what the history policy asks for, so arguably it is not loss and does not need to block. Since 0.14.0 the window comes from max_samples, so this is rarely visible, but a KEEP_LAST writer with max_samples set still blocks.

🤖 Generated with Claude Code