From 05ae2b1a97a134978cb0e665e160c87813276cf7 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Tue, 15 May 2018 10:11:41 -0700 Subject: [PATCH] Fix leak if buffer is never taken --- .../rmw_fastrtps_cpp/custom_client_info.hpp | 18 ++++++++++-------- rmw_fastrtps_cpp/src/rmw_response.cpp | 4 +--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/rmw_fastrtps_cpp/include/rmw_fastrtps_cpp/custom_client_info.hpp b/rmw_fastrtps_cpp/include/rmw_fastrtps_cpp/custom_client_info.hpp index 8f137ea6c..b41c04e1d 100644 --- a/rmw_fastrtps_cpp/include/rmw_fastrtps_cpp/custom_client_info.hpp +++ b/rmw_fastrtps_cpp/include/rmw_fastrtps_cpp/custom_client_info.hpp @@ -17,6 +17,8 @@ #include #include +#include +#include #include "fastcdr/FastBuffer.h" @@ -43,7 +45,7 @@ typedef struct CustomClientInfo typedef struct CustomClientResponse { eprosima::fastrtps::rtps::SampleIdentity sample_identity_; - eprosima::fastcdr::FastBuffer * buffer_; + std::unique_ptr buffer_; CustomClientResponse() : buffer_(nullptr) {} @@ -63,10 +65,10 @@ class ClientListener : public eprosima::fastrtps::SubscriberListener assert(sub); CustomClientResponse response; - response.buffer_ = new eprosima::fastcdr::FastBuffer(); + response.buffer_.reset(new eprosima::fastcdr::FastBuffer()); eprosima::fastrtps::SampleInfo_t sinfo; - if (sub->takeNextData(response.buffer_, &sinfo)) { + if (sub->takeNextData(response.buffer_.get(), &sinfo)) { if (eprosima::fastrtps::rtps::ALIVE == sinfo.sampleKind) { response.sample_identity_ = sinfo.related_sample_identity; @@ -75,7 +77,7 @@ class ClientListener : public eprosima::fastrtps::SubscriberListener if (conditionMutex_ != nullptr) { std::unique_lock clock(*conditionMutex_); - list.push_back(response); + list.emplace_back(std::move(response)); // the change to list_has_data_ needs to be mutually exclusive with // rmw_wait() which checks hasData() and decides if wait() needs to // be called @@ -83,7 +85,7 @@ class ClientListener : public eprosima::fastrtps::SubscriberListener clock.unlock(); conditionVariable_->notify_one(); } else { - list.push_back(response); + list.emplace_back(std::move(response)); list_has_data_.store(true); } } @@ -100,19 +102,19 @@ class ClientListener : public eprosima::fastrtps::SubscriberListener if (conditionMutex_ != nullptr) { std::unique_lock clock(*conditionMutex_); if (!list.empty()) { - response = list.front(); + response = std::move(list.front()); list.pop_front(); list_has_data_.store(!list.empty()); } } else { if (!list.empty()) { - response = list.front(); + response = std::move(list.front()); list.pop_front(); list_has_data_.store(!list.empty()); } } - return response; + return std::move(response); } void diff --git a/rmw_fastrtps_cpp/src/rmw_response.cpp b/rmw_fastrtps_cpp/src/rmw_response.cpp index 7c57dec9b..b15bc94fa 100644 --- a/rmw_fastrtps_cpp/src/rmw_response.cpp +++ b/rmw_fastrtps_cpp/src/rmw_response.cpp @@ -53,14 +53,12 @@ rmw_take_response( CustomClientResponse response = info->listener_->getResponse(); if (response.buffer_ != nullptr) { - _deserialize_ros_message(response.buffer_, ros_response, info->response_type_support_, + _deserialize_ros_message(response.buffer_.get(), ros_response, info->response_type_support_, info->typesupport_identifier_); request_header->sequence_number = ((int64_t)response.sample_identity_.sequence_number().high) << 32 | response.sample_identity_.sequence_number().low; - delete response.buffer_; - *taken = true; }