Skip to content

Commit a525a24

Browse files
Fix incorrect internal clear inside RingBufferImplementation (#3116)
* Don't call clear on backing data structure Signed-off-by: Michael Carlstrom <rmc@carlstrom.com> * Add destructive test Signed-off-by: Michael Carlstrom <rmc@carlstrom.com> * Add default Signed-off-by: Michael Carlstrom <rmc@carlstrom.com> * Add default Signed-off-by: Michael Carlstrom <rmc@carlstrom.com> * Lint Signed-off-by: Michael Carlstrom <rmc@carlstrom.com> * Change check for msvc Signed-off-by: Michael Carlstrom <rmc@carlstrom.com> --------- Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
1 parent c516367 commit a525a24

2 files changed

Lines changed: 36 additions & 1 deletion

File tree

rclcpp/include/rclcpp/experimental/buffers/ring_buffer_implementation.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,6 @@ class RingBufferImplementation : public BufferImplementationBase<BufferT>
231231

232232
inline void clear_()
233233
{
234-
ring_buffer_.clear();
235234
size_ = 0;
236235
read_index_ = 0;
237236
write_index_ = capacity_ - 1;

rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,42 @@ TEST(TestRingBufferImplementation, test_buffer_clear) {
165165
EXPECT_EQ('d', d);
166166
}
167167

168+
namespace detail
169+
{
170+
171+
struct Tracked
172+
{
173+
inline static int destroyed = 0;
174+
int value;
175+
176+
Tracked()
177+
: value(0) {}
178+
explicit Tracked(int v)
179+
: value(v) {}
180+
~Tracked() {destroyed++;}
181+
};
182+
} // namespace detail
183+
184+
TEST(TestRingBufferImplementation, clear_is_non_destructive)
185+
{
186+
detail::Tracked::destroyed = 0;
187+
188+
rclcpp::experimental::buffers::RingBufferImplementation<detail::Tracked> rb(2);
189+
190+
rb.enqueue(detail::Tracked(1));
191+
rb.enqueue(detail::Tracked(2));
192+
193+
detail::Tracked::destroyed = 0;
194+
195+
rb.clear();
196+
197+
EXPECT_EQ(detail::Tracked::destroyed, 0);
198+
199+
// Outside buffer should be removed
200+
rb.enqueue(detail::Tracked(3));
201+
EXPECT_GE(detail::Tracked::destroyed, 1);
202+
}
203+
168204
TEST(TestRingBufferImplementation, handle_nullptr_deletion) {
169205
rclcpp::experimental::buffers::RingBufferImplementation<std::unique_ptr<int>> rb(3);
170206
rb.enqueue(std::make_unique<int>(42));

0 commit comments

Comments
 (0)