Fix leak if buffer is never taken

ros2 · May 15, 2018 · 05ae2b1 · 05ae2b1
1 parent 3700d75
commit 05ae2b1
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 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
@@ -17,6 +17,8 @@
 
 #include <atomic>
 #include <list>
+#include <memory>
+#include <utility>
 
 #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<eprosima::fastcdr::FastBuffer> 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,15 +77,15 @@ class ClientListener : public eprosima::fastrtps::SubscriberListener
 
           if (conditionMutex_ != nullptr) {
             std::unique_lock<std::mutex> 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
             list_has_data_.store(true);
             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<std::mutex> 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
@@ -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;
   }