6.3.1

The CMake 'install' command should never be used to copy content from
the source tree to the build directory like this. The destination
directory is affected by the 'DESTDIR' variable on UNIX platforms, which
will get prefixed onto the destination directory and we'll end up
installing the test resources into the target install space.

This change will eliminate the excess test collateral present in the
rclcpp deb packages being installed to '/tmp/binarydeb'.

Signed-off-by: Scott K Logan <logans@cottsay.net>

rclcpp/CHANGELOG.rst

@@ -2,6 +2,19 @@
 Changelog for package rclcpp
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+6.3.1 (2021-02-08)
+------------------
+* Reference test resources directly from source tree (`#1543 <https://github.com/ros2/rclcpp/issues/1543>`_)
+* clear statistics after window reset (`#1531 <https://github.com/ros2/rclcpp/issues/1531>`_) (`#1535 <https://github.com/ros2/rclcpp/issues/1535>`_)
+* Fix a minor string error in the topic_statistics test. (`#1541 <https://github.com/ros2/rclcpp/issues/1541>`_)
+* Avoid `Resource deadlock avoided` if use intra_process_comms (`#1530 <https://github.com/ros2/rclcpp/issues/1530>`_)
+* Avoid an object copy in parameter_value.cpp. (`#1538 <https://github.com/ros2/rclcpp/issues/1538>`_)
+* Assert that the publisher_list size is 1. (`#1537 <https://github.com/ros2/rclcpp/issues/1537>`_)
+* Don't access objects after they have been std::move (`#1536 <https://github.com/ros2/rclcpp/issues/1536>`_)
+* Update for checking correct variable (`#1534 <https://github.com/ros2/rclcpp/issues/1534>`_)
+* Destroy msg extracted from LoanedMessage. (`#1305 <https://github.com/ros2/rclcpp/issues/1305>`_)
+* Contributors: Chen Lihui, Chris Lalancette, Ivan Santiago Paunovic, Miaofei Mei, Scott K Logan, William Woodall, hsgwa
+
 6.3.0 (2021-01-25)
 ------------------
 * Add instrumentation for linking a timer to a node (`#1500 <https://github.com/ros2/rclcpp/issues/1500>`_)

rclcpp/include/rclcpp/create_subscription.hpp

@@ -101,7 +101,7 @@ create_subscription(
     auto sub_call_back = [weak_subscription_topic_stats]() {
         auto subscription_topic_stats = weak_subscription_topic_stats.lock();
         if (subscription_topic_stats) {
-          subscription_topic_stats->publish_message();
+          subscription_topic_stats->publish_message_and_reset_measurements();
         }
       };
 

rclcpp/include/rclcpp/experimental/intra_process_manager.hpp

@@ -307,7 +307,7 @@ private:
   {
     SubscriptionInfo() = default;
 
-    rclcpp::experimental::SubscriptionIntraProcessBase::SharedPtr subscription;
+    rclcpp::experimental::SubscriptionIntraProcessBase::WeakPtr subscription;
     rmw_qos_profile_t qos;
     const char * topic_name;
     bool use_take_shared_method;
@@ -361,13 +361,16 @@ private:
       if (subscription_it == subscriptions_.end()) {
         throw std::runtime_error("subscription has unexpectedly gone out of scope");
       }
-      auto subscription_base = subscription_it->second.subscription;
+      auto subscription_base = subscription_it->second.subscription.lock();
+      if (subscription_base) {
+        auto subscription = std::static_pointer_cast<
+          rclcpp::experimental::SubscriptionIntraProcess<MessageT>
+          >(subscription_base);
 
-      auto subscription = std::static_pointer_cast<
-        rclcpp::experimental::SubscriptionIntraProcess<MessageT>
-        >(subscription_base);
-
-      subscription->provide_intra_process_message(message);
+        subscription->provide_intra_process_message(message);
+      } else {
+        subscriptions_.erase(id);
+      }
     }
   }
 
@@ -389,24 +392,27 @@ private:
       if (subscription_it == subscriptions_.end()) {
         throw std::runtime_error("subscription has unexpectedly gone out of scope");
       }
-      auto subscription_base = subscription_it->second.subscription;
+      auto subscription_base = subscription_it->second.subscription.lock();
+      if (subscription_base) {
+        auto subscription = std::static_pointer_cast<
+          rclcpp::experimental::SubscriptionIntraProcess<MessageT>
+          >(subscription_base);
 
-      auto subscription = std::static_pointer_cast<
-        rclcpp::experimental::SubscriptionIntraProcess<MessageT>
-        >(subscription_base);
+        if (std::next(it) == subscription_ids.end()) {
+          // If this is the last subscription, give up ownership
+          subscription->provide_intra_process_message(std::move(message));
+        } else {
+          // Copy the message since we have additional subscriptions to serve
+          MessageUniquePtr copy_message;
+          Deleter deleter = message.get_deleter();
+          auto ptr = MessageAllocTraits::allocate(*allocator.get(), 1);
+          MessageAllocTraits::construct(*allocator.get(), ptr, *message);
+          copy_message = MessageUniquePtr(ptr, deleter);
 
-      if (std::next(it) == subscription_ids.end()) {
-        // If this is the last subscription, give up ownership
-        subscription->provide_intra_process_message(std::move(message));
+          subscription->provide_intra_process_message(std::move(copy_message));
+        }
       } else {
-        // Copy the message since we have additional subscriptions to serve
-        MessageUniquePtr copy_message;
-        Deleter deleter = message.get_deleter();
-        auto ptr = MessageAllocTraits::allocate(*allocator.get(), 1);
-        MessageAllocTraits::construct(*allocator.get(), ptr, *message);
-        copy_message = MessageUniquePtr(ptr, deleter);
-
-        subscription->provide_intra_process_message(std::move(copy_message));
+        subscriptions_.erase(subscription_it);
       }
     }
   }

rclcpp/include/rclcpp/loaned_message.hpp

@@ -183,14 +183,30 @@ public:
   /**
    * A call to `release()` will unmanage the memory for the ROS message.
    * That means that the destructor of this class will not free the memory on scope exit.
+   * If the message is loaned from the middleware but not be published, the user needs to call
+   * `rcl_return_loaned_message_from_publisher` manually.
+   * If the memory is from the local allocator, the memory is freed when the unique pointer
+   * goes out instead.
    *
-   * \return Raw pointer to the message instance.
+   * \return std::unique_ptr to the message instance.
    */
-  MessageT * release()
+  std::unique_ptr<MessageT, std::function<void(MessageT *)>>
+  release()
   {
     auto msg = message_;
     message_ = nullptr;
-    return msg;
+
+    if (pub_.can_loan_messages()) {
+      return std::unique_ptr<MessageT, std::function<void(MessageT *)>>(msg, [](MessageT *) {});
+    }
+
+    return std::unique_ptr<MessageT, std::function<void(MessageT *)>>(
+      msg,
+      [allocator = message_allocator_](MessageT * msg_ptr) mutable {
+        // call destructor before deallocating
+        msg_ptr->~MessageT();
+        allocator.deallocate(msg_ptr, 1);
+      });
   }
 
 protected:

rclcpp/include/rclcpp/publisher.hpp

@@ -261,7 +261,7 @@ public:
     if (this->can_loan_messages()) {
       // we release the ownership from the rclpp::LoanedMessage instance
       // and let the middleware clean up the memory.
-      this->do_loaned_message_publish(loaned_msg.release());
+      this->do_loaned_message_publish(std::move(loaned_msg.release()));
     } else {
       // we don't release the ownership, let the middleware copy the ros message
       // and thus the destructor of rclcpp::LoanedMessage cleans up the memory.
@@ -310,9 +310,9 @@ protected:
   }
 
   void
-  do_loaned_message_publish(MessageT * msg)
+  do_loaned_message_publish(std::unique_ptr<MessageT, std::function<void(MessageT *)>> msg)
   {
-    auto status = rcl_publish_loaned_message(publisher_handle_.get(), msg, nullptr);
+    auto status = rcl_publish_loaned_message(publisher_handle_.get(), msg.get(), nullptr);
 
     if (RCL_RET_PUBLISHER_INVALID == status) {
       rcl_reset_error();  // next call will reset error message if not context

rclcpp/include/rclcpp/subscription.hpp

@@ -171,11 +171,7 @@ public:
 
       // First create a SubscriptionIntraProcess which will be given to the intra-process manager.
       auto context = node_base->get_context();
-      using SubscriptionIntraProcessT = rclcpp::experimental::SubscriptionIntraProcess<
-        CallbackMessageT,
-        AllocatorT,
-        typename MessageUniquePtr::deleter_type>;
-      auto subscription_intra_process = std::make_shared<SubscriptionIntraProcessT>(
+      subscription_intra_process_ = std::make_shared<SubscriptionIntraProcessT>(
         callback,
         options.get_allocator(),
         context,
@@ -185,12 +181,12 @@ public:
       TRACEPOINT(
         rclcpp_subscription_init,
         static_cast<const void *>(get_subscription_handle().get()),
-        static_cast<const void *>(subscription_intra_process.get()));
+        static_cast<const void *>(subscription_intra_process_.get()));
 
       // Add it to the intra process manager.
       using rclcpp::experimental::IntraProcessManager;
       auto ipm = context->get_sub_context<IntraProcessManager>();
-      uint64_t intra_process_subscription_id = ipm->add_subscription(subscription_intra_process);
+      uint64_t intra_process_subscription_id = ipm->add_subscription(subscription_intra_process_);
       this->setup_intra_process(intra_process_subscription_id, ipm);
     }
 
@@ -347,6 +343,11 @@ private:
     message_memory_strategy_;
   /// Component which computes and publishes topic statistics for this subscriber
   SubscriptionTopicStatisticsSharedPtr subscription_topic_statistics_{nullptr};
+  using SubscriptionIntraProcessT = rclcpp::experimental::SubscriptionIntraProcess<
+    CallbackMessageT,
+    AllocatorT,
+    typename MessageUniquePtr::deleter_type>;
+  std::shared_ptr<SubscriptionIntraProcessT> subscription_intra_process_;
 };
 
 }  // namespace rclcpp

rclcpp/include/rclcpp/topic_statistics/subscription_topic_statistics.hpp

@@ -127,7 +127,7 @@ public:
   /**
    * This method acquires a lock to prevent race conditions to collectors list.
    */
-  virtual void publish_message()
+  virtual void publish_message_and_reset_measurements()
   {
     std::vector<MetricsMessage> msgs;
     rclcpp::Time window_end{get_current_nanoseconds_since_epoch()};
@@ -136,6 +136,7 @@ public:
       std::lock_guard<std::mutex> lock(mutex_);
       for (auto & collector : subscriber_statistics_collectors_) {
         const auto collected_stats = collector->GetStatisticsResults();
+        collector->ClearCurrentMeasurements();
 
         auto message = libstatistics_collector::collector::GenerateStatisticMessage(
           node_name_,

rclcpp/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rclcpp</name>
-  <version>6.3.0</version>
+  <version>6.3.1</version>
   <description>The ROS client library in C++.</description>
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>
   <maintainer email="mabel@openrobotics.org">Mabel Zhang</maintainer>

rclcpp/src/rclcpp/intra_process_manager.cpp

@@ -157,7 +157,13 @@ IntraProcessManager::get_subscription_intra_process(uint64_t intra_process_subsc
   if (subscription_it == subscriptions_.end()) {
     return nullptr;
   } else {
-    return subscription_it->second.subscription;
+    auto subscription = subscription_it->second.subscription.lock();
+    if (subscription) {
+      return subscription;
+    } else {
+      subscriptions_.erase(subscription_it);
+      return nullptr;
+    }
   }
 }
 

rclcpp/src/rclcpp/parameter_value.cpp

@@ -67,7 +67,7 @@ array_to_string(
   type_array << "[";
   type_array.setf(format_flags, std::ios_base::basefield | std::ios::boolalpha);
   type_array << std::showbase;
-  for (const ValType value : array) {
+  for (const ValType & value : array) {
     if (!first_item) {
       type_array << ", ";
     } else {

rclcpp/test/CMakeLists.txt

@@ -4,6 +4,8 @@ find_package(test_msgs REQUIRED)
 
 include(cmake/rclcpp_add_build_failure_test.cmake)
 
+set(TEST_RESOURCES_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/resources")
+
 add_subdirectory(benchmark)
 add_subdirectory(rclcpp)
 
@@ -11,8 +13,3 @@ ament_add_gtest(test_rclcpp_gtest_macros utils/test_rclcpp_gtest_macros.cpp)
 if(TARGET test_rclcpp_gtest_macros)
   target_link_libraries(test_rclcpp_gtest_macros ${PROJECT_NAME})
 endif()
-
-# Install test resources
-install(
-  DIRECTORY resources
-  DESTINATION ${CMAKE_CURRENT_BINARY_DIR})

rclcpp/test/rclcpp/CMakeLists.txt

@@ -2,7 +2,7 @@ find_package(ament_cmake_gtest REQUIRED)
 
 find_package(rmw_implementation_cmake REQUIRED)
 
-add_definitions(-DTEST_RESOURCES_DIRECTORY="${CMAKE_CURRENT_BINARY_DIR}/../resources")
+add_definitions(-DTEST_RESOURCES_DIRECTORY="${TEST_RESOURCES_DIRECTORY}")
 
 rosidl_generate_interfaces(${PROJECT_NAME}_test_msgs
   ../msg/Header.msg

rclcpp/test/rclcpp/node_interfaces/test_node_parameters.cpp

@@ -125,7 +125,7 @@ TEST_F(TestNodeParameters, parameter_overrides)
   auto * node_parameters_interface =
     dynamic_cast<rclcpp::node_interfaces::NodeParameters *>(
     node2->get_node_parameters_interface().get());
-  ASSERT_NE(nullptr, node_parameters);
+  ASSERT_NE(nullptr, node_parameters_interface);
 
   const auto & parameter_overrides = node_parameters_interface->get_parameter_overrides();
   EXPECT_EQ(2u, parameter_overrides.size());

rclcpp/test/rclcpp/test_loaned_message.cpp

@@ -67,7 +67,7 @@ TEST_F(TestLoanedMessage, release) {
   auto node = std::make_shared<rclcpp::Node>("loaned_message_test_node");
   auto pub = node->create_publisher<MessageT>("loaned_message_test_topic", 1);
 
-  MessageT * msg = nullptr;
+  std::unique_ptr<MessageT, std::function<void(MessageT *)>> msg;
   {
     auto loaned_msg = pub->borrow_loaned_message();
     ASSERT_TRUE(loaned_msg.is_valid());
@@ -81,6 +81,15 @@ TEST_F(TestLoanedMessage, release) {
 
   ASSERT_EQ(42.0f, msg->float64_value);
 
+  // Generally, the memory released from `LoanedMessage::release()` will be freed
+  // in deleter of unique_ptr or is managed in the middleware after calling
+  // `Publisher::do_loaned_message_publish` inside Publisher::publish().
+  if (pub->can_loan_messages()) {
+    ASSERT_EQ(
+      RCL_RET_OK,
+      rcl_return_loaned_message_from_publisher(pub->get_publisher_handle().get(), msg.get()));
+  }
+
   SUCCEED();
 }
 

rclcpp/test/rclcpp/test_node.cpp

@@ -2415,7 +2415,7 @@ TEST_F(TestNode, get_publishers_subscriptions_info_by_topic) {
   auto publisher = node->create_publisher<test_msgs::msg::BasicTypes>(topic_name, qos);
   // List should have one item
   auto publisher_list = node->get_publishers_info_by_topic(fq_topic_name);
-  EXPECT_EQ(publisher_list.size(), (size_t)1);
+  ASSERT_EQ(publisher_list.size(), (size_t)1);
   // Subscription list should be empty
   EXPECT_TRUE(node->get_subscriptions_info_by_topic(fq_topic_name).empty());
   // Verify publisher list has the right data.

rclcpp/test/rclcpp/test_publisher.cpp

@@ -462,9 +462,9 @@ class TestPublisherProtectedMethods : public rclcpp::Publisher<MessageT, Allocat
 public:
   using rclcpp::Publisher<MessageT, AllocatorT>::Publisher;
 
-  void publish_loaned_message(MessageT * msg)
+  void publish_loaned_message(rclcpp::LoanedMessage<MessageT, AllocatorT> && loaned_msg)
   {
-    this->do_loaned_message_publish(msg);
+    this->do_loaned_message_publish(std::move(loaned_msg.release()));
   }
 
   void call_default_incompatible_qos_callback(rclcpp::QOSOfferedIncompatibleQoSInfo & event) const
@@ -479,13 +479,14 @@ TEST_F(TestPublisher, do_loaned_message_publish_error) {
   auto publisher =
     node->create_publisher<test_msgs::msg::Empty, std::allocator<void>, PublisherT>("topic", 10);
 
-  auto msg = std::make_shared<test_msgs::msg::Empty>();
+  auto msg = publisher->borrow_loaned_message();
+
   {
     // Using 'self' instead of 'lib:rclcpp' because `rcl_publish_loaned_message` is entirely
     // defined in a header
     auto mock = mocking_utils::patch_and_return(
       "self", rcl_publish_loaned_message, RCL_RET_PUBLISHER_INVALID);
-    EXPECT_THROW(publisher->publish_loaned_message(msg.get()), rclcpp::exceptions::RCLError);
+    EXPECT_THROW(publisher->publish_loaned_message(std::move(msg)), rclcpp::exceptions::RCLError);
   }
 }
 

rclcpp/test/rclcpp/test_serialized_message.cpp

@@ -212,8 +212,6 @@ TEST(TestSerializedMessage, assignment_operators) {
   serialized_msg_move = std::move(serialized_message_to_assign);
   EXPECT_EQ(13u, serialized_msg_move.capacity());
   EXPECT_EQ(content_size, serialized_msg_move.size());
-  EXPECT_EQ(0u, serialized_message_to_assign.capacity());
-  EXPECT_EQ(0u, serialized_message_to_assign.size());
 
   // Test move assignment with = operator, with a rcl_serialized_message_t
   rclcpp::SerializedMessage serialized_msg_move_rcl(2);
@@ -222,7 +220,6 @@ TEST(TestSerializedMessage, assignment_operators) {
   serialized_msg_move_rcl = std::move(rcl_serialized_msg);
   EXPECT_EQ(13u, serialized_msg_move_rcl.capacity());
   EXPECT_EQ(content_size, serialized_msg_move_rcl.size());
-  EXPECT_EQ(0u, rcl_serialized_msg.buffer_capacity);
 
   // Error because it was moved
   EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, rmw_serialized_message_fini(&rcl_serialized_msg));

rclcpp/test/rclcpp/topic_statistics/test_subscription_topic_statistics.cpp

@@ -45,6 +45,7 @@
 
 namespace
 {
+constexpr const std::chrono::seconds defaultStatisticsPublishPeriod{1};
 constexpr const char kTestPubNodeName[]{"test_pub_stats_node"};
 constexpr const char kTestSubNodeName[]{"test_sub_stats_node"};
 constexpr const char kTestSubStatsTopic[]{"/test_sub_stats_topic"};
@@ -53,10 +54,17 @@ constexpr const char kTestTopicStatisticsTopic[]{"/test_topic_statistics_topic"}
 constexpr const char kMessageAgeSourceLabel[]{"message_age"};
 constexpr const char kMessagePeriodSourceLabel[]{"message_period"};
 constexpr const uint64_t kNoSamples{0};
-constexpr const std::chrono::seconds kTestDuration{10};
-constexpr const uint64_t kNumExpectedMessages{8};
-constexpr const uint64_t kNumExpectedMessageAgeMessages{kNumExpectedMessages / 2};
-constexpr const uint64_t kNumExpectedMessagePeriodMessages{kNumExpectedMessages / 2};
+constexpr const std::chrono::seconds kTestTimeout{10};
+constexpr const uint64_t kNumExpectedWindows{4};
+constexpr const uint64_t kNumExpectedMessages{kNumExpectedWindows * 2};
+constexpr const uint64_t kNumExpectedMessageAgeMessages{kNumExpectedWindows};
+constexpr const uint64_t kNumExpectedMessagePeriodMessages{kNumExpectedWindows};
+constexpr const std::chrono::seconds kUnstableMessageAgeWindowDuration{
+  defaultStatisticsPublishPeriod * (kNumExpectedWindows / 2)};
+// kUnstableMessageAgeWindowDuration can take following value.
+// Min: defaultStatisticsPublishPeriod * 2
+// Max: defaultStatisticsPublishPeriod * (kNumExpectedWindows - 2)
+constexpr const std::chrono::seconds kUnstableMessageAgeOffset{std::chrono::seconds{1}};
 }  // namespace
 
 using rclcpp::msg::MessageWithHeader;
@@ -161,6 +169,49 @@ private:
   std::uniform_int_distribution<uint32_t> uniform_dist_;
 };
 
+/**
+ * TransitionMessageStamp publisher node : used to publish MessageWithHeader with `header` value set
+ * The message age results change during the test.
+ */
+
+class TransitionMessageStampPublisher : public rclcpp::Node
+{
+public:
+  TransitionMessageStampPublisher(
+    const std::string & name, const std::string & topic,
+    const std::chrono::seconds transition_duration, const std::chrono::seconds message_age_offset,
+    const std::chrono::milliseconds & publish_period = std::chrono::milliseconds{100})
+  : Node(name), transition_duration_(transition_duration), message_age_offset_(message_age_offset)
+  {
+    publisher_ = create_publisher<MessageWithHeader>(topic, 10);
+    publish_timer_ = this->create_wall_timer(publish_period, [this]() {this->publish_message();});
+    start_time_ = this->now();
+  }
+
+private:
+  void publish_message()
+  {
+    auto msg = MessageWithHeader{};
+    auto now = this->now();
+    auto elapsed_time = now - start_time_;
+    if (elapsed_time < transition_duration_) {
+      // Apply only to the topic statistics in the first half
+      // Subtract offset so message_age is always >= offset.
+      msg.header.stamp = now - message_age_offset_;
+    } else {
+      msg.header.stamp = now;
+    }
+    publisher_->publish(msg);
+  }
+
+  std::chrono::seconds transition_duration_;
+  std::chrono::seconds message_age_offset_;
+  rclcpp::Time start_time_;
+
+  rclcpp::Publisher<MessageWithHeader>::SharedPtr publisher_;
+  rclcpp::TimerBase::SharedPtr publish_timer_;
+};
+
 /**
  * Empty subscriber node: used to create subscriber topic statistics requirements
  */
@@ -202,6 +253,7 @@ public:
     // manually enable topic statistics via options
     auto options = rclcpp::SubscriptionOptions();
     options.topic_stats_options.state = rclcpp::TopicStatisticsState::Enable;
+    options.topic_stats_options.publish_period = defaultStatisticsPublishPeriod;
 
     auto callback = [](MessageWithHeader::UniquePtr msg) {
         (void) msg;
@@ -278,7 +330,7 @@ void check_if_statistic_message_is_populated(const MetricsMessage & message_to_c
         EXPECT_LT(0, stats_point.data) << "unexpected avg " << stats_point.data;
         break;
       case StatisticDataType::STATISTICS_DATA_TYPE_MINIMUM:
-        EXPECT_LT(0, stats_point.data) << "unexpected mi n" << stats_point.data;
+        EXPECT_LT(0, stats_point.data) << "unexpected min " << stats_point.data;
         break;
       case StatisticDataType::STATISTICS_DATA_TYPE_MAXIMUM:
         EXPECT_LT(0, stats_point.data) << "unexpected max " << stats_point.data;
@@ -382,7 +434,7 @@ TEST_F(TestSubscriptionTopicStatisticsFixture, test_receive_stats_for_message_no
   // Spin and get future
   ex.spin_until_future_complete(
     statistics_listener->GetFuture(),
-    kTestDuration);
+    kTestTimeout);
 
   // Compare message counts, sample count should be the same as published and received count
   EXPECT_EQ(kNumExpectedMessages, statistics_listener->GetNumberOfMessagesReceived());
@@ -447,7 +499,7 @@ TEST_F(TestSubscriptionTopicStatisticsFixture, test_receive_stats_for_message_wi
   // Spin and get future
   ex.spin_until_future_complete(
     statistics_listener->GetFuture(),
-    kTestDuration);
+    kTestTimeout);
 
   // Compare message counts, sample count should be the same as published and received count
   EXPECT_EQ(kNumExpectedMessages, statistics_listener->GetNumberOfMessagesReceived());
@@ -476,3 +528,61 @@ TEST_F(TestSubscriptionTopicStatisticsFixture, test_receive_stats_for_message_wi
     check_if_statistic_message_is_populated(msg);
   }
 }
+
+TEST_F(TestSubscriptionTopicStatisticsFixture, test_receive_stats_include_window_reset)
+{
+  // Create a MessageWithHeader publisher
+  auto msg_with_header_publisher = std::make_shared<TransitionMessageStampPublisher>(
+    kTestPubNodeName, kTestSubStatsTopic, kUnstableMessageAgeWindowDuration,
+    kUnstableMessageAgeOffset);
+
+  // msg_with_header_subscriber has a topic statistics instance as part of its
+  // subscription this will listen to and generate statistics
+  auto msg_with_header_subscriber =
+    std::make_shared<MessageWithHeaderSubscriber>(kTestSubNodeName, kTestSubStatsTopic);
+  // Create a listener for topic statistics messages
+  auto statistics_listener = std::make_shared<rclcpp::topic_statistics::MetricsMessageSubscriber>(
+    "test_receive_stats_include_window_reset", "/statistics", kNumExpectedMessages);
+
+  rclcpp::executors::SingleThreadedExecutor ex;
+  ex.add_node(msg_with_header_publisher);
+  ex.add_node(statistics_listener);
+  ex.add_node(msg_with_header_subscriber);
+
+  // Spin and get future
+  ex.spin_until_future_complete(statistics_listener->GetFuture(), kTestTimeout);
+
+  const auto received_messages = statistics_listener->GetReceivedMessages();
+  EXPECT_EQ(kNumExpectedMessages, received_messages.size());
+
+  auto message_age_offset =
+    std::chrono::duration<double, std::milli>(kUnstableMessageAgeOffset).count();
+
+  // Check that the first statistic contains the offset inside of its window
+  auto head_message = received_messages[0];
+  for (const auto & stats_point : head_message.statistics) {
+    const auto type = stats_point.data_type;
+    switch (type) {
+      case StatisticDataType::STATISTICS_DATA_TYPE_MINIMUM:
+      case StatisticDataType::STATISTICS_DATA_TYPE_MAXIMUM:
+        EXPECT_GE(stats_point.data, message_age_offset);
+        break;
+      default:
+        break;
+    }
+  }
+
+  // Check that the last statistic does not contain the offset outside of its window
+  auto tail_message = received_messages[received_messages.size() - 1];
+  for (const auto & stats_point : tail_message.statistics) {
+    const auto type = stats_point.data_type;
+    switch (type) {
+      case StatisticDataType::STATISTICS_DATA_TYPE_MINIMUM:
+      case StatisticDataType::STATISTICS_DATA_TYPE_MAXIMUM:
+        EXPECT_LT(stats_point.data, message_age_offset);
+        break;
+      default:
+        break;
+    }
+  }
+}

rclcpp_action/CHANGELOG.rst

@@ -3,6 +3,12 @@ Changelog for package rclcpp_action
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
+6.3.1 (2021-02-08)
+------------------
+* Finalize rcl_handle to prevent leak (`#1528 <https://github.com/ros2/rclcpp/issues/1528>`_) (`#1529 <https://github.com/ros2/rclcpp/issues/1529>`_)
+* Fix `#1526 <https://github.com/ros2/rclcpp/issues/1526>`_. (`#1527 <https://github.com/ros2/rclcpp/issues/1527>`_)
+* Contributors: y-okumura-isp
+
 6.3.0 (2021-01-25)
 ------------------
 * Fix action server deadlock (`#1285 <https://github.com/ros2/rclcpp/issues/1285>`_) (`#1313 <https://github.com/ros2/rclcpp/issues/1313>`_)

rclcpp_action/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rclcpp_action</name>
-  <version>6.3.0</version>
+  <version>6.3.1</version>
   <description>Adds action APIs for C++.</description>
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>
   <maintainer email="mabel@openrobotics.org">Mabel Zhang</maintainer>

rclcpp_action/src/server.cpp

@@ -48,10 +48,11 @@ public:
   // Lock for action_server_
   std::recursive_mutex action_server_reentrant_mutex_;
 
-  std::shared_ptr<rcl_action_server_t> action_server_;
-
   rclcpp::Clock::SharedPtr clock_;
 
+  // Do not declare this before clock_ as this depends on clock_(see #1526)
+  std::shared_ptr<rcl_action_server_t> action_server_;
+
   size_t num_subscriptions_ = 0;
   size_t num_timers_ = 0;
   size_t num_clients_ = 0;

rclcpp_action/test/test_server_goal_handle.cpp

@@ -56,7 +56,16 @@ public:
   void SetUp()
   {
     std::shared_ptr<rcl_action_goal_handle_t> rcl_handle =
-      std::make_shared<rcl_action_goal_handle_t>();
+      std::shared_ptr<rcl_action_goal_handle_t>(
+      new rcl_action_goal_handle_t,
+      [](rcl_action_goal_handle_t * p) {
+        if (nullptr == p) {
+          return;
+        }
+        rcl_ret_t ret = rcl_action_goal_handle_fini(p);
+        EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
+        delete p;
+      });
     *rcl_handle.get() = rcl_action_get_zero_initialized_goal_handle();
     rcutils_allocator_t allocator = rcutils_get_default_allocator();
     rcl_action_goal_info_t goal_info = rcl_action_get_zero_initialized_goal_info();

rclcpp_components/CHANGELOG.rst

@@ -2,6 +2,9 @@
 Changelog for package rclcpp_components
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+6.3.1 (2021-02-08)
+------------------
+
 6.3.0 (2021-01-25)
 ------------------
 

rclcpp_components/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rclcpp_components</name>
-  <version>6.3.0</version>
+  <version>6.3.1</version>
   <description>Package containing tools for dynamically loadable components</description>
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>
   <maintainer email="mabel@openrobotics.org">Mabel Zhang</maintainer>

rclcpp_lifecycle/CHANGELOG.rst

@@ -3,6 +3,9 @@ Changelog for package rclcpp_lifecycle
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
+6.3.1 (2021-02-08)
+------------------
+
 6.3.0 (2021-01-25)
 ------------------
 

rclcpp_lifecycle/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>rclcpp_lifecycle</name>
-  <version>6.3.0</version>
+  <version>6.3.1</version>
   <description>Package containing a prototype for lifecycle implementation</description>
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>
   <maintainer email="mabel@openrobotics.org">Mabel Zhang</maintainer>