From 0b9be601aebd1b25a8e2e3635c400e76d0da97f6 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Mon, 30 Mar 2026 08:49:04 -0700 Subject: [PATCH 1/6] Don't call clear on backing data structure Signed-off-by: Michael Carlstrom --- .../rclcpp/experimental/buffers/ring_buffer_implementation.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/rclcpp/include/rclcpp/experimental/buffers/ring_buffer_implementation.hpp b/rclcpp/include/rclcpp/experimental/buffers/ring_buffer_implementation.hpp index 69ed8fcb1c..4867b080f0 100644 --- a/rclcpp/include/rclcpp/experimental/buffers/ring_buffer_implementation.hpp +++ b/rclcpp/include/rclcpp/experimental/buffers/ring_buffer_implementation.hpp @@ -231,7 +231,6 @@ class RingBufferImplementation : public BufferImplementationBase inline void clear_() { - ring_buffer_.clear(); size_ = 0; read_index_ = 0; write_index_ = capacity_ - 1; From a166d89cd397ff29b9fce136fe49e835e7dbfb33 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Tue, 31 Mar 2026 16:45:26 -0700 Subject: [PATCH 2/6] Add destructive test Signed-off-by: Michael Carlstrom --- .../test_ring_buffer_implementation.cpp | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp b/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp index 8a5e0be55e..7df41e83d4 100644 --- a/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp +++ b/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp @@ -165,6 +165,38 @@ TEST(TestRingBufferImplementation, test_buffer_clear) { EXPECT_EQ('d', d); } +namespace detail +{ + +struct Tracked { + static int destroyed; + int value; + + Tracked(int v) : value(v) {} + ~Tracked() { destroyed++; } +}; +} // detail namespace + +TEST(TestRingBufferImplementation, clear_is_non_destructive) +{ + detail::Tracked::destroyed = 0; + + rclcpp::experimental::buffers::RingBufferImplementation rb(2); + + rb.enqueue(detail::Tracked(1)); + rb.enqueue(detail::Tracked(2)); + + detail::Tracked::destroyed = 0; + + rb.clear(); + + EXPECT_EQ(detail::Tracked::destroyed, 0); + + // Outside buffer should be removed + rb.enqueue(detail::Tracked(3)); + EXPECT_EQ(detail::Tracked::destroyed, 1); +} + TEST(TestRingBufferImplementation, handle_nullptr_deletion) { rclcpp::experimental::buffers::RingBufferImplementation> rb(3); rb.enqueue(std::make_unique(42)); From c54efb587e62b81c93c85765a5347a148cb36f68 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Tue, 31 Mar 2026 18:04:00 -0700 Subject: [PATCH 3/6] Add default Signed-off-by: Michael Carlstrom --- rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp b/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp index 7df41e83d4..68c15520ee 100644 --- a/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp +++ b/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp @@ -172,6 +172,7 @@ struct Tracked { static int destroyed; int value; + Tracked() : value(0) {} Tracked(int v) : value(v) {} ~Tracked() { destroyed++; } }; From 7f0b7731a26a0cd3ac11b6d612c3d80cd2243841 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Tue, 31 Mar 2026 18:36:49 -0700 Subject: [PATCH 4/6] Add default Signed-off-by: Michael Carlstrom --- rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp b/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp index 68c15520ee..f8b91315de 100644 --- a/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp +++ b/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp @@ -169,7 +169,7 @@ namespace detail { struct Tracked { - static int destroyed; + inline static int destroyed = 0; int value; Tracked() : value(0) {} From 0079c431dde34b5079440fe9d640c0b148d90d28 Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Tue, 31 Mar 2026 21:55:11 -0700 Subject: [PATCH 5/6] Lint Signed-off-by: Michael Carlstrom --- .../test/rclcpp/test_ring_buffer_implementation.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp b/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp index f8b91315de..61bf1acc1b 100644 --- a/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp +++ b/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp @@ -168,15 +168,18 @@ TEST(TestRingBufferImplementation, test_buffer_clear) { namespace detail { -struct Tracked { +struct Tracked +{ inline static int destroyed = 0; int value; - Tracked() : value(0) {} - Tracked(int v) : value(v) {} - ~Tracked() { destroyed++; } + Tracked() + : value(0) {} + explicit Tracked(int v) + : value(v) {} + ~Tracked() {destroyed++;} }; -} // detail namespace +} // namespace detail TEST(TestRingBufferImplementation, clear_is_non_destructive) { From 6ddd6734df72e195aa8eba90660cbd31a7ff97af Mon Sep 17 00:00:00 2001 From: Michael Carlstrom Date: Wed, 1 Apr 2026 08:18:19 -0700 Subject: [PATCH 6/6] Change check for msvc Signed-off-by: Michael Carlstrom --- rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp b/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp index 61bf1acc1b..687c34428b 100644 --- a/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp +++ b/rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp @@ -198,7 +198,7 @@ TEST(TestRingBufferImplementation, clear_is_non_destructive) // Outside buffer should be removed rb.enqueue(detail::Tracked(3)); - EXPECT_EQ(detail::Tracked::destroyed, 1); + EXPECT_GE(detail::Tracked::destroyed, 1); } TEST(TestRingBufferImplementation, handle_nullptr_deletion) {