Skip to content

Commit 12f5509

Browse files
fix(tl): stop the replay watermark at an unresolvable record
max_seq was fixed before the enqueue loop, so a record whose descriptor could no longer be resolved — its slot overwritten and its durable segment unlinked between the scan and the mmap — was skipped while its sequence stayed under the watermark. At 1-64 KiB the same sample's live datagram is inline and deliverable, and dedup dropped it: the sample was lost, not lapped, despite the comment claiming otherwise. A watermark cannot express a hole, so it stops below the unresolved sequence and the rest of the history arrives as datagrams. Reported by Copilot on #60.
1 parent 64ff3db commit 12f5509

2 files changed

Lines changed: 83 additions & 1 deletion

File tree

‎rmw_unix_socket_cpp/src/rmw_subscription.cpp‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,13 @@ rmw_subscription_t * rmw_create_subscription(
315315
if (!rmw_uds::shm_resolve_incoming(
316316
pull_cache, ctx->domain_id, msg.header, msg.payload))
317317
{
318-
continue; // large payload's segment already evicted: lapped
318+
// The slot was overwritten and its segment unlinked between the
319+
// scan and this resolve, so the sample is not delivered here. A
320+
// watermark cannot express a hole: it stops below this sequence
321+
// and the rest arrives as datagrams — claiming the sequence would
322+
// drop the in-flight datagram carrying that very sample.
323+
max_seq = rec.sequence_number - 1;
324+
break;
319325
}
320326
msg.received_timestamp_ns = now_ns;
321327
sub_data->message_queue.push_back(std::move(msg));

‎rmw_unix_socket_cpp/test/test_rmw_qos.cpp‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,13 @@
1818
#include <atomic>
1919
#include <cstdlib>
2020
#include <cstring>
21+
#include <set>
22+
#include <string>
2123
#include <thread>
2224
#include <chrono>
2325

26+
#include <dirent.h>
27+
#include <sys/mman.h>
2428
#include <sys/socket.h>
2529
#include <unistd.h>
2630

@@ -1387,6 +1391,78 @@ TEST_F(QosTest, TransientLocalWatermarkDropsForgedDuplicate)
13871391
auto _r2 [[maybe_unused]] = rmw_destroy_publisher(node, pub);
13881392
}
13891393

1394+
TEST_F(QosTest, ReplayWatermarkStopsAtAnUnresolvableRecord)
1395+
{
1396+
// An over-cap latched record whose durable segment is unlinked after the
1397+
// scan copied its descriptor cannot be resolved, so it is never delivered.
1398+
// The watermark must not claim its sequence: the same sample's live
1399+
// datagram is inline at this size and may be in flight, and dedup would
1400+
// drop it — losing the sample outright. Unlinking the segments before the
1401+
// pull stands in for the concurrent overwrite that produces this state.
1402+
auto seq_ts = rosidl_typesupport_cpp::get_message_type_support_handle<
1403+
test_msgs::msg::UnboundedSequences>();
1404+
auto qos = make_qos(
1405+
RMW_QOS_POLICY_RELIABILITY_RELIABLE,
1406+
RMW_QOS_POLICY_DURABILITY_TRANSIENT_LOCAL, 5);
1407+
1408+
char prefix[64];
1409+
std::snprintf(prefix, sizeof(prefix), "ros2_uds_data_%zu_%d_",
1410+
static_cast<size_t>(node->context->actual_domain_id),
1411+
static_cast<int>(getpid()));
1412+
auto data_segments = [&prefix]() {
1413+
std::set<std::string> names;
1414+
DIR * dir = opendir("/dev/shm");
1415+
if (dir == nullptr) {
1416+
return names;
1417+
}
1418+
while (auto * entry = readdir(dir)) {
1419+
if (std::strncmp(entry->d_name, prefix, std::strlen(prefix)) == 0) {
1420+
names.insert(std::string("/") + entry->d_name);
1421+
}
1422+
}
1423+
closedir(dir);
1424+
return names;
1425+
};
1426+
const auto before = data_segments();
1427+
1428+
auto pub_opts = rmw_get_default_publisher_options();
1429+
auto * pub = rmw_create_publisher(node, seq_ts, "/wm_unresolvable", &qos, &pub_opts);
1430+
ASSERT_NE(nullptr, pub);
1431+
1432+
test_msgs::msg::UnboundedSequences msg;
1433+
msg.uint8_values.resize(4 * 1024); // over TL_EMBED_CAP: staged durably
1434+
for (int i = 1; i <= 3; ++i) { // sequences 1..3
1435+
msg.uint8_values[0] = static_cast<uint8_t>(i);
1436+
EXPECT_EQ(RMW_RET_OK, rmw_publish(pub, &msg, nullptr));
1437+
}
1438+
1439+
size_t evicted = 0;
1440+
for (const auto & name : data_segments()) {
1441+
if (before.count(name) == 0) {
1442+
shm_unlink(name.c_str());
1443+
++evicted;
1444+
}
1445+
}
1446+
ASSERT_GT(evicted, 0u) << "no durable segment was staged for the latched samples";
1447+
1448+
auto sub_opts = rmw_get_default_subscription_options();
1449+
auto * sub = rmw_create_subscription(node, seq_ts, "/wm_unresolvable", &qos, &sub_opts);
1450+
ASSERT_NE(nullptr, sub);
1451+
1452+
auto * pub_data = static_cast<rmw_uds::UdsPublisher *>(pub->data);
1453+
auto * sub_data = static_cast<rmw_uds::UdsSubscription *>(sub->data);
1454+
std::array<uint8_t, 16> key;
1455+
std::memcpy(key.data(), pub_data->gid.data, key.size());
1456+
{
1457+
std::lock_guard<std::mutex> lock(sub_data->queue_mutex);
1458+
EXPECT_EQ(0, sub_data->replayed_watermarks[key])
1459+
<< "the watermark claimed sequences the replay never delivered";
1460+
}
1461+
1462+
auto _r1 [[maybe_unused]] = rmw_destroy_subscription(node, sub);
1463+
auto _r2 [[maybe_unused]] = rmw_destroy_publisher(node, pub);
1464+
}
1465+
13901466
TEST_F(QosTest, TransientLocalTwoPublishersBothHistoriesPulled)
13911467
{
13921468
// Two latched publishers on one topic: a late joiner must receive both

0 commit comments

Comments
 (0)