21.0.8

Signed-off-by: Yadunund <yadunund@gmail.com>

rclcpp/CHANGELOG.rst

@@ -2,24 +2,56 @@
 Changelog for package rclcpp
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-21.2.0 (2023-06-07)
+21.0.8 (2024-11-08)
 -------------------
-* remove nolint since ament_cpplint updated for the c++17 header (`#2198 <https://github.com/ros2/rclcpp/issues/2198>`_)
-* Feature/available capacity of ipm (`#2173 <https://github.com/ros2/rclcpp/issues/2173>`_)
-* add mutex to protect events_executor current entity collection (`#2187 <https://github.com/ros2/rclcpp/issues/2187>`_)
-* Declare rclcpp callbacks before the rcl entities (`#2024 <https://github.com/ros2/rclcpp/issues/2024>`_)
-* Contributors: Alberto Soragna, Chen Lihui, DensoADAS, mauropasse
+* associated clocks should be protected by mutex. (`#2258 <https://github.com/ros2/rclcpp/issues/2258>`_)
+* Skip client_qos test (`#2658 <https://github.com/ros2/rclcpp/issues/2658>`_)
+* Use the same context for the specified node in rclcpp::spin functions. (`#2618 <https://github.com/ros2/rclcpp/issues/2618>`_)
+* Contributors: Cristóbal Arroyo, Tomoya Fujita
 
-21.1.1 (2023-05-11)
+21.0.7 (2024-07-10)
 -------------------
-* Fix race condition in events-executor (`#2177 <https://github.com/ros2/rclcpp/issues/2177>`_)
-* Add missing stdexcept include (`#2186 <https://github.com/ros2/rclcpp/issues/2186>`_)
-* Fix a format-security warning when building with clang (`#2171 <https://github.com/ros2/rclcpp/issues/2171>`_)
-* Fix delivered message kind (`#2175 <https://github.com/ros2/rclcpp/issues/2175>`_)
-* Contributors: Alberto Soragna, Chris Lalancette, methylDragon, Øystein Sture
+* Add test creating two content filter topics with the same topic name (`#2550 <https://github.com/ros2/rclcpp/issues/2550>`_)
+* Revise the description of service configure_introspection() (`#2514 <https://github.com/ros2/rclcpp/issues/2514>`_)
+* Contributors: Alejandro Hernández Cordero, Barry Xu
 
-21.1.0 (2023-04-27)
+21.0.6 (2024-04-19)
 -------------------
+* address ambiguous auto variable. (`#2486 <https://github.com/ros2/rclcpp/issues/2486>`_)
+* Contributors: Tomoya Fujita
+
+21.0.5 (2024-02-07)
+-------------------
+* Fix data race in EventHandlerBase (`#2387 <https://github.com/ros2/rclcpp/issues/2387>`_)
+* Contributors: mauropasse
+
+21.0.4 (2023-11-17)
+-------------------
+* Disable the loaned messages inside the executor. (`#2365 <https://github.com/ros2/rclcpp/issues/2365>`_)
+* Add missing 'enable_rosout' comments (`#2346 <https://github.com/ros2/rclcpp/issues/2346>`_)
+* Address rate related flaky tests. (`#2341 <https://github.com/ros2/rclcpp/issues/2341>`_)
+* Add missing stdexcept include (`#2333 <https://github.com/ros2/rclcpp/issues/2333>`_)
+* Update SignalHandler get_global_signal_handler to avoid complex types in static memory (`#2322 <https://github.com/ros2/rclcpp/issues/2322>`_)
+* Fix C++20 allocator construct deprecation (`#2318 <https://github.com/ros2/rclcpp/issues/2318>`_)
+* Topic correct typeadapter deduction (`#2298 <https://github.com/ros2/rclcpp/issues/2298>`_)
+* Contributors: AiVerisimilitude, Chen Lihui, Chris Lalancette, Jiaqi Li, Øystein Sture, Tomoya Fujita, William Woodall
+
+21.0.3 (2023-09-08)
+-------------------
+* Do not crash Executor when send_response fails due to client failure. (`#2279 <https://github.com/ros2/rclcpp/issues/2279>`_)
+* Add new node interface TypeDescriptionsInterface to provide GetTypeDescription service (`#2236 <https://github.com/ros2/rclcpp/issues/2236>`_)
+* Contributors: Emerson Knapp, Tomoya Fujita, Zang MingJie
+
+21.0.2 (2023-07-14)
+-------------------
+* Fix warnings related to comparison of integer expressions of different signedness (`#2222 <https://github.com/ros2/rclcpp/issues/2222>`_)
+* Fix race condition in events-executor (`#2191 <https://github.com/ros2/rclcpp/issues/2191>`_)
+* Contributors: Alberto Soragna, Tomoya Fujita
+
+21.0.1 (2023-05-11)
+-------------------
+* Fix delivered message kind (`#2175 <https://github.com/ros2/rclcpp/issues/2175>`_) (`#2178 <https://github.com/ros2/rclcpp/issues/2178>`_)
+* Contributors: mergify[bot]
 
 21.0.0 (2023-04-18)
 -------------------

rclcpp/CMakeLists.txt

@@ -64,6 +64,7 @@ set(${PROJECT_NAME}_SRCS
   src/rclcpp/executors/executor_notify_waitable.cpp
   src/rclcpp/executors/multi_threaded_executor.cpp
   src/rclcpp/executors/single_threaded_executor.cpp
+  src/rclcpp/executors/static_executor_entities_collector.cpp
   src/rclcpp/executors/static_single_threaded_executor.cpp
   src/rclcpp/expand_topic_or_service_name.cpp
   src/rclcpp/experimental/executors/events_executor/events_executor.cpp
@@ -91,6 +92,7 @@ set(${PROJECT_NAME}_SRCS
   src/rclcpp/node_interfaces/node_time_source.cpp
   src/rclcpp/node_interfaces/node_timers.cpp
   src/rclcpp/node_interfaces/node_topics.cpp
+  src/rclcpp/node_interfaces/node_type_descriptions.cpp
   src/rclcpp/node_interfaces/node_waitables.cpp
   src/rclcpp/node_options.cpp
   src/rclcpp/parameter.cpp

rclcpp/include/rclcpp/any_service_callback.hpp

@@ -156,7 +156,7 @@ public:
     const std::shared_ptr<rmw_request_id_t> & request_header,
     std::shared_ptr<typename ServiceT::Request> request)
   {
-    TRACETOOLS_TRACEPOINT(callback_start, static_cast<const void *>(this), false);
+    TRACEPOINT(callback_start, static_cast<const void *>(this), false);
     if (std::holds_alternative<std::monostate>(callback_)) {
       // TODO(ivanpauno): Remove the set method, and force the users of this class
       // to pass a callback at construnciton.
@@ -182,7 +182,7 @@ public:
       const auto & cb = std::get<SharedPtrWithRequestHeaderCallback>(callback_);
       cb(request_header, std::move(request), response);
     }
-    TRACETOOLS_TRACEPOINT(callback_end, static_cast<const void *>(this));
+    TRACEPOINT(callback_end, static_cast<const void *>(this));
     return response;
   }
 
@@ -191,9 +191,9 @@ public:
 #ifndef TRACETOOLS_DISABLED
     std::visit(
       [this](auto && arg) {
-        if (TRACETOOLS_TRACEPOINT_ENABLED(rclcpp_callback_register)) {
+        if (TRACEPOINT_ENABLED(rclcpp_callback_register)) {
           char * symbol = tracetools::get_symbol(arg);
-          TRACETOOLS_DO_TRACEPOINT(
+          DO_TRACEPOINT(
             rclcpp_callback_register,
             static_cast<const void *>(this),
             symbol);

rclcpp/include/rclcpp/any_subscription_callback.hpp

@@ -20,7 +20,7 @@
 #include <stdexcept>
 #include <type_traits>
 #include <utility>
-#include <variant>
+#include <variant>  // NOLINT[build/include_order]
 
 #include "rosidl_runtime_cpp/traits.hpp"
 #include "tracetools/tracetools.h"
@@ -492,7 +492,7 @@ public:
     std::shared_ptr<ROSMessageType> message,
     const rclcpp::MessageInfo & message_info)
   {
-    TRACETOOLS_TRACEPOINT(callback_start, static_cast<const void *>(this), false);
+    TRACEPOINT(callback_start, static_cast<const void *>(this), false);
     // Check if the variant is "unset", throw if it is.
     if (callback_variant_.index() == 0) {
       if (std::get<0>(callback_variant_) == nullptr) {
@@ -583,7 +583,7 @@ public:
           static_assert(always_false_v<T>, "unhandled callback type");
         }
       }, callback_variant_);
-    TRACETOOLS_TRACEPOINT(callback_end, static_cast<const void *>(this));
+    TRACEPOINT(callback_end, static_cast<const void *>(this));
   }
 
   // Dispatch when input is a serialized message and the output could be anything.
@@ -592,7 +592,7 @@ public:
     std::shared_ptr<rclcpp::SerializedMessage> serialized_message,
     const rclcpp::MessageInfo & message_info)
   {
-    TRACETOOLS_TRACEPOINT(callback_start, static_cast<const void *>(this), false);
+    TRACEPOINT(callback_start, static_cast<const void *>(this), false);
     // Check if the variant is "unset", throw if it is.
     if (callback_variant_.index() == 0) {
       if (std::get<0>(callback_variant_) == nullptr) {
@@ -663,7 +663,7 @@ public:
           static_assert(always_false_v<T>, "unhandled callback type");
         }
       }, callback_variant_);
-    TRACETOOLS_TRACEPOINT(callback_end, static_cast<const void *>(this));
+    TRACEPOINT(callback_end, static_cast<const void *>(this));
   }
 
   void
@@ -671,7 +671,7 @@ public:
     std::shared_ptr<const SubscribedType> message,
     const rclcpp::MessageInfo & message_info)
   {
-    TRACETOOLS_TRACEPOINT(callback_start, static_cast<const void *>(this), true);
+    TRACEPOINT(callback_start, static_cast<const void *>(this), true);
     // Check if the variant is "unset", throw if it is.
     if (callback_variant_.index() == 0) {
       if (std::get<0>(callback_variant_) == nullptr) {
@@ -793,7 +793,7 @@ public:
           static_assert(always_false_v<T>, "unhandled callback type");
         }
       }, callback_variant_);
-    TRACETOOLS_TRACEPOINT(callback_end, static_cast<const void *>(this));
+    TRACEPOINT(callback_end, static_cast<const void *>(this));
   }
 
   void
@@ -801,7 +801,7 @@ public:
     std::unique_ptr<SubscribedType, SubscribedTypeDeleter> message,
     const rclcpp::MessageInfo & message_info)
   {
-    TRACETOOLS_TRACEPOINT(callback_start, static_cast<const void *>(this), true);
+    TRACEPOINT(callback_start, static_cast<const void *>(this), true);
     // Check if the variant is "unset", throw if it is.
     if (callback_variant_.index() == 0) {
       if (std::get<0>(callback_variant_) == nullptr) {
@@ -927,7 +927,7 @@ public:
           static_assert(always_false_v<T>, "unhandled callback type");
         }
       }, callback_variant_);
-    TRACETOOLS_TRACEPOINT(callback_end, static_cast<const void *>(this));
+    TRACEPOINT(callback_end, static_cast<const void *>(this));
   }
 
   constexpr
@@ -965,9 +965,9 @@ public:
 #ifndef TRACETOOLS_DISABLED
     std::visit(
       [this](auto && callback) {
-        if (TRACETOOLS_TRACEPOINT_ENABLED(rclcpp_callback_register)) {
+        if (TRACEPOINT_ENABLED(rclcpp_callback_register)) {
           char * symbol = tracetools::get_symbol(callback);
-          TRACETOOLS_DO_TRACEPOINT(
+          DO_TRACEPOINT(
             rclcpp_callback_register,
             static_cast<const void *>(this),
             symbol);

rclcpp/include/rclcpp/callback_group.hpp

@@ -187,35 +187,14 @@ public:
   RCLCPP_PUBLIC
   size_t size() const;
 
-  /// Return a reference to the 'can be taken' atomic boolean.
-  /**
-   * The resulting bool will be true in the case that no executor is currently
-   * using an executable entity from this group.
-   * The resulting bool will be false in the case that an executor is currently
-   * using an executable entity from this group, and the group policy doesn't
-   * allow a second take (eg mutual exclusion)
-   * \return a reference to the flag
-   */
   RCLCPP_PUBLIC
   std::atomic_bool &
   can_be_taken_from();
 
-  /// Get the group type.
-  /**
-   * \return the group type
-   */
   RCLCPP_PUBLIC
   const CallbackGroupType &
   type() const;
 
-  /// Collect all of the entity pointers contained in this callback group.
-  /**
-   * \param[in] sub_func Function to execute for each subscription
-   * \param[in] service_func Function to execute for each service
-   * \param[in] client_func Function to execute for each client
-   * \param[in] timer_func Function to execute for each timer
-   * \param[in] waitable_fuinc Function to execute for each waitable
-   */
   RCLCPP_PUBLIC
   void collect_all_ptrs(
     std::function<void(const rclcpp::SubscriptionBase::SharedPtr &)> sub_func,

rclcpp/include/rclcpp/client.hpp

@@ -20,13 +20,13 @@
 #include <future>
 #include <memory>
 #include <mutex>
-#include <optional>
+#include <optional>  // NOLINT, cpplint doesn't think this is a cpp std header
 #include <sstream>
 #include <string>
 #include <tuple>
 #include <unordered_map>
 #include <utility>
-#include <variant>
+#include <variant>  // NOLINT
 #include <vector>
 
 #include "rcl/client.h"
@@ -363,16 +363,12 @@ protected:
   std::shared_ptr<rclcpp::Context> context_;
   rclcpp::Logger node_logger_;
 
-  std::recursive_mutex callback_mutex_;
-  // It is important to declare on_new_response_callback_ before
-  // client_handle_, so on destruction the client is
-  // destroyed first. Otherwise, the rmw client callback
-  // would point briefly to a destroyed function.
-  std::function<void(size_t)> on_new_response_callback_{nullptr};
-  // Declare client_handle_ after callback
   std::shared_ptr<rcl_client_t> client_handle_;
 
   std::atomic<bool> in_use_by_wait_set_{false};
+
+  std::recursive_mutex callback_mutex_;
+  std::function<void(size_t)> on_new_response_callback_{nullptr};
 };
 
 template<typename ServiceT>
@@ -849,7 +845,7 @@ protected:
         "Received invalid sequence number. Ignoring...");
       return std::nullopt;
     }
-    auto value = std::move(it->second.second);
+    std::optional<CallbackInfoVariant> value = std::move(it->second.second);
     this->pending_requests_.erase(request_number);
     return value;
   }

rclcpp/include/rclcpp/event_handler.hpp

@@ -260,6 +260,16 @@ public:
     }
   }
 
+  ~EventHandler()
+  {
+    // Since the rmw event listener holds a reference to the
+    // "on ready" callback, we need to clear it on destruction of this class.
+    // This clearing is not needed for other rclcpp entities like pub/subs, since
+    // they do own the underlying rmw entities, which are destroyed
+    // on their rclcpp destructors, thus no risk of dangling pointers.
+    clear_on_ready_callback();
+  }
+
   /// Take data so that the callback cannot be scheduled again
   std::shared_ptr<void>
   take_data() override

rclcpp/include/rclcpp/executor.hpp

@@ -19,7 +19,6 @@
 #include <cassert>
 #include <chrono>
 #include <cstdlib>
-#include <deque>
 #include <iostream>
 #include <list>
 #include <map>
@@ -30,24 +29,26 @@
 
 #include "rcl/guard_condition.h"
 #include "rcl/wait.h"
-#include "rclcpp/executors/executor_notify_waitable.hpp"
 #include "rcpputils/scope_exit.hpp"
 
 #include "rclcpp/context.hpp"
 #include "rclcpp/contexts/default_context.hpp"
 #include "rclcpp/guard_condition.hpp"
 #include "rclcpp/executor_options.hpp"
-#include "rclcpp/executors/executor_entities_collection.hpp"
-#include "rclcpp/executors/executor_entities_collector.hpp"
 #include "rclcpp/future_return_code.hpp"
+#include "rclcpp/memory_strategies.hpp"
+#include "rclcpp/memory_strategy.hpp"
 #include "rclcpp/node_interfaces/node_base_interface.hpp"
 #include "rclcpp/utilities.hpp"
 #include "rclcpp/visibility_control.hpp"
-#include "rclcpp/wait_set.hpp"
 
 namespace rclcpp
 {
 
+typedef std::map<rclcpp::CallbackGroup::WeakPtr,
+    rclcpp::node_interfaces::NodeBaseInterface::WeakPtr,
+    std::owner_less<rclcpp::CallbackGroup::WeakPtr>> WeakCallbackGroupsToNodesMap;
+
 // Forward declaration is used in convenience method signature.
 class Node;
 class ExecutorImplementation;
@@ -402,6 +403,17 @@ public:
   void
   cancel();
 
+  /// Support dynamic switching of the memory strategy.
+  /**
+   * Switching the memory strategy while the executor is spinning in another threading could have
+   * unintended consequences.
+   * \param[in] memory_strategy Shared pointer to the memory strategy to set.
+   * \throws std::runtime_error if memory_strategy is null
+   */
+  RCLCPP_PUBLIC
+  void
+  set_memory_strategy(memory_strategy::MemoryStrategy::SharedPtr memory_strategy);
+
   /// Returns true if the executor is currently spinning.
   /**
    * This function can be called asynchronously from any thread.
@@ -486,11 +498,6 @@ protected:
   static void
   execute_client(rclcpp::ClientBase::SharedPtr client);
 
-  /// Gather all of the waitable entities from associated nodes and callback groups.
-  RCLCPP_PUBLIC
-  void
-  collect_entities();
-
   /// Block until more work becomes avilable or timeout is reached.
   /**
    * Builds a set of waitable entities, which are passed to the middleware.
@@ -502,6 +509,62 @@ protected:
   void
   wait_for_work(std::chrono::nanoseconds timeout = std::chrono::nanoseconds(-1));
 
+  /// Find node associated with a callback group
+  /**
+   * \param[in] weak_groups_to_nodes map of callback groups to nodes
+   * \param[in] group callback group to find assocatiated node
+   * \return Pointer to associated node if found, else nullptr
+   */
+  RCLCPP_PUBLIC
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr
+  get_node_by_group(
+    const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
+    rclcpp::CallbackGroup::SharedPtr group);
+
+  /// Return true if the node has been added to this executor.
+  /**
+   * \param[in] node_ptr a shared pointer that points to a node base interface
+   * \param[in] weak_groups_to_nodes map to nodes to lookup
+   * \return true if the node is associated with the executor, otherwise false
+   */
+  RCLCPP_PUBLIC
+  bool
+  has_node(
+    const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+    const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes) const;
+
+  /// Find the callback group associated with a timer
+  /**
+   * \param[in] timer Timer to find associated callback group
+   * \return Pointer to callback group node if found, else nullptr
+   */
+  RCLCPP_PUBLIC
+  rclcpp::CallbackGroup::SharedPtr
+  get_group_by_timer(rclcpp::TimerBase::SharedPtr timer);
+
+  /// Add a callback group to an executor
+  /**
+   * \see rclcpp::Executor::add_callback_group
+   */
+  RCLCPP_PUBLIC
+  virtual void
+  add_callback_group_to_map(
+    rclcpp::CallbackGroup::SharedPtr group_ptr,
+    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+    WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
+    bool notify = true) RCPPUTILS_TSA_REQUIRES(mutex_);
+
+  /// Remove a callback group from the executor.
+  /**
+   * \see rclcpp::Executor::remove_callback_group
+   */
+  RCLCPP_PUBLIC
+  virtual void
+  remove_callback_group_from_map(
+    rclcpp::CallbackGroup::SharedPtr group_ptr,
+    WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
+    bool notify = true) RCPPUTILS_TSA_REQUIRES(mutex_);
+
   /// Check for executable in ready state and populate union structure.
   /**
    * \param[out] any_executable populated union structure of ready executable
@@ -512,6 +575,33 @@ protected:
   bool
   get_next_ready_executable(AnyExecutable & any_executable);
 
+  /// Check for executable in ready state and populate union structure.
+  /**
+   * This is the implementation of get_next_ready_executable that takes into
+   * account the current state of callback groups' association with nodes and
+   * executors.
+   *
+   * This checks in a particular order for available work:
+   *   * Timers
+   *   * Subscriptions
+   *   * Services
+   *   * Clients
+   *   * Waitable
+   *
+   * If the next executable is not associated with this executor/node pair,
+   * then this method will return false.
+   *
+   * \param[out] any_executable populated union structure of ready executable
+   * \param[in] weak_groups_to_nodes mapping of callback groups to nodes
+   * \return true if an executable was ready and any_executable was populated,
+   *   otherwise false
+   */
+  RCLCPP_PUBLIC
+  bool
+  get_next_ready_executable_from_map(
+    AnyExecutable & any_executable,
+    const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes);
+
   /// Wait for executable in ready state and populate union structure.
   /**
    * If an executable is ready, it will return immediately, otherwise
@@ -529,6 +619,21 @@ protected:
     AnyExecutable & any_executable,
     std::chrono::nanoseconds timeout = std::chrono::nanoseconds(-1));
 
+  /// Add all callback groups that can be automatically added from associated nodes.
+  /**
+   * The executor, before collecting entities, verifies if any callback group from
+   * nodes associated with the executor, which is not already associated to an executor,
+   * can be automatically added to this executor.
+   * This takes care of any callback group that has been added to a node but not explicitly added
+   * to the executor.
+   * It is important to note that in order for the callback groups to be automatically added to an
+   * executor through this function, the node of the callback groups needs to have been added
+   * through the `add_node` method.
+   */
+  RCLCPP_PUBLIC
+  virtual void
+  add_callback_groups_from_nodes_associated_to_executor() RCPPUTILS_TSA_REQUIRES(mutex_);
+
   /// Spinning state, used to prevent multi threaded calls to spin and to cancel blocking spins.
   std::atomic_bool spinning;
 
@@ -538,8 +643,16 @@ protected:
   /// Guard condition for signaling the rmw layer to wake up for system shutdown.
   std::shared_ptr<rclcpp::GuardCondition> shutdown_guard_condition_;
 
+  /// Wait set for managing entities that the rmw layer waits on.
+  rcl_wait_set_t wait_set_ = rcl_get_zero_initialized_wait_set();
+
+  // Mutex to protect the subsequent memory_strategy_.
   mutable std::mutex mutex_;
 
+  /// The memory strategy: an interface for handling user-defined memory allocation strategies.
+  memory_strategy::MemoryStrategy::SharedPtr
+  memory_strategy_ RCPPUTILS_TSA_PT_GUARDED_BY(mutex_);
+
   /// The context associated with this executor.
   std::shared_ptr<rclcpp::Context> context_;
 
@@ -549,33 +662,39 @@ protected:
   virtual void
   spin_once_impl(std::chrono::nanoseconds timeout);
 
-  /// Waitable containing guard conditions controlling the executor flow.
-  /**
-   * This waitable contains the interrupt and shutdown guard condition, as well
-   * as the guard condition associated with each node and callback group.
-   * By default, if any change is detected in the monitored entities, the notify
-   * waitable will awake the executor and rebuild  the collections.
-   */
-  std::shared_ptr<rclcpp::executors::ExecutorNotifyWaitable> notify_waitable_;
+  typedef std::map<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr,
+      const rclcpp::GuardCondition *,
+      std::owner_less<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr>>
+    WeakNodesToGuardConditionsMap;
 
-  std::atomic_bool entities_need_rebuild_;
+  typedef std::map<rclcpp::CallbackGroup::WeakPtr,
+      const rclcpp::GuardCondition *,
+      std::owner_less<rclcpp::CallbackGroup::WeakPtr>>
+    WeakCallbackGroupsToGuardConditionsMap;
 
-  /// Collector used to associate executable entities from nodes and guard conditions
-  rclcpp::executors::ExecutorEntitiesCollector collector_;
+  /// maps nodes to guard conditions
+  WeakNodesToGuardConditionsMap
+  weak_nodes_to_guard_conditions_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
 
-  /// Waitset to be waited on.
-  rclcpp::WaitSet wait_set_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
+  /// maps callback groups to guard conditions
+  WeakCallbackGroupsToGuardConditionsMap
+  weak_groups_to_guard_conditions_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
 
-  /// Hold the current state of the collection being waited on by the waitset
-  rclcpp::executors::ExecutorEntitiesCollection current_collection_ RCPPUTILS_TSA_GUARDED_BY(
-    mutex_);
+  /// maps callback groups associated to nodes
+  WeakCallbackGroupsToNodesMap
+  weak_groups_associated_with_executor_to_nodes_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
 
-  /// Hold the current state of the notify waitable being waited on by the waitset
-  std::shared_ptr<rclcpp::executors::ExecutorNotifyWaitable> current_notify_waitable_
-  RCPPUTILS_TSA_GUARDED_BY(mutex_);
+  /// maps callback groups to nodes associated with executor
+  WeakCallbackGroupsToNodesMap
+  weak_groups_to_nodes_associated_with_executor_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
 
-  /// Hold the list of executables currently available to be executed.
-  std::deque<rclcpp::AnyExecutable> ready_executables_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
+  /// maps all callback groups to nodes
+  WeakCallbackGroupsToNodesMap
+  weak_groups_to_nodes_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
+
+  /// nodes that are associated with the executor
+  std::list<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr>
+  weak_nodes_ RCPPUTILS_TSA_GUARDED_BY(mutex_);
 
   /// shutdown callback handle registered to Context
   rclcpp::OnShutdownCallbackHandle shutdown_callback_handle_;

rclcpp/include/rclcpp/executors.hpp

@@ -108,7 +108,9 @@ spin_until_future_complete(
   const FutureT & future,
   std::chrono::duration<TimeRepT, TimeT> timeout = std::chrono::duration<TimeRepT, TimeT>(-1))
 {
-  rclcpp::executors::SingleThreadedExecutor executor;
+  rclcpp::ExecutorOptions options;
+  options.context = node_ptr->get_context();
+  rclcpp::executors::SingleThreadedExecutor executor(options);
   return executors::spin_node_until_future_complete<FutureT>(executor, node_ptr, future, timeout);
 }
 

rclcpp/include/rclcpp/executors/executor_entities_collection.hpp

@@ -178,12 +178,6 @@ struct ExecutorEntitiesCollection
 
   /// Clear the entities collection
   void clear();
-
-  /// Remove entities that have expired weak ownership
-  /**
-   * \return The total number of removed entities
-   */
-  size_t remove_expired_entities();
 };
 
 /// Build an entities collection from callback groups

rclcpp/include/rclcpp/executors/static_executor_entities_collector.hpp

@@ -0,0 +1,357 @@
+// Copyright 2020 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef RCLCPP__EXECUTORS__STATIC_EXECUTOR_ENTITIES_COLLECTOR_HPP_
+#define RCLCPP__EXECUTORS__STATIC_EXECUTOR_ENTITIES_COLLECTOR_HPP_
+
+#include <chrono>
+#include <list>
+#include <map>
+#include <memory>
+#include <vector>
+
+#include "rcl/guard_condition.h"
+#include "rcl/wait.h"
+
+#include "rclcpp/experimental/executable_list.hpp"
+#include "rclcpp/macros.hpp"
+#include "rclcpp/memory_strategy.hpp"
+#include "rclcpp/visibility_control.hpp"
+#include "rclcpp/waitable.hpp"
+
+namespace rclcpp
+{
+namespace executors
+{
+typedef std::map<rclcpp::CallbackGroup::WeakPtr,
+    rclcpp::node_interfaces::NodeBaseInterface::WeakPtr,
+    std::owner_less<rclcpp::CallbackGroup::WeakPtr>> WeakCallbackGroupsToNodesMap;
+
+class StaticExecutorEntitiesCollector final
+  : public rclcpp::Waitable,
+  public std::enable_shared_from_this<StaticExecutorEntitiesCollector>
+{
+public:
+  RCLCPP_SMART_PTR_DEFINITIONS(StaticExecutorEntitiesCollector)
+
+  // Constructor
+  RCLCPP_PUBLIC
+  StaticExecutorEntitiesCollector() = default;
+
+  // Destructor
+  RCLCPP_PUBLIC
+  ~StaticExecutorEntitiesCollector();
+
+  /// Initialize StaticExecutorEntitiesCollector
+  /**
+   * \param p_wait_set A reference to the wait set to be used in the executor
+   * \param memory_strategy Shared pointer to the memory strategy to set.
+   * \throws std::runtime_error if memory strategy is null
+   */
+  RCLCPP_PUBLIC
+  void
+  init(
+    rcl_wait_set_t * p_wait_set,
+    rclcpp::memory_strategy::MemoryStrategy::SharedPtr memory_strategy);
+
+  /// Finalize StaticExecutorEntitiesCollector to clear resources
+  RCLCPP_PUBLIC
+  bool
+  is_init() {return initialized_;}
+
+  RCLCPP_PUBLIC
+  void
+  fini();
+
+  /// Execute the waitable.
+  RCLCPP_PUBLIC
+  void
+  execute(std::shared_ptr<void> & data) override;
+
+  /// Take the data so that it can be consumed with `execute`.
+  /**
+   * For `StaticExecutorEntitiesCollector`, this always return `nullptr`.
+   * \sa rclcpp::Waitable::take_data()
+   */
+  RCLCPP_PUBLIC
+  std::shared_ptr<void>
+  take_data() override;
+
+  /// Function to add_handles_to_wait_set and wait for work and
+  /**
+   * block until the wait set is ready or until the timeout has been exceeded.
+   * \throws std::runtime_error if wait set couldn't be cleared or filled.
+   * \throws any rcl errors from rcl_wait, \see rclcpp::exceptions::throw_from_rcl_error()
+   */
+  RCLCPP_PUBLIC
+  void
+  refresh_wait_set(std::chrono::nanoseconds timeout = std::chrono::nanoseconds(-1));
+
+  /**
+   * \throws std::runtime_error if it couldn't add guard condition to wait set
+   */
+  RCLCPP_PUBLIC
+  void
+  add_to_wait_set(rcl_wait_set_t * wait_set) override;
+
+  RCLCPP_PUBLIC
+  size_t
+  get_number_of_ready_guard_conditions() override;
+
+  /// Add a callback group to an executor.
+  /**
+   * \see rclcpp::Executor::add_callback_group
+   */
+  RCLCPP_PUBLIC
+  bool
+  add_callback_group(
+    rclcpp::CallbackGroup::SharedPtr group_ptr,
+    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr);
+
+  /// Add a callback group to an executor.
+  /**
+   * \see rclcpp::Executor::add_callback_group
+   * \return boolean whether the node from the callback group is new
+   */
+  RCLCPP_PUBLIC
+  bool
+  add_callback_group(
+    rclcpp::CallbackGroup::SharedPtr group_ptr,
+    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+    WeakCallbackGroupsToNodesMap & weak_groups_to_nodes);
+
+  /// Remove a callback group from the executor.
+  /**
+   * \see rclcpp::Executor::remove_callback_group
+   */
+  RCLCPP_PUBLIC
+  bool
+  remove_callback_group(
+    rclcpp::CallbackGroup::SharedPtr group_ptr);
+
+  /// Remove a callback group from the executor.
+  /**
+   * \see rclcpp::Executor::remove_callback_group_from_map
+   */
+  RCLCPP_PUBLIC
+  bool
+  remove_callback_group_from_map(
+    rclcpp::CallbackGroup::SharedPtr group_ptr,
+    WeakCallbackGroupsToNodesMap & weak_groups_to_nodes);
+
+  /**
+   * \see rclcpp::Executor::add_node()
+   * \throw std::runtime_error if node was already added
+   */
+  RCLCPP_PUBLIC
+  bool
+  add_node(
+    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr);
+
+  /**
+   * \see rclcpp::Executor::remove_node()
+   * \throw std::runtime_error if no guard condition is associated with node.
+   */
+  RCLCPP_PUBLIC
+  bool
+  remove_node(
+    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr);
+
+  RCLCPP_PUBLIC
+  std::vector<rclcpp::CallbackGroup::WeakPtr>
+  get_all_callback_groups();
+
+  /// Get callback groups that belong to executor.
+  /**
+   * \see rclcpp::Executor::get_manually_added_callback_groups()
+   */
+  RCLCPP_PUBLIC
+  std::vector<rclcpp::CallbackGroup::WeakPtr>
+  get_manually_added_callback_groups();
+
+  /// Get callback groups that belong to executor.
+  /**
+   * \see rclcpp::Executor::get_automatically_added_callback_groups_from_nodes()
+   */
+  RCLCPP_PUBLIC
+  std::vector<rclcpp::CallbackGroup::WeakPtr>
+  get_automatically_added_callback_groups_from_nodes();
+
+  /// Complete all available queued work without blocking.
+  /**
+   * This function checks if after the guard condition was triggered
+   * (or a spurious wakeup happened) we are really ready to execute
+   * i.e. re-collect entities
+   */
+  RCLCPP_PUBLIC
+  bool
+  is_ready(rcl_wait_set_t * wait_set) override;
+
+  /// Return number of timers
+  /**
+   * \return number of timers
+   */
+  RCLCPP_PUBLIC
+  size_t
+  get_number_of_timers() {return exec_list_.number_of_timers;}
+
+  /// Return number of subscriptions
+  /**
+   * \return number of subscriptions
+   */
+  RCLCPP_PUBLIC
+  size_t
+  get_number_of_subscriptions() {return exec_list_.number_of_subscriptions;}
+
+  /// Return number of services
+  /**
+   * \return number of services
+   */
+  RCLCPP_PUBLIC
+  size_t
+  get_number_of_services() {return exec_list_.number_of_services;}
+
+  /// Return number of clients
+  /**
+   * \return number of clients
+   */
+  RCLCPP_PUBLIC
+  size_t
+  get_number_of_clients() {return exec_list_.number_of_clients;}
+
+  /// Return number of waitables
+  /**
+   * \return number of waitables
+   */
+  RCLCPP_PUBLIC
+  size_t
+  get_number_of_waitables() {return exec_list_.number_of_waitables;}
+
+  /** Return a SubscritionBase Sharedptr by index.
+   * \param[in] i The index of the SubscritionBase
+   * \return a SubscritionBase shared pointer
+   * \throws std::out_of_range if the argument is higher than the size of the structrue.
+   */
+  RCLCPP_PUBLIC
+  rclcpp::SubscriptionBase::SharedPtr
+  get_subscription(size_t i) {return exec_list_.subscription[i];}
+
+  /** Return a TimerBase Sharedptr by index.
+   * \param[in] i The index of the TimerBase
+   * \return a TimerBase shared pointer
+   * \throws std::out_of_range if the argument is higher than the size.
+   */
+  RCLCPP_PUBLIC
+  rclcpp::TimerBase::SharedPtr
+  get_timer(size_t i) {return exec_list_.timer[i];}
+
+  /** Return a ServiceBase Sharedptr by index.
+   * \param[in] i The index of the ServiceBase
+   * \return a ServiceBase shared pointer
+   * \throws std::out_of_range if the argument is higher than the size.
+   */
+  RCLCPP_PUBLIC
+  rclcpp::ServiceBase::SharedPtr
+  get_service(size_t i) {return exec_list_.service[i];}
+
+  /** Return a ClientBase Sharedptr by index
+   * \param[in] i The index of the ClientBase
+   * \return a ClientBase shared pointer
+   * \throws std::out_of_range if the argument is higher than the size.
+   */
+  RCLCPP_PUBLIC
+  rclcpp::ClientBase::SharedPtr
+  get_client(size_t i) {return exec_list_.client[i];}
+
+  /** Return a Waitable Sharedptr by index
+   * \param[in] i The index of the Waitable
+   * \return a Waitable shared pointer
+   * \throws std::out_of_range if the argument is higher than the size.
+   */
+  RCLCPP_PUBLIC
+  rclcpp::Waitable::SharedPtr
+  get_waitable(size_t i) {return exec_list_.waitable[i];}
+
+private:
+  /// Function to reallocate space for entities in the wait set.
+  /**
+   * \throws std::runtime_error if wait set couldn't be cleared or resized.
+   */
+  void
+  prepare_wait_set();
+
+  void
+  fill_executable_list();
+
+  void
+  fill_memory_strategy();
+
+  /// Return true if the node belongs to the collector
+  /**
+   * \param[in] node_ptr a node base interface shared pointer
+   * \param[in] weak_groups_to_nodes map to nodes to lookup
+   * \return boolean whether a node belongs the collector
+   */
+  bool
+  has_node(
+    const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+    const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes) const;
+
+  /// Add all callback groups that can be automatically added by any executor
+  /// and is not already associated with an executor from nodes
+  /// that are associated with executor
+  /**
+   * \see rclcpp::Executor::add_callback_groups_from_nodes_associated_to_executor()
+   */
+  void
+  add_callback_groups_from_nodes_associated_to_executor();
+
+  void
+  fill_executable_list_from_map(const WeakCallbackGroupsToNodesMap & weak_groups_to_nodes);
+
+  /// Memory strategy: an interface for handling user-defined memory allocation strategies.
+  rclcpp::memory_strategy::MemoryStrategy::SharedPtr memory_strategy_;
+
+  // maps callback groups to nodes.
+  WeakCallbackGroupsToNodesMap weak_groups_associated_with_executor_to_nodes_;
+  // maps callback groups to nodes.
+  WeakCallbackGroupsToNodesMap weak_groups_to_nodes_associated_with_executor_;
+
+  typedef std::map<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr,
+      const rclcpp::GuardCondition *,
+      std::owner_less<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr>>
+    WeakNodesToGuardConditionsMap;
+  WeakNodesToGuardConditionsMap weak_nodes_to_guard_conditions_;
+
+  /// List of weak nodes registered in the static executor
+  std::list<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr> weak_nodes_;
+
+  // Mutex to protect vector of new nodes.
+  std::mutex new_nodes_mutex_;
+  std::vector<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr> new_nodes_;
+
+  /// Wait set for managing entities that the rmw layer waits on.
+  rcl_wait_set_t * p_wait_set_ = nullptr;
+
+  /// Executable list: timers, subscribers, clients, services and waitables
+  rclcpp::experimental::ExecutableList exec_list_;
+
+  /// Bool to check if the entities collector has been initialized
+  bool initialized_ = false;
+};
+
+}  // namespace executors
+}  // namespace rclcpp
+
+#endif  // RCLCPP__EXECUTORS__STATIC_EXECUTOR_ENTITIES_COLLECTOR_HPP_

rclcpp/include/rclcpp/executors/static_single_threaded_executor.hpp

@@ -15,13 +15,24 @@
 #ifndef RCLCPP__EXECUTORS__STATIC_SINGLE_THREADED_EXECUTOR_HPP_
 #define RCLCPP__EXECUTORS__STATIC_SINGLE_THREADED_EXECUTOR_HPP_
 
-#include <atomic>
 #include <chrono>
+#include <cassert>
+#include <cstdlib>
 #include <memory>
+#include <vector>
+#include <string>
+
+#include "rmw/rmw.h"
 
 #include "rclcpp/executor.hpp"
-#include "rclcpp/executors/executor_entities_collection.hpp"
-#include "rclcpp/executors/single_threaded_executor.hpp"
+#include "rclcpp/executors/static_executor_entities_collector.hpp"
+#include "rclcpp/experimental/executable_list.hpp"
+#include "rclcpp/macros.hpp"
+#include "rclcpp/memory_strategies.hpp"
+#include "rclcpp/node.hpp"
+#include "rclcpp/rate.hpp"
+#include "rclcpp/utilities.hpp"
+#include "rclcpp/visibility_control.hpp"
 
 namespace rclcpp
 {
@@ -54,7 +65,7 @@ public:
   explicit StaticSingleThreadedExecutor(
     const rclcpp::ExecutorOptions & options = rclcpp::ExecutorOptions());
 
-  /// Default destructor.
+  /// Default destrcutor.
   RCLCPP_PUBLIC
   virtual ~StaticSingleThreadedExecutor();
 
@@ -105,20 +116,92 @@ public:
   void
   spin_all(std::chrono::nanoseconds max_duration) override;
 
+  /// Add a callback group to an executor.
+  /**
+   * \sa rclcpp::Executor::add_callback_group
+   */
+  RCLCPP_PUBLIC
+  void
+  add_callback_group(
+    rclcpp::CallbackGroup::SharedPtr group_ptr,
+    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+    bool notify = true) override;
+
+  /// Remove callback group from the executor
+  /**
+   * \sa rclcpp::Executor::remove_callback_group
+   */
+  RCLCPP_PUBLIC
+  void
+  remove_callback_group(
+    rclcpp::CallbackGroup::SharedPtr group_ptr,
+    bool notify = true) override;
+
+  /// Add a node to the executor.
+  /**
+   * \sa rclcpp::Executor::add_node
+   */
+  RCLCPP_PUBLIC
+  void
+  add_node(
+    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+    bool notify = true) override;
+
+  /// Convenience function which takes Node and forwards NodeBaseInterface.
+  /**
+   * \sa rclcpp::StaticSingleThreadedExecutor::add_node
+   */
+  RCLCPP_PUBLIC
+  void
+  add_node(std::shared_ptr<rclcpp::Node> node_ptr, bool notify = true) override;
+
+  /// Remove a node from the executor.
+  /**
+   * \sa rclcpp::Executor::remove_node
+   */
+  RCLCPP_PUBLIC
+  void
+  remove_node(
+    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+    bool notify = true) override;
+
+  /// Convenience function which takes Node and forwards NodeBaseInterface.
+  /**
+   * \sa rclcpp::Executor::remove_node
+   */
+  RCLCPP_PUBLIC
+  void
+  remove_node(std::shared_ptr<rclcpp::Node> node_ptr, bool notify = true) override;
+
+  RCLCPP_PUBLIC
+  std::vector<rclcpp::CallbackGroup::WeakPtr>
+  get_all_callback_groups() override;
+
+  /// Get callback groups that belong to executor.
+  /**
+   * \sa rclcpp::Executor::get_manually_added_callback_groups()
+   */
+  RCLCPP_PUBLIC
+  std::vector<rclcpp::CallbackGroup::WeakPtr>
+  get_manually_added_callback_groups() override;
+
+  /// Get callback groups that belong to executor.
+  /**
+   * \sa rclcpp::Executor::get_automatically_added_callback_groups_from_nodes()
+   */
+  RCLCPP_PUBLIC
+  std::vector<rclcpp::CallbackGroup::WeakPtr>
+  get_automatically_added_callback_groups_from_nodes() override;
+
 protected:
   /**
    * @brief Executes ready executables from wait set.
-   * @param collection entities to evaluate for ready executables.
-   * @param wait_result result to check for ready executables.
    * @param spin_once if true executes only the first ready executable.
    * @return true if any executable was ready.
    */
   RCLCPP_PUBLIC
   bool
-  execute_ready_executables(
-    const rclcpp::executors::ExecutorEntitiesCollection & collection,
-    rclcpp::WaitResult<rclcpp::WaitSet> & wait_result,
-    bool spin_once);
+  execute_ready_executables(bool spin_once = false);
 
   RCLCPP_PUBLIC
   void
@@ -130,6 +213,8 @@ protected:
 
 private:
   RCLCPP_DISABLE_COPY(StaticSingleThreadedExecutor)
+
+  StaticExecutorEntitiesCollector::SharedPtr entities_collector_;
 };
 
 }  // namespace executors

rclcpp/include/rclcpp/experimental/buffers/buffer_implementation_base.hpp

@@ -33,7 +33,6 @@ public:
 
   virtual void clear() = 0;
   virtual bool has_data() const = 0;
-  virtual size_t available_capacity() const = 0;
 };
 
 }  // namespace buffers

rclcpp/include/rclcpp/experimental/buffers/intra_process_buffer.hpp

@@ -44,7 +44,6 @@ public:
 
   virtual bool has_data() const = 0;
   virtual bool use_take_shared_method() const = 0;
-  virtual size_t available_capacity() const = 0;
 };
 
 template<
@@ -96,7 +95,7 @@ public:
 
     buffer_ = std::move(buffer_impl);
 
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_buffer_to_ipb,
       static_cast<const void *>(buffer_.get()),
       static_cast<const void *>(this));
@@ -144,11 +143,6 @@ public:
     return std::is_same<BufferT, MessageSharedPtr>::value;
   }
 
-  size_t available_capacity() const override
-  {
-    return buffer_->available_capacity();
-  }
-
 private:
   std::unique_ptr<BufferImplementationBase<BufferT>> buffer_;
 

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

@@ -52,10 +52,7 @@ public:
     if (capacity == 0) {
       throw std::invalid_argument("capacity must be a positive, non-zero value");
     }
-    TRACETOOLS_TRACEPOINT(
-      rclcpp_construct_ring_buffer,
-      static_cast<const void *>(this),
-      capacity_);
+    TRACEPOINT(rclcpp_construct_ring_buffer, static_cast<const void *>(this), capacity_);
   }
 
   virtual ~RingBufferImplementation() {}
@@ -72,7 +69,7 @@ public:
 
     write_index_ = next_(write_index_);
     ring_buffer_[write_index_] = std::move(request);
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_ring_buffer_enqueue,
       static_cast<const void *>(this),
       write_index_,
@@ -101,7 +98,7 @@ public:
     }
 
     auto request = std::move(ring_buffer_[read_index_]);
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_ring_buffer_dequeue,
       static_cast<const void *>(this),
       read_index_,
@@ -151,21 +148,9 @@ public:
     return is_full_();
   }
 
-  /// Get the remaining capacity to store messages
-  /**
-   * This member function is thread-safe.
-   *
-   * \return the number of free capacity for new messages
-   */
-  size_t available_capacity() const
-  {
-    std::lock_guard<std::mutex> lock(mutex_);
-    return available_capacity_();
-  }
-
   void clear()
   {
-    TRACETOOLS_TRACEPOINT(rclcpp_ring_buffer_clear, static_cast<const void *>(this));
+    TRACEPOINT(rclcpp_ring_buffer_clear, static_cast<const void *>(this));
   }
 
 private:
@@ -204,17 +189,6 @@ private:
     return size_ == capacity_;
   }
 
-  /// Get the remaining capacity to store messages
-  /**
-   * This member function is not thread-safe.
-   *
-   * \return the number of free capacity for new messages
-   */
-  inline size_t available_capacity_() const
-  {
-    return capacity_ - size_;
-  }
-
   size_t capacity_;
 
   std::vector<BufferT> ring_buffer_;

rclcpp/include/rclcpp/experimental/executors/events_executor/events_executor.hpp

@@ -274,11 +274,8 @@ private:
   rclcpp::experimental::executors::EventsQueue::UniquePtr events_queue_;
 
   std::shared_ptr<rclcpp::executors::ExecutorEntitiesCollector> entities_collector_;
-  std::shared_ptr<rclcpp::executors::ExecutorNotifyWaitable> notify_waitable_;
-
-  /// Mutex to protect the current_entities_collection_
-  std::recursive_mutex collection_mutex_;
   std::shared_ptr<rclcpp::executors::ExecutorEntitiesCollection> current_entities_collection_;
+  std::shared_ptr<rclcpp::executors::ExecutorNotifyWaitable> notify_waitable_;
 
   /// Flag used to reduce the number of unnecessary waitable events
   std::atomic<bool> notify_waitable_event_pushed_ {false};

rclcpp/include/rclcpp/experimental/intra_process_manager.hpp

@@ -306,11 +306,6 @@ public:
   rclcpp::experimental::SubscriptionIntraProcessBase::SharedPtr
   get_subscription_intra_process(uint64_t intra_process_subscription_id);
 
-  /// Return the lowest available capacity for all subscription buffers for a publisher id.
-  RCLCPP_PUBLIC
-  size_t
-  lowest_available_capacity(const uint64_t intra_process_publisher_id) const;
-
 private:
   struct SplittedSubscriptions
   {
@@ -486,13 +481,13 @@ private:
                 "subscription use different allocator types, which is not supported");
       }
 
-      if constexpr (rclcpp::TypeAdapter<MessageT>::is_specialized::value) {
+      if constexpr (rclcpp::TypeAdapter<MessageT, ROSMessageType>::is_specialized::value) {
         ROSMessageTypeAllocator ros_message_alloc(allocator);
-        auto ptr = ros_message_alloc.allocate(1);
-        ros_message_alloc.construct(ptr);
+        auto ptr = ROSMessageTypeAllocatorTraits::allocate(ros_message_alloc, 1);
+        ROSMessageTypeAllocatorTraits::construct(ros_message_alloc, ptr);
         ROSMessageTypeDeleter deleter;
         allocator::set_allocator_for_deleter(&deleter, &allocator);
-        rclcpp::TypeAdapter<MessageT>::convert_to_ros_message(*message, *ptr);
+        rclcpp::TypeAdapter<MessageT, ROSMessageType>::convert_to_ros_message(*message, *ptr);
         auto ros_msg = std::unique_ptr<ROSMessageType, ROSMessageTypeDeleter>(ptr, deleter);
         ros_message_subscription->provide_intra_process_message(std::move(ros_msg));
       } else {

rclcpp/include/rclcpp/experimental/subscription_intra_process.hpp

@@ -87,7 +87,7 @@ public:
       buffer_type),
     any_callback_(callback)
   {
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_subscription_callback_added,
       static_cast<const void *>(this),
       static_cast<const void *>(&any_callback_));

rclcpp/include/rclcpp/experimental/subscription_intra_process_base.hpp

@@ -62,11 +62,6 @@ public:
   void
   add_to_wait_set(rcl_wait_set_t * wait_set) override;
 
-  RCLCPP_PUBLIC
-  virtual
-  size_t
-  available_capacity() const = 0;
-
   bool
   is_ready(rcl_wait_set_t * wait_set) override = 0;
 

rclcpp/include/rclcpp/experimental/subscription_intra_process_buffer.hpp

@@ -93,7 +93,7 @@ public:
       buffer_type,
       qos_profile,
       std::make_shared<Alloc>(subscribed_type_allocator_));
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_ipb_to_subscription,
       static_cast<const void *>(buffer_.get()),
       static_cast<const void *>(this));
@@ -169,11 +169,6 @@ public:
     return buffer_->use_take_shared_method();
   }
 
-  size_t available_capacity() const override
-  {
-    return buffer_->available_capacity();
-  }
-
 protected:
   void
   trigger_guard_condition() override

rclcpp/include/rclcpp/node.hpp

@@ -56,6 +56,7 @@
 #include "rclcpp/node_interfaces/node_time_source_interface.hpp"
 #include "rclcpp/node_interfaces/node_timers_interface.hpp"
 #include "rclcpp/node_interfaces/node_topics_interface.hpp"
+#include "rclcpp/node_interfaces/node_type_descriptions_interface.hpp"
 #include "rclcpp/node_interfaces/node_waitables_interface.hpp"
 #include "rclcpp/node_options.hpp"
 #include "rclcpp/parameter.hpp"
@@ -1454,6 +1455,11 @@ public:
   rclcpp::node_interfaces::NodeTimeSourceInterface::SharedPtr
   get_node_time_source_interface();
 
+  /// Return the Node's internal NodeTypeDescriptionsInterface implementation.
+  RCLCPP_PUBLIC
+  rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
+  get_node_type_descriptions_interface();
+
   /// Return the sub-namespace, if this is a sub-node, otherwise an empty string.
   /**
    * The returned sub-namespace is either the accumulated sub-namespaces which
@@ -1591,6 +1597,11 @@ private:
   const rclcpp::NodeOptions node_options_;
   const std::string sub_namespace_;
   const std::string effective_namespace_;
+
+  /// Static map(s) containing extra member variables for Node without changing its ABI.
+  // See node.cpp for more details.
+  class BackportMembers;
+  static BackportMembers backport_members_;
 };
 
 }  // namespace rclcpp

rclcpp/include/rclcpp/node_interfaces/node_interfaces.hpp

@@ -30,6 +30,7 @@
   rclcpp::node_interfaces::NodeTimeSourceInterface, \
   rclcpp::node_interfaces::NodeTimersInterface, \
   rclcpp::node_interfaces::NodeTopicsInterface, \
+  rclcpp::node_interfaces::NodeTypeDescriptionsInterface, \
   rclcpp::node_interfaces::NodeWaitablesInterface
 
 
@@ -118,6 +119,7 @@ public:
    *   - rclcpp::node_interfaces::NodeTimeSourceInterface
    *   - rclcpp::node_interfaces::NodeTimersInterface
    *   - rclcpp::node_interfaces::NodeTopicsInterface
+   *   - rclcpp::node_interfaces::NodeTypeDescriptionsInterface
    *   - rclcpp::node_interfaces::NodeWaitablesInterface
    *
    * Or you use custom interfaces as long as you make a template specialization

rclcpp/include/rclcpp/node_interfaces/node_type_descriptions.hpp

@@ -0,0 +1,63 @@
+// Copyright 2023 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_HPP_
+#define RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_HPP_
+
+#include <memory>
+
+#include "rclcpp/macros.hpp"
+#include "rclcpp/node_interfaces/node_base_interface.hpp"
+#include "rclcpp/node_interfaces/node_logging_interface.hpp"
+#include "rclcpp/node_interfaces/node_parameters_interface.hpp"
+#include "rclcpp/node_interfaces/node_services_interface.hpp"
+#include "rclcpp/node_interfaces/node_topics_interface.hpp"
+#include "rclcpp/node_interfaces/node_type_descriptions_interface.hpp"
+#include "rclcpp/visibility_control.hpp"
+
+namespace rclcpp
+{
+namespace node_interfaces
+{
+
+/// Implementation of the NodeTypeDescriptions part of the Node API.
+class NodeTypeDescriptions : public NodeTypeDescriptionsInterface
+{
+public:
+  RCLCPP_SMART_PTR_ALIASES_ONLY(NodeTypeDescriptions)
+
+  RCLCPP_PUBLIC
+  explicit NodeTypeDescriptions(
+    NodeBaseInterface::SharedPtr node_base,
+    NodeLoggingInterface::SharedPtr node_logging,
+    NodeParametersInterface::SharedPtr node_parameters,
+    NodeServicesInterface::SharedPtr node_services);
+
+  RCLCPP_PUBLIC
+  virtual
+  ~NodeTypeDescriptions();
+
+private:
+  RCLCPP_DISABLE_COPY(NodeTypeDescriptions)
+
+  // Pimpl hides helper types and functions used for wrapping a C service, which would be
+  // awkward to expose in this header.
+  class NodeTypeDescriptionsImpl;
+  std::unique_ptr<NodeTypeDescriptionsImpl> impl_;
+};
+
+}  // namespace node_interfaces
+}  // namespace rclcpp
+
+#endif  // RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_HPP_

rclcpp/include/rclcpp/node_interfaces/node_type_descriptions_interface.hpp

@@ -0,0 +1,44 @@
+// Copyright 2023 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_INTERFACE_HPP_
+#define RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_INTERFACE_HPP_
+
+#include "rclcpp/macros.hpp"
+#include "rclcpp/node_interfaces/detail/node_interfaces_helpers.hpp"
+#include "rclcpp/visibility_control.hpp"
+
+namespace rclcpp
+{
+namespace node_interfaces
+{
+
+/// Pure virtual interface class for the NodeTypeDescriptions part of the Node API.
+class NodeTypeDescriptionsInterface
+{
+public:
+  RCLCPP_SMART_PTR_ALIASES_ONLY(NodeTypeDescriptionsInterface)
+
+  RCLCPP_PUBLIC
+  virtual
+  ~NodeTypeDescriptionsInterface() = default;
+};
+
+}  // namespace node_interfaces
+}  // namespace rclcpp
+
+RCLCPP_NODE_INTERFACE_HELPERS_SUPPORT(
+  rclcpp::node_interfaces::NodeTypeDescriptionsInterface, type_descriptions)
+
+#endif  // RCLCPP__NODE_INTERFACES__NODE_TYPE_DESCRIPTIONS_INTERFACE_HPP_

rclcpp/include/rclcpp/node_options.hpp

@@ -43,6 +43,7 @@ public:
    *   - arguments = {}
    *   - parameter_overrides = {}
    *   - use_global_arguments = true
+   *   - enable_rosout = true
    *   - use_intra_process_comms = false
    *   - enable_topic_statistics = false
    *   - start_parameter_services = true

rclcpp/include/rclcpp/publisher.hpp

@@ -421,7 +421,7 @@ protected:
   void
   do_inter_process_publish(const ROSMessageType & msg)
   {
-    TRACETOOLS_TRACEPOINT(rclcpp_publish, nullptr, static_cast<const void *>(&msg));
+    TRACEPOINT(rclcpp_publish, nullptr, static_cast<const void *>(&msg));
     auto status = rcl_publish(publisher_handle_.get(), &msg, nullptr);
 
     if (RCL_RET_PUBLISHER_INVALID == status) {
@@ -484,7 +484,7 @@ protected:
     if (!msg) {
       throw std::runtime_error("cannot publish msg which is a null pointer");
     }
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_intra_publish,
       static_cast<const void *>(publisher_handle_.get()),
       msg.get());
@@ -506,7 +506,7 @@ protected:
     if (!msg) {
       throw std::runtime_error("cannot publish msg which is a null pointer");
     }
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_intra_publish,
       static_cast<const void *>(publisher_handle_.get()),
       msg.get());
@@ -529,7 +529,7 @@ protected:
     if (!msg) {
       throw std::runtime_error("cannot publish msg which is a null pointer");
     }
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_intra_publish,
       static_cast<const void *>(publisher_handle_.get()),
       msg.get());

rclcpp/include/rclcpp/publisher_base.hpp

@@ -215,17 +215,6 @@ public:
   std::vector<rclcpp::NetworkFlowEndpoint>
   get_network_flow_endpoints() const;
 
-  /// Return the lowest available capacity for all subscription buffers.
-  /**
-   * For intraprocess communication return the lowest buffer capacity for all subscriptions.
-   * If intraprocess is disabled or no intraprocess subscriptions present, return maximum of size_t.
-   * On failure return 0.
-   * \return lowest buffer capacity for all subscriptions
-   */
-  RCLCPP_PUBLIC
-  size_t
-  lowest_available_ipm_capacity() const;
-
   /// Wait until all published messages are acknowledged or until the specified timeout elapses.
   /**
    * This method waits until all published messages are acknowledged by all matching

rclcpp/include/rclcpp/service.hpp

@@ -265,19 +265,15 @@ protected:
 
   std::shared_ptr<rcl_node_t> node_handle_;
 
-  std::recursive_mutex callback_mutex_;
-  // It is important to declare on_new_request_callback_ before
-  // service_handle_, so on destruction the service is
-  // destroyed first. Otherwise, the rmw service callback
-  // would point briefly to a destroyed function.
-  std::function<void(size_t)> on_new_request_callback_{nullptr};
-  // Declare service_handle_ after callback
   std::shared_ptr<rcl_service_t> service_handle_;
   bool owns_rcl_handle_ = true;
 
   rclcpp::Logger node_logger_;
 
   std::atomic<bool> in_use_by_wait_set_{false};
+
+  std::recursive_mutex callback_mutex_;
+  std::function<void(size_t)> on_new_request_callback_{nullptr};
 };
 
 template<typename ServiceT>
@@ -352,7 +348,7 @@ public:
 
       rclcpp::exceptions::throw_from_rcl_error(ret, "could not create service");
     }
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_service_callback_added,
       static_cast<const void *>(get_service_handle().get()),
       static_cast<const void *>(&any_callback_));
@@ -387,7 +383,7 @@ public:
     }
 
     service_handle_ = service_handle;
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_service_callback_added,
       static_cast<const void *>(get_service_handle().get()),
       static_cast<const void *>(&any_callback_));
@@ -424,7 +420,7 @@ public:
     // In this case, rcl owns the service handle memory
     service_handle_ = std::shared_ptr<rcl_service_t>(new rcl_service_t);
     service_handle_->impl = service_handle->impl;
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_service_callback_added,
       static_cast<const void *>(get_service_handle().get()),
       static_cast<const void *>(&any_callback_));
@@ -486,12 +482,20 @@ public:
   {
     rcl_ret_t ret = rcl_send_response(get_service_handle().get(), &req_id, &response);
 
+    if (ret == RCL_RET_TIMEOUT) {
+      RCLCPP_WARN(
+        node_logger_.get_child("rclcpp"),
+        "failed to send response to %s (timeout): %s",
+        this->get_service_name(), rcl_get_error_string().str);
+      rcl_reset_error();
+      return;
+    }
     if (ret != RCL_RET_OK) {
       rclcpp::exceptions::throw_from_rcl_error(ret, "failed to send response");
     }
   }
 
-  /// Configure client introspection.
+  /// Configure service introspection.
   /**
    * \param[in] clock clock to use to generate introspection timestamps
    * \param[in] qos_service_event_pub QoS settings to use when creating the introspection publisher

rclcpp/include/rclcpp/subscription.hpp

@@ -185,7 +185,7 @@ public:
         this->get_topic_name(),  // important to get like this, as it has the fully-qualified name
         qos_profile,
         resolve_intra_process_buffer_type(options_.intra_process_buffer_type, callback));
-      TRACETOOLS_TRACEPOINT(
+      TRACEPOINT(
         rclcpp_subscription_init,
         static_cast<const void *>(get_subscription_handle().get()),
         static_cast<const void *>(subscription_intra_process_.get()));
@@ -201,11 +201,11 @@ public:
       this->subscription_topic_statistics_ = std::move(subscription_topic_statistics);
     }
 
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_subscription_init,
       static_cast<const void *>(get_subscription_handle().get()),
       static_cast<const void *>(this));
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_subscription_callback_added,
       static_cast<const void *>(this),
       static_cast<const void *>(&any_callback_));

rclcpp/include/rclcpp/subscription_base.hpp

@@ -645,14 +645,6 @@ protected:
   rclcpp::node_interfaces::NodeBaseInterface * const node_base_;
 
   std::shared_ptr<rcl_node_t> node_handle_;
-
-  std::recursive_mutex callback_mutex_;
-  // It is important to declare on_new_message_callback_ before
-  // subscription_handle_, so on destruction the subscription is
-  // destroyed first. Otherwise, the rmw subscription callback
-  // would point briefly to a destroyed function.
-  std::function<void(size_t)> on_new_message_callback_{nullptr};
-  // Declare subscription_handle_ after callback
   std::shared_ptr<rcl_subscription_t> subscription_handle_;
   std::shared_ptr<rcl_subscription_t> intra_process_subscription_handle_;
   rclcpp::Logger node_logger_;
@@ -677,6 +669,9 @@ private:
   std::atomic<bool> intra_process_subscription_waitable_in_use_by_wait_set_{false};
   std::unordered_map<rclcpp::EventHandlerBase *,
     std::atomic<bool>> qos_events_in_use_by_wait_set_;
+
+  std::recursive_mutex callback_mutex_;
+  std::function<void(size_t)> on_new_message_callback_{nullptr};
 };
 
 }  // namespace rclcpp

rclcpp/include/rclcpp/timer.hpp

@@ -223,14 +223,14 @@ public:
   )
   : TimerBase(clock, period, context), callback_(std::forward<FunctorT>(callback))
   {
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_timer_callback_added,
       static_cast<const void *>(get_timer_handle().get()),
       reinterpret_cast<const void *>(&callback_));
 #ifndef TRACETOOLS_DISABLED
-    if (TRACETOOLS_TRACEPOINT_ENABLED(rclcpp_callback_register)) {
+    if (TRACEPOINT_ENABLED(rclcpp_callback_register)) {
       char * symbol = tracetools::get_symbol(callback_);
-      TRACETOOLS_DO_TRACEPOINT(
+      DO_TRACEPOINT(
         rclcpp_callback_register,
         reinterpret_cast<const void *>(&callback_),
         symbol);
@@ -269,9 +269,9 @@ public:
   void
   execute_callback() override
   {
-    TRACETOOLS_TRACEPOINT(callback_start, reinterpret_cast<const void *>(&callback_), false);
+    TRACEPOINT(callback_start, reinterpret_cast<const void *>(&callback_), false);
     execute_callback_delegate<>();
-    TRACETOOLS_TRACEPOINT(callback_end, reinterpret_cast<const void *>(&callback_));
+    TRACEPOINT(callback_end, reinterpret_cast<const void *>(&callback_));
   }
 
   // void specialization

rclcpp/include/rclcpp/wait_set_policies/detail/storage_policy_common.hpp

@@ -104,7 +104,7 @@ protected:
       // TODO(wjwwood): support custom allocator, maybe restrict to polymorphic allocator
       rcl_get_default_allocator());
     if (RCL_RET_OK != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret, "Failed to create wait set");
+      rclcpp::exceptions::throw_from_rcl_error(ret);
     }
 
     // (Re)build the wait set for the first time.
@@ -192,7 +192,8 @@ protected:
       size_t services_from_waitables = 0;
       size_t events_from_waitables = 0;
       for (const auto & waitable_entry : waitables) {
-        if (!waitable_entry.waitable) {
+        auto waitable_ptr_pair = get_raw_pointer_from_smart_pointer(waitable_entry.waitable);
+        if (nullptr == waitable_ptr_pair.second) {
           // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
           if (HasStrongOwnership) {
             // This will not happen in fixed sized storage, as it holds
@@ -203,13 +204,13 @@ protected:
           needs_pruning_ = true;
           continue;
         }
-        const auto & waitable = waitable_entry.waitable;
-        subscriptions_from_waitables += waitable->get_number_of_ready_subscriptions();
-        guard_conditions_from_waitables += waitable->get_number_of_ready_guard_conditions();
-        timers_from_waitables += waitable->get_number_of_ready_timers();
-        clients_from_waitables += waitable->get_number_of_ready_clients();
-        services_from_waitables += waitable->get_number_of_ready_services();
-        events_from_waitables += waitable->get_number_of_ready_events();
+        rclcpp::Waitable & waitable = *waitable_ptr_pair.second;
+        subscriptions_from_waitables += waitable.get_number_of_ready_subscriptions();
+        guard_conditions_from_waitables += waitable.get_number_of_ready_guard_conditions();
+        timers_from_waitables += waitable.get_number_of_ready_timers();
+        clients_from_waitables += waitable.get_number_of_ready_clients();
+        services_from_waitables += waitable.get_number_of_ready_services();
+        events_from_waitables += waitable.get_number_of_ready_events();
       }
       rcl_ret_t ret = rcl_wait_set_resize(
         &rcl_wait_set_,
@@ -221,7 +222,7 @@ protected:
         events_from_waitables
       );
       if (RCL_RET_OK != ret) {
-        rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't resize the wait set");
+        rclcpp::exceptions::throw_from_rcl_error(ret);
       }
       was_resized = true;
       // Assumption: the calling code ensures this function is not called
@@ -237,13 +238,15 @@ protected:
     if (!was_resized) {
       rcl_ret_t ret = rcl_wait_set_clear(&rcl_wait_set_);
       if (RCL_RET_OK != ret) {
-        rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't clear the wait set");
+        rclcpp::exceptions::throw_from_rcl_error(ret);
       }
     }
 
     // Add subscriptions.
     for (const auto & subscription_entry : subscriptions) {
-      if (!subscription_entry.subscription) {
+      auto subscription_ptr_pair =
+        get_raw_pointer_from_smart_pointer(subscription_entry.subscription);
+      if (nullptr == subscription_ptr_pair.second) {
         // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
         if (HasStrongOwnership) {
           // This will not happen in fixed sized storage, as it holds
@@ -254,13 +257,12 @@ protected:
         needs_pruning_ = true;
         continue;
       }
-
       rcl_ret_t ret = rcl_wait_set_add_subscription(
         &rcl_wait_set_,
-        subscription_entry.subscription->get_subscription_handle().get(),
+        subscription_ptr_pair.second->get_subscription_handle().get(),
         nullptr);
       if (RCL_RET_OK != ret) {
-        rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't fill wait set");
+        rclcpp::exceptions::throw_from_rcl_error(ret);
       }
     }
 
@@ -269,7 +271,8 @@ protected:
       [this](const auto & inner_guard_conditions)
       {
         for (const auto & guard_condition : inner_guard_conditions) {
-          if (!guard_condition) {
+          auto guard_condition_ptr_pair = get_raw_pointer_from_smart_pointer(guard_condition);
+          if (nullptr == guard_condition_ptr_pair.second) {
             // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
             if (HasStrongOwnership) {
               // This will not happen in fixed sized storage, as it holds
@@ -282,10 +285,10 @@ protected:
           }
           rcl_ret_t ret = rcl_wait_set_add_guard_condition(
             &rcl_wait_set_,
-            &guard_condition->get_rcl_guard_condition(),
+            &guard_condition_ptr_pair.second->get_rcl_guard_condition(),
             nullptr);
           if (RCL_RET_OK != ret) {
-            rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't fill wait set");
+            rclcpp::exceptions::throw_from_rcl_error(ret);
           }
         }
       };
@@ -298,7 +301,8 @@ protected:
 
     // Add timers.
     for (const auto & timer : timers) {
-      if (!timer) {
+      auto timer_ptr_pair = get_raw_pointer_from_smart_pointer(timer);
+      if (nullptr == timer_ptr_pair.second) {
         // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
         if (HasStrongOwnership) {
           // This will not happen in fixed sized storage, as it holds
@@ -311,16 +315,17 @@ protected:
       }
       rcl_ret_t ret = rcl_wait_set_add_timer(
         &rcl_wait_set_,
-        timer->get_timer_handle().get(),
+        timer_ptr_pair.second->get_timer_handle().get(),
         nullptr);
       if (RCL_RET_OK != ret) {
-        rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't fill wait set");
+        rclcpp::exceptions::throw_from_rcl_error(ret);
       }
     }
 
     // Add clients.
     for (const auto & client : clients) {
-      if (!client) {
+      auto client_ptr_pair = get_raw_pointer_from_smart_pointer(client);
+      if (nullptr == client_ptr_pair.second) {
         // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
         if (HasStrongOwnership) {
           // This will not happen in fixed sized storage, as it holds
@@ -333,17 +338,17 @@ protected:
       }
       rcl_ret_t ret = rcl_wait_set_add_client(
         &rcl_wait_set_,
-        client->get_client_handle().get(),
+        client_ptr_pair.second->get_client_handle().get(),
         nullptr);
       if (RCL_RET_OK != ret) {
         rclcpp::exceptions::throw_from_rcl_error(ret);
       }
     }
 
-
     // Add services.
     for (const auto & service : services) {
-      if (!service) {
+      auto service_ptr_pair = get_raw_pointer_from_smart_pointer(service);
+      if (nullptr == service_ptr_pair.second) {
         // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
         if (HasStrongOwnership) {
           // This will not happen in fixed sized storage, as it holds
@@ -356,16 +361,17 @@ protected:
       }
       rcl_ret_t ret = rcl_wait_set_add_service(
         &rcl_wait_set_,
-        service->get_service_handle().get(),
+        service_ptr_pair.second->get_service_handle().get(),
         nullptr);
       if (RCL_RET_OK != ret) {
-        rclcpp::exceptions::throw_from_rcl_error(ret, "Couldn't fill wait set");
+        rclcpp::exceptions::throw_from_rcl_error(ret);
       }
     }
 
     // Add waitables.
     for (auto & waitable_entry : waitables) {
-      if (!waitable_entry.waitable) {
+      auto waitable_ptr_pair = get_raw_pointer_from_smart_pointer(waitable_entry.waitable);
+      if (nullptr == waitable_ptr_pair.second) {
         // In this case it was probably stored as a weak_ptr, but is now locking to nullptr.
         if (HasStrongOwnership) {
           // This will not happen in fixed sized storage, as it holds
@@ -376,7 +382,8 @@ protected:
         needs_pruning_ = true;
         continue;
       }
-      waitable_entry.waitable->add_to_wait_set(&rcl_wait_set_);
+      rclcpp::Waitable & waitable = *waitable_ptr_pair.second;
+      waitable.add_to_wait_set(&rcl_wait_set_);
     }
   }
 

rclcpp/include/rclcpp/wait_set_policies/dynamic_storage.hpp

@@ -204,19 +204,15 @@ public:
   void
   storage_rebuild_rcl_wait_set(const ArrayOfExtraGuardConditions & extra_guard_conditions)
   {
-    this->storage_acquire_ownerships();
-
     this->storage_rebuild_rcl_wait_set_with_sets(
-      shared_subscriptions_,
-      shared_guard_conditions_,
+      subscriptions_,
+      guard_conditions_,
       extra_guard_conditions,
-      shared_timers_,
-      shared_clients_,
-      shared_services_,
-      shared_waitables_
+      timers_,
+      clients_,
+      services_,
+      waitables_
     );
-
-    this->storage_release_ownerships();
   }
 
   template<class EntityT, class SequenceOfEntitiesT>
@@ -411,7 +407,6 @@ public:
         }
       };
     // Lock all the weak pointers and hold them until released.
-    lock_all(subscriptions_, shared_subscriptions_);
     lock_all(guard_conditions_, shared_guard_conditions_);
     lock_all(timers_, shared_timers_);
     lock_all(clients_, shared_clients_);
@@ -443,7 +438,6 @@ public:
           shared_ptr.reset();
         }
       };
-    reset_all(shared_subscriptions_);
     reset_all(shared_guard_conditions_);
     reset_all(shared_timers_);
     reset_all(shared_clients_);

rclcpp/include/rclcpp/wait_set_policies/sequential_synchronization.hpp

@@ -290,7 +290,7 @@ protected:
         return create_wait_result(WaitResultKind::Empty);
       } else {
         // Some other error case, throw.
-        rclcpp::exceptions::throw_from_rcl_error(ret, "rcl_wait() failed");
+        rclcpp::exceptions::throw_from_rcl_error(ret);
       }
     } while (should_loop());
 

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>21.2.0</version>
+  <version>21.0.8</version>
   <description>The ROS client library in C++.</description>
 
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>

rclcpp/src/rclcpp/any_executable.cpp

@@ -16,16 +16,13 @@
 
 using rclcpp::AnyExecutable;
 
-RCLCPP_PUBLIC
 AnyExecutable::AnyExecutable()
 : subscription(nullptr),
   timer(nullptr),
   service(nullptr),
   client(nullptr),
-  waitable(nullptr),
   callback_group(nullptr),
-  node_base(nullptr),
-  data(nullptr)
+  node_base(nullptr)
 {}
 
 AnyExecutable::~AnyExecutable()

rclcpp/src/rclcpp/callback_group.cpp

@@ -66,7 +66,6 @@ CallbackGroup::size() const
     timer_ptrs_.size() +
     waitable_ptrs_.size();
 }
-
 void CallbackGroup::collect_all_ptrs(
   std::function<void(const rclcpp::SubscriptionBase::SharedPtr &)> sub_func,
   std::function<void(const rclcpp::ServiceBase::SharedPtr &)> service_func,

rclcpp/src/rclcpp/context.cpp

@@ -496,7 +496,7 @@ Context::sleep_for(const std::chrono::nanoseconds & nanoseconds)
       std::unique_lock<std::mutex> lock(interrupt_mutex_);
       auto start = std::chrono::steady_clock::now();
       // this will release the lock while waiting
-      interrupt_condition_variable_.wait_for(lock, nanoseconds);
+      interrupt_condition_variable_.wait_for(lock, time_left);
       time_left -= std::chrono::steady_clock::now() - start;
     }
   } while (time_left > std::chrono::nanoseconds::zero() && this->is_valid());

rclcpp/src/rclcpp/event_handler.cpp

@@ -39,13 +39,6 @@ UnsupportedEventTypeException::UnsupportedEventTypeException(
 
 EventHandlerBase::~EventHandlerBase()
 {
-  // Since the rmw event listener holds a reference to
-  // this callback, we need to clear it on destruction of this class.
-  // This clearing is not needed for other rclcpp entities like pub/subs, since
-  // they do own the underlying rmw entities, which are destroyed
-  // on their rclcpp destructors, thus no risk of dangling pointers.
-  clear_on_ready_callback();
-
   if (rcl_event_fini(&event_handle_) != RCL_RET_OK) {
     RCUTILS_LOG_ERROR_NAMED(
       "rclcpp",

rclcpp/src/rclcpp/executor.cpp

@@ -13,8 +13,6 @@
 // limitations under the License.
 
 #include <algorithm>
-#include <chrono>
-#include <iterator>
 #include <memory>
 #include <map>
 #include <string>
@@ -24,14 +22,13 @@
 
 #include "rcl/allocator.h"
 #include "rcl/error_handling.h"
-#include "rclcpp/executors/executor_notify_waitable.hpp"
-#include "rclcpp/subscription_wait_set_mask.hpp"
 #include "rcpputils/scope_exit.hpp"
 
 #include "rclcpp/dynamic_typesupport/dynamic_message.hpp"
 #include "rclcpp/exceptions.hpp"
 #include "rclcpp/executor.hpp"
 #include "rclcpp/guard_condition.hpp"
+#include "rclcpp/memory_strategy.hpp"
 #include "rclcpp/node.hpp"
 #include "rclcpp/utilities.hpp"
 
@@ -41,25 +38,16 @@
 
 using namespace std::chrono_literals;
 
+using rclcpp::exceptions::throw_from_rcl_error;
 using rclcpp::Executor;
 
-/// Mask to indicate to the waitset to only add the subscription.
-/// The events and intraprocess waitable are already added via the callback group.
-static constexpr rclcpp::SubscriptionWaitSetMask kDefaultSubscriptionMask = {true, false, false};
-
 class rclcpp::ExecutorImplementation {};
 
 Executor::Executor(const rclcpp::ExecutorOptions & options)
 : spinning(false),
   interrupt_guard_condition_(std::make_shared<rclcpp::GuardCondition>(options.context)),
   shutdown_guard_condition_(std::make_shared<rclcpp::GuardCondition>(options.context)),
-  notify_waitable_(std::make_shared<rclcpp::executors::ExecutorNotifyWaitable>(
-      [this]() {
-        this->entities_need_rebuild_.store(true);
-      })),
-  collector_(notify_waitable_),
-  wait_set_({}, {}, {}, {}, {}, {}, options.context),
-  current_notify_waitable_(notify_waitable_),
+  memory_strategy_(options.memory_strategy),
   impl_(std::make_unique<rclcpp::ExecutorImplementation>())
 {
   // Store the context for later use.
@@ -73,41 +61,74 @@ Executor::Executor(const rclcpp::ExecutorOptions & options)
       }
     });
 
-  notify_waitable_->add_guard_condition(interrupt_guard_condition_);
-  notify_waitable_->add_guard_condition(shutdown_guard_condition_);
+  // The number of guard conditions is always at least 2: 1 for the ctrl-c guard cond,
+  // and one for the executor's guard cond (interrupt_guard_condition_)
+  memory_strategy_->add_guard_condition(*shutdown_guard_condition_.get());
+
+  // Put the executor's guard condition in
+  memory_strategy_->add_guard_condition(*interrupt_guard_condition_.get());
+  rcl_allocator_t allocator = memory_strategy_->get_allocator();
+
+  rcl_ret_t ret = rcl_wait_set_init(
+    &wait_set_,
+    0, 2, 0, 0, 0, 0,
+    context_->get_rcl_context().get(),
+    allocator);
+  if (RCL_RET_OK != ret) {
+    RCUTILS_LOG_ERROR_NAMED(
+      "rclcpp",
+      "failed to create wait set: %s", rcl_get_error_string().str);
+    rcl_reset_error();
+    throw_from_rcl_error(ret, "Failed to create wait set in Executor constructor");
+  }
 }
 
 Executor::~Executor()
 {
-  std::lock_guard<std::mutex> guard(mutex_);
-
-  notify_waitable_->remove_guard_condition(interrupt_guard_condition_);
-  notify_waitable_->remove_guard_condition(shutdown_guard_condition_);
-  current_collection_.timers.update(
-    {}, {},
-    [this](auto timer) {wait_set_.remove_timer(timer);});
-
-  current_collection_.subscriptions.update(
-    {}, {},
-    [this](auto subscription) {
-      wait_set_.remove_subscription(subscription, kDefaultSubscriptionMask);
+  // Disassociate all callback groups.
+  for (auto & pair : weak_groups_to_nodes_) {
+    auto group = pair.first.lock();
+    if (group) {
+      std::atomic_bool & has_executor = group->get_associated_with_executor_atomic();
+      has_executor.store(false);
+    }
+  }
+  // Disassociate all nodes.
+  std::for_each(
+    weak_nodes_.begin(), weak_nodes_.end(), []
+      (rclcpp::node_interfaces::NodeBaseInterface::WeakPtr weak_node_ptr) {
+      auto shared_node_ptr = weak_node_ptr.lock();
+      if (shared_node_ptr) {
+        std::atomic_bool & has_executor = shared_node_ptr->get_associated_with_executor_atomic();
+        has_executor.store(false);
+      }
     });
+  weak_nodes_.clear();
+  weak_groups_associated_with_executor_to_nodes_.clear();
+  weak_groups_to_nodes_associated_with_executor_.clear();
+  weak_groups_to_nodes_.clear();
+  for (const auto & pair : weak_groups_to_guard_conditions_) {
+    auto guard_condition = pair.second;
+    memory_strategy_->remove_guard_condition(guard_condition);
+  }
+  weak_groups_to_guard_conditions_.clear();
 
-  current_collection_.clients.update(
-    {}, {},
-    [this](auto client) {wait_set_.remove_client(client);});
+  for (const auto & pair : weak_nodes_to_guard_conditions_) {
+    auto guard_condition = pair.second;
+    memory_strategy_->remove_guard_condition(guard_condition);
+  }
+  weak_nodes_to_guard_conditions_.clear();
 
-  current_collection_.services.update(
-    {}, {},
-    [this](auto service) {wait_set_.remove_service(service);});
-
-  current_collection_.guard_conditions.update(
-    {}, {},
-    [this](auto guard_condition) {wait_set_.remove_guard_condition(guard_condition);});
-
-  current_collection_.waitables.update(
-    {}, {},
-    [this](auto waitable) {wait_set_.remove_waitable(waitable);});
+  // Finalize the wait set.
+  if (rcl_wait_set_fini(&wait_set_) != RCL_RET_OK) {
+    RCUTILS_LOG_ERROR_NAMED(
+      "rclcpp",
+      "failed to destroy wait set: %s", rcl_get_error_string().str);
+    rcl_reset_error();
+  }
+  // Remove and release the sigint guard condition
+  memory_strategy_->remove_guard_condition(shutdown_guard_condition_.get());
+  memory_strategy_->remove_guard_condition(interrupt_guard_condition_.get());
 
   // Remove shutdown callback handle registered to Context
   if (!context_->remove_on_shutdown_callback(shutdown_callback_handle_)) {
@@ -121,39 +142,95 @@ Executor::~Executor()
 std::vector<rclcpp::CallbackGroup::WeakPtr>
 Executor::get_all_callback_groups()
 {
-  this->collector_.update_collections();
-  return this->collector_.get_all_callback_groups();
+  std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
+  std::lock_guard<std::mutex> guard{mutex_};
+  for (const auto & group_node_ptr : weak_groups_associated_with_executor_to_nodes_) {
+    groups.push_back(group_node_ptr.first);
+  }
+  for (auto const & group_node_ptr : weak_groups_to_nodes_associated_with_executor_) {
+    groups.push_back(group_node_ptr.first);
+  }
+  return groups;
 }
 
 std::vector<rclcpp::CallbackGroup::WeakPtr>
 Executor::get_manually_added_callback_groups()
 {
-  this->collector_.update_collections();
-  return this->collector_.get_manually_added_callback_groups();
+  std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
+  std::lock_guard<std::mutex> guard{mutex_};
+  for (auto const & group_node_ptr : weak_groups_associated_with_executor_to_nodes_) {
+    groups.push_back(group_node_ptr.first);
+  }
+  return groups;
 }
 
 std::vector<rclcpp::CallbackGroup::WeakPtr>
 Executor::get_automatically_added_callback_groups_from_nodes()
 {
-  this->collector_.update_collections();
-  return this->collector_.get_automatically_added_callback_groups();
+  std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
+  std::lock_guard<std::mutex> guard{mutex_};
+  for (auto const & group_node_ptr : weak_groups_to_nodes_associated_with_executor_) {
+    groups.push_back(group_node_ptr.first);
+  }
+  return groups;
 }
 
 void
-Executor::add_callback_group(
+Executor::add_callback_groups_from_nodes_associated_to_executor()
+{
+  for (auto & weak_node : weak_nodes_) {
+    auto node = weak_node.lock();
+    if (node) {
+      node->for_each_callback_group(
+        [this, node](rclcpp::CallbackGroup::SharedPtr shared_group_ptr)
+        {
+          if (
+            shared_group_ptr->automatically_add_to_executor_with_node() &&
+            !shared_group_ptr->get_associated_with_executor_atomic().load())
+          {
+            this->add_callback_group_to_map(
+              shared_group_ptr,
+              node,
+              weak_groups_to_nodes_associated_with_executor_,
+              true);
+          }
+        });
+    }
+  }
+}
+
+void
+Executor::add_callback_group_to_map(
   rclcpp::CallbackGroup::SharedPtr group_ptr,
   rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+  rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
   bool notify)
 {
-  (void) node_ptr;
-  this->collector_.add_callback_group(group_ptr);
+  // If the callback_group already has an executor
+  std::atomic_bool & has_executor = group_ptr->get_associated_with_executor_atomic();
+  if (has_executor.exchange(true)) {
+    throw std::runtime_error("Callback group has already been added to an executor.");
+  }
 
-  if (!spinning.load()) {
-    std::lock_guard<std::mutex> guard(mutex_);
-    this->collect_entities();
+  rclcpp::CallbackGroup::WeakPtr weak_group_ptr = group_ptr;
+  auto insert_info =
+    weak_groups_to_nodes.insert(std::make_pair(weak_group_ptr, node_ptr));
+  bool was_inserted = insert_info.second;
+  if (!was_inserted) {
+    throw std::runtime_error("Callback group was already added to executor.");
+  }
+  // Also add to the map that contains all callback groups
+  weak_groups_to_nodes_.insert(std::make_pair(weak_group_ptr, node_ptr));
+
+  if (node_ptr->get_context()->is_valid()) {
+    auto callback_group_guard_condition = group_ptr->get_notify_guard_condition();
+    weak_groups_to_guard_conditions_[weak_group_ptr] = callback_group_guard_condition.get();
+    // Add the callback_group's notify condition to the guard condition handles
+    memory_strategy_->add_guard_condition(*callback_group_guard_condition);
   }
 
   if (notify) {
+    // Interrupt waiting to handle new node
     try {
       interrupt_guard_condition_->trigger();
     } catch (const rclcpp::exceptions::RCLError & ex) {
@@ -164,23 +241,91 @@ Executor::add_callback_group(
   }
 }
 
+void
+Executor::add_callback_group(
+  rclcpp::CallbackGroup::SharedPtr group_ptr,
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+  bool notify)
+{
+  std::lock_guard<std::mutex> guard{mutex_};
+  this->add_callback_group_to_map(
+    group_ptr,
+    node_ptr,
+    weak_groups_associated_with_executor_to_nodes_,
+    notify);
+}
+
 void
 Executor::add_node(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, bool notify)
 {
-  this->collector_.add_node(node_ptr);
-
-  if (!spinning.load()) {
-    std::lock_guard<std::mutex> guard(mutex_);
-    this->collect_entities();
+  // If the node already has an executor
+  std::atomic_bool & has_executor = node_ptr->get_associated_with_executor_atomic();
+  if (has_executor.exchange(true)) {
+    throw std::runtime_error(
+            std::string("Node '") + node_ptr->get_fully_qualified_name() +
+            "' has already been added to an executor.");
   }
+  std::lock_guard<std::mutex> guard{mutex_};
+  node_ptr->for_each_callback_group(
+    [this, node_ptr, notify](rclcpp::CallbackGroup::SharedPtr group_ptr)
+    {
+      if (!group_ptr->get_associated_with_executor_atomic().load() &&
+      group_ptr->automatically_add_to_executor_with_node())
+      {
+        this->add_callback_group_to_map(
+          group_ptr,
+          node_ptr,
+          weak_groups_to_nodes_associated_with_executor_,
+          notify);
+      }
+    });
 
-  if (notify) {
-    try {
-      interrupt_guard_condition_->trigger();
-    } catch (const rclcpp::exceptions::RCLError & ex) {
-      throw std::runtime_error(
-              std::string(
-                "Failed to trigger guard condition on node add: ") + ex.what());
+  const auto gc = node_ptr->get_shared_notify_guard_condition();
+  weak_nodes_to_guard_conditions_[node_ptr] = gc.get();
+  // Add the node's notify condition to the guard condition handles
+  memory_strategy_->add_guard_condition(*gc);
+  weak_nodes_.push_back(node_ptr);
+}
+
+void
+Executor::remove_callback_group_from_map(
+  rclcpp::CallbackGroup::SharedPtr group_ptr,
+  rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap & weak_groups_to_nodes,
+  bool notify)
+{
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr;
+  rclcpp::CallbackGroup::WeakPtr weak_group_ptr = group_ptr;
+  auto iter = weak_groups_to_nodes.find(weak_group_ptr);
+  if (iter != weak_groups_to_nodes.end()) {
+    node_ptr = iter->second.lock();
+    if (node_ptr == nullptr) {
+      throw std::runtime_error("Node must not be deleted before its callback group(s).");
+    }
+    weak_groups_to_nodes.erase(iter);
+    weak_groups_to_nodes_.erase(group_ptr);
+    std::atomic_bool & has_executor = group_ptr->get_associated_with_executor_atomic();
+    has_executor.store(false);
+  } else {
+    throw std::runtime_error("Callback group needs to be associated with executor.");
+  }
+  // If the node was matched and removed, interrupt waiting.
+  if (!has_node(node_ptr, weak_groups_to_nodes_associated_with_executor_) &&
+    !has_node(node_ptr, weak_groups_associated_with_executor_to_nodes_))
+  {
+    auto iter = weak_groups_to_guard_conditions_.find(weak_group_ptr);
+    if (iter != weak_groups_to_guard_conditions_.end()) {
+      memory_strategy_->remove_guard_condition(iter->second);
+    }
+    weak_groups_to_guard_conditions_.erase(weak_group_ptr);
+
+    if (notify) {
+      try {
+        interrupt_guard_condition_->trigger();
+      } catch (const rclcpp::exceptions::RCLError & ex) {
+        throw std::runtime_error(
+                std::string(
+                  "Failed to trigger guard condition on callback group remove: ") + ex.what());
+      }
     }
   }
 }
@@ -190,21 +335,11 @@ Executor::remove_callback_group(
   rclcpp::CallbackGroup::SharedPtr group_ptr,
   bool notify)
 {
-  this->collector_.remove_callback_group(group_ptr);
-
-  if (!spinning.load()) {
-    std::lock_guard<std::mutex> guard(mutex_);
-    this->collect_entities();
-  }
-  if (notify) {
-    try {
-      interrupt_guard_condition_->trigger();
-    } catch (const rclcpp::exceptions::RCLError & ex) {
-      throw std::runtime_error(
-              std::string(
-                "Failed to trigger guard condition on callback group remove: ") + ex.what());
-    }
-  }
+  std::lock_guard<std::mutex> guard{mutex_};
+  this->remove_callback_group_from_map(
+    group_ptr,
+    weak_groups_associated_with_executor_to_nodes_,
+    notify);
 }
 
 void
@@ -216,22 +351,48 @@ Executor::add_node(std::shared_ptr<rclcpp::Node> node_ptr, bool notify)
 void
 Executor::remove_node(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, bool notify)
 {
-  this->collector_.remove_node(node_ptr);
-
-  if (!spinning.load()) {
-    std::lock_guard<std::mutex> guard(mutex_);
-    this->collect_entities();
+  if (!node_ptr->get_associated_with_executor_atomic().load()) {
+    throw std::runtime_error("Node needs to be associated with an executor.");
   }
 
-  if (notify) {
-    try {
-      interrupt_guard_condition_->trigger();
-    } catch (const rclcpp::exceptions::RCLError & ex) {
-      throw std::runtime_error(
-              std::string(
-                "Failed to trigger guard condition on node remove: ") + ex.what());
+  std::lock_guard<std::mutex> guard{mutex_};
+  bool found_node = false;
+  auto node_it = weak_nodes_.begin();
+  while (node_it != weak_nodes_.end()) {
+    bool matched = (node_it->lock() == node_ptr);
+    if (matched) {
+      found_node = true;
+      node_it = weak_nodes_.erase(node_it);
+    } else {
+      ++node_it;
     }
   }
+  if (!found_node) {
+    throw std::runtime_error("Node needs to be associated with this executor.");
+  }
+
+  for (auto it = weak_groups_to_nodes_associated_with_executor_.begin();
+    it != weak_groups_to_nodes_associated_with_executor_.end(); )
+  {
+    auto weak_node_ptr = it->second;
+    auto shared_node_ptr = weak_node_ptr.lock();
+    auto group_ptr = it->first.lock();
+
+    // Increment iterator before removing in case it's invalidated
+    it++;
+    if (shared_node_ptr == node_ptr) {
+      remove_callback_group_from_map(
+        group_ptr,
+        weak_groups_to_nodes_associated_with_executor_,
+        notify);
+    }
+  }
+
+  memory_strategy_->remove_guard_condition(node_ptr->get_shared_notify_guard_condition().get());
+  weak_nodes_to_guard_conditions_.erase(node_ptr);
+
+  std::atomic_bool & has_executor = node_ptr->get_associated_with_executor_atomic();
+  has_executor.store(false);
 }
 
 void
@@ -298,44 +459,21 @@ Executor::spin_some_impl(std::chrono::nanoseconds max_duration, bool exhaustive)
     throw std::runtime_error("spin_some() called while already spinning");
   }
   RCPPUTILS_SCOPE_EXIT(this->spinning.store(false); );
-
-  size_t work_in_queue = 0;
-  bool has_waited = false;
-
-  {
-    std::lock_guard<std::mutex> lock(mutex_);
-    work_in_queue = ready_executables_.size();
-  }
-  // The logic below is to guarantee that we:
-  // a) run all of the work in the queue before we spin the first time
-  // b) spin at least once
-  // c) run all of the work in the queue after we spin
-
+  bool work_available = false;
   while (rclcpp::ok(context_) && spinning.load() && max_duration_not_elapsed()) {
     AnyExecutable any_exec;
-    if (work_in_queue > 0) {
-      // If there is work in the queue, then execute it
-      // This covers the case that there are things left in the queue from a
-      // previous spin.
-      if (get_next_ready_executable(any_exec)) {
-        execute_any_executable(any_exec);
-      }
-    } else if (!has_waited && !work_in_queue) {
-      // Once the ready queue is empty, then we need to wait at least once.
-      wait_for_work(std::chrono::milliseconds(0));
-      has_waited = true;
-    } else if (has_waited && !work_in_queue) {
-      // Once we have emptied the ready queue, but have already waited:
-      if (!exhaustive) {
-        // In the case of spin some, then we can exit
-        break;
-      } else {
-        // In the case of spin all, then we will allow ourselves to wait again.
-        has_waited = false;
-      }
+    if (!work_available) {
+      wait_for_work(std::chrono::milliseconds::zero());
+    }
+    if (get_next_ready_executable(any_exec)) {
+      execute_any_executable(any_exec);
+      work_available = true;
+    } else {
+      if (!work_available || !exhaustive) {
+        break;
+      }
+      work_available = false;
     }
-    std::lock_guard<std::mutex> lock(mutex_);
-    work_in_queue = ready_executables_.size();
   }
 }
 
@@ -370,21 +508,30 @@ Executor::cancel()
   }
 }
 
+void
+Executor::set_memory_strategy(rclcpp::memory_strategy::MemoryStrategy::SharedPtr memory_strategy)
+{
+  if (memory_strategy == nullptr) {
+    throw std::runtime_error("Received NULL memory strategy in executor.");
+  }
+  std::lock_guard<std::mutex> guard{mutex_};
+  memory_strategy_ = memory_strategy;
+}
+
 void
 Executor::execute_any_executable(AnyExecutable & any_exec)
 {
   if (!spinning.load()) {
     return;
   }
-
   if (any_exec.timer) {
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_executor_execute,
       static_cast<const void *>(any_exec.timer->get_timer_handle().get()));
     execute_timer(any_exec.timer);
   }
   if (any_exec.subscription) {
-    TRACETOOLS_TRACEPOINT(
+    TRACEPOINT(
       rclcpp_executor_execute,
       static_cast<const void *>(any_exec.subscription->get_subscription_handle().get()));
     execute_subscription(any_exec.subscription);
@@ -398,10 +545,16 @@ Executor::execute_any_executable(AnyExecutable & any_exec)
   if (any_exec.waitable) {
     any_exec.waitable->execute(any_exec.data);
   }
-
   // Reset the callback_group, regardless of type
-  if (any_exec.callback_group) {
-    any_exec.callback_group->can_be_taken_from().store(true);
+  any_exec.callback_group->can_be_taken_from().store(true);
+  // Wake the wait, because it may need to be recalculated or work that
+  // was previously blocked is now available.
+  try {
+    interrupt_guard_condition_->trigger();
+  } catch (const rclcpp::exceptions::RCLError & ex) {
+    throw std::runtime_error(
+            std::string(
+              "Failed to trigger guard condition from execute_any_executable: ") + ex.what());
   }
 }
 
@@ -500,6 +653,11 @@ Executor::execute_subscription(rclcpp::SubscriptionBase::SharedPtr subscription)
             subscription->get_topic_name(),
             [&]() {return subscription->take_type_erased(message.get(), message_info);},
             [&]() {subscription->handle_message(message, message_info);});
+          // TODO(clalancette): In the case that the user is using the MessageMemoryPool,
+          // and they take a shared_ptr reference to the message in the callback, this can
+          // inadvertently return the message to the pool when the user is still using it.
+          // This is a bug that needs to be fixed in the pool, and we should probably have
+          // a custom deleter for the message that actually does the return_message().
           subscription->return_message(message);
         }
         break;
@@ -570,111 +728,228 @@ Executor::execute_client(
     [&]() {client->handle_response(request_header, response);});
 }
 
-void
-Executor::collect_entities()
-{
-  // Get the current list of available waitables from the collector.
-  rclcpp::executors::ExecutorEntitiesCollection collection;
-  this->collector_.update_collections();
-  auto callback_groups = this->collector_.get_all_callback_groups();
-  rclcpp::executors::build_entities_collection(callback_groups, collection);
-
-  // Make a copy of notify waitable so we can continue to mutate the original
-  // one outside of the execute loop.
-  // This prevents the collection of guard conditions in the waitable from changing
-  // while we are waiting on it.
-  if (notify_waitable_) {
-    current_notify_waitable_ = std::make_shared<rclcpp::executors::ExecutorNotifyWaitable>(
-      *notify_waitable_);
-    auto notify_waitable = std::static_pointer_cast<rclcpp::Waitable>(current_notify_waitable_);
-    collection.waitables.insert({notify_waitable.get(), {notify_waitable, {}}});
-  }
-
-  // Update each of the groups of entities in the current collection, adding or removing
-  // from the wait set as necessary.
-  current_collection_.timers.update(
-    collection.timers,
-    [this](auto timer) {wait_set_.add_timer(timer);},
-    [this](auto timer) {wait_set_.remove_timer(timer);});
-
-  current_collection_.subscriptions.update(
-    collection.subscriptions,
-    [this](auto subscription) {
-      wait_set_.add_subscription(subscription, kDefaultSubscriptionMask);
-    },
-    [this](auto subscription) {
-      wait_set_.remove_subscription(subscription, kDefaultSubscriptionMask);
-    });
-
-  current_collection_.clients.update(
-    collection.clients,
-    [this](auto client) {wait_set_.add_client(client);},
-    [this](auto client) {wait_set_.remove_client(client);});
-
-  current_collection_.services.update(
-    collection.services,
-    [this](auto service) {wait_set_.add_service(service);},
-    [this](auto service) {wait_set_.remove_service(service);});
-
-  current_collection_.guard_conditions.update(
-    collection.guard_conditions,
-    [this](auto guard_condition) {wait_set_.add_guard_condition(guard_condition);},
-    [this](auto guard_condition) {wait_set_.remove_guard_condition(guard_condition);});
-
-  current_collection_.waitables.update(
-    collection.waitables,
-    [this](auto waitable) {
-      wait_set_.add_waitable(waitable);
-    },
-    [this](auto waitable) {wait_set_.remove_waitable(waitable);});
-
-  // In the case that an entity already has an expired weak pointer
-  // before being removed from the waitset, additionally prune the waitset.
-  this->wait_set_.prune_deleted_entities();
-  this->entities_need_rebuild_.store(false);
-}
-
 void
 Executor::wait_for_work(std::chrono::nanoseconds timeout)
 {
-  TRACETOOLS_TRACEPOINT(rclcpp_executor_wait_for_work, timeout.count());
+  TRACEPOINT(rclcpp_executor_wait_for_work, timeout.count());
   {
     std::lock_guard<std::mutex> guard(mutex_);
-    if (current_collection_.empty() || this->entities_need_rebuild_.load()) {
-      this->collect_entities();
+
+    // Check weak_nodes_ to find any callback group that is not owned
+    // by an executor and add it to the list of callbackgroups for
+    // collect entities. Also exchange to false so it is not
+    // allowed to add to another executor
+    add_callback_groups_from_nodes_associated_to_executor();
+
+    // Collect the subscriptions and timers to be waited on
+    memory_strategy_->clear_handles();
+    bool has_invalid_weak_groups_or_nodes =
+      memory_strategy_->collect_entities(weak_groups_to_nodes_);
+
+    if (has_invalid_weak_groups_or_nodes) {
+      std::vector<rclcpp::CallbackGroup::WeakPtr> invalid_group_ptrs;
+      for (auto pair : weak_groups_to_nodes_) {
+        auto weak_group_ptr = pair.first;
+        auto weak_node_ptr = pair.second;
+        if (weak_group_ptr.expired() || weak_node_ptr.expired()) {
+          invalid_group_ptrs.push_back(weak_group_ptr);
+          auto node_guard_pair = weak_nodes_to_guard_conditions_.find(weak_node_ptr);
+          if (node_guard_pair != weak_nodes_to_guard_conditions_.end()) {
+            auto guard_condition = node_guard_pair->second;
+            weak_nodes_to_guard_conditions_.erase(weak_node_ptr);
+            memory_strategy_->remove_guard_condition(guard_condition);
+          }
+        }
+      }
+      std::for_each(
+        invalid_group_ptrs.begin(), invalid_group_ptrs.end(),
+        [this](rclcpp::CallbackGroup::WeakPtr group_ptr) {
+          if (weak_groups_to_nodes_associated_with_executor_.find(group_ptr) !=
+          weak_groups_to_nodes_associated_with_executor_.end())
+          {
+            weak_groups_to_nodes_associated_with_executor_.erase(group_ptr);
+          }
+          if (weak_groups_associated_with_executor_to_nodes_.find(group_ptr) !=
+          weak_groups_associated_with_executor_to_nodes_.end())
+          {
+            weak_groups_associated_with_executor_to_nodes_.erase(group_ptr);
+          }
+          auto callback_guard_pair = weak_groups_to_guard_conditions_.find(group_ptr);
+          if (callback_guard_pair != weak_groups_to_guard_conditions_.end()) {
+            auto guard_condition = callback_guard_pair->second;
+            weak_groups_to_guard_conditions_.erase(group_ptr);
+            memory_strategy_->remove_guard_condition(guard_condition);
+          }
+          weak_groups_to_nodes_.erase(group_ptr);
+        });
+    }
+
+    // clear wait set
+    rcl_ret_t ret = rcl_wait_set_clear(&wait_set_);
+    if (ret != RCL_RET_OK) {
+      throw_from_rcl_error(ret, "Couldn't clear wait set");
+    }
+
+    // The size of waitables are accounted for in size of the other entities
+    ret = rcl_wait_set_resize(
+      &wait_set_, memory_strategy_->number_of_ready_subscriptions(),
+      memory_strategy_->number_of_guard_conditions(), memory_strategy_->number_of_ready_timers(),
+      memory_strategy_->number_of_ready_clients(), memory_strategy_->number_of_ready_services(),
+      memory_strategy_->number_of_ready_events());
+    if (RCL_RET_OK != ret) {
+      throw_from_rcl_error(ret, "Couldn't resize the wait set");
+    }
+
+    if (!memory_strategy_->add_handles_to_wait_set(&wait_set_)) {
+      throw std::runtime_error("Couldn't fill wait set");
     }
   }
 
-  auto wait_result = wait_set_.wait(timeout);
-  if (wait_result.kind() == WaitResultKind::Empty) {
+  rcl_ret_t status =
+    rcl_wait(&wait_set_, std::chrono::duration_cast<std::chrono::nanoseconds>(timeout).count());
+  if (status == RCL_RET_WAIT_SET_EMPTY) {
     RCUTILS_LOG_WARN_NAMED(
       "rclcpp",
-      "empty wait set received in wait(). This should never happen.");
+      "empty wait set received in rcl_wait(). This should never happen.");
+  } else if (status != RCL_RET_OK && status != RCL_RET_TIMEOUT) {
+    using rclcpp::exceptions::throw_from_rcl_error;
+    throw_from_rcl_error(status, "rcl_wait() failed");
   }
-  rclcpp::executors::ready_executables(current_collection_, wait_result, ready_executables_);
+
+  // check the null handles in the wait set and remove them from the handles in memory strategy
+  // for callback-based entities
+  std::lock_guard<std::mutex> guard(mutex_);
+  memory_strategy_->remove_null_handles(&wait_set_);
+}
+
+rclcpp::node_interfaces::NodeBaseInterface::SharedPtr
+Executor::get_node_by_group(
+  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
+  weak_groups_to_nodes,
+  rclcpp::CallbackGroup::SharedPtr group)
+{
+  if (!group) {
+    return nullptr;
+  }
+  rclcpp::CallbackGroup::WeakPtr weak_group_ptr(group);
+  const auto finder = weak_groups_to_nodes.find(weak_group_ptr);
+  if (finder != weak_groups_to_nodes.end()) {
+    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr = finder->second.lock();
+    return node_ptr;
+  }
+  return nullptr;
+}
+
+rclcpp::CallbackGroup::SharedPtr
+Executor::get_group_by_timer(rclcpp::TimerBase::SharedPtr timer)
+{
+  std::lock_guard<std::mutex> guard{mutex_};
+  for (const auto & pair : weak_groups_associated_with_executor_to_nodes_) {
+    auto group = pair.first.lock();
+    if (!group) {
+      continue;
+    }
+    auto timer_ref = group->find_timer_ptrs_if(
+      [timer](const rclcpp::TimerBase::SharedPtr & timer_ptr) -> bool {
+        return timer_ptr == timer;
+      });
+    if (timer_ref) {
+      return group;
+    }
+  }
+
+  for (const auto & pair : weak_groups_to_nodes_associated_with_executor_) {
+    auto group = pair.first.lock();
+    if (!group) {
+      continue;
+    }
+    auto timer_ref = group->find_timer_ptrs_if(
+      [timer](const rclcpp::TimerBase::SharedPtr & timer_ptr) -> bool {
+        return timer_ptr == timer;
+      });
+    if (timer_ref) {
+      return group;
+    }
+  }
+  return nullptr;
 }
 
 bool
 Executor::get_next_ready_executable(AnyExecutable & any_executable)
 {
-  TRACETOOLS_TRACEPOINT(rclcpp_executor_get_next_ready);
+  bool success = get_next_ready_executable_from_map(any_executable, weak_groups_to_nodes_);
+  return success;
+}
 
-  std::lock_guard<std::mutex> guard(mutex_);
-  if (ready_executables_.size() == 0) {
-    return false;
+bool
+Executor::get_next_ready_executable_from_map(
+  AnyExecutable & any_executable,
+  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
+  weak_groups_to_nodes)
+{
+  TRACEPOINT(rclcpp_executor_get_next_ready);
+  bool success = false;
+  std::lock_guard<std::mutex> guard{mutex_};
+  // Check the timers to see if there are any that are ready
+  memory_strategy_->get_next_timer(any_executable, weak_groups_to_nodes);
+  if (any_executable.timer) {
+    success = true;
+  }
+  if (!success) {
+    // Check the subscriptions to see if there are any that are ready
+    memory_strategy_->get_next_subscription(any_executable, weak_groups_to_nodes);
+    if (any_executable.subscription) {
+      success = true;
+    }
+  }
+  if (!success) {
+    // Check the services to see if there are any that are ready
+    memory_strategy_->get_next_service(any_executable, weak_groups_to_nodes);
+    if (any_executable.service) {
+      success = true;
+    }
+  }
+  if (!success) {
+    // Check the clients to see if there are any that are ready
+    memory_strategy_->get_next_client(any_executable, weak_groups_to_nodes);
+    if (any_executable.client) {
+      success = true;
+    }
+  }
+  if (!success) {
+    // Check the waitables to see if there are any that are ready
+    memory_strategy_->get_next_waitable(any_executable, weak_groups_to_nodes);
+    if (any_executable.waitable) {
+      any_executable.data = any_executable.waitable->take_data();
+      success = true;
+    }
+  }
+  // At this point any_executable should be valid with either a valid subscription
+  // or a valid timer, or it should be a null shared_ptr
+  if (success) {
+    rclcpp::CallbackGroup::WeakPtr weak_group_ptr = any_executable.callback_group;
+    auto iter = weak_groups_to_nodes.find(weak_group_ptr);
+    if (iter == weak_groups_to_nodes.end()) {
+      success = false;
+    }
   }
 
-  any_executable = ready_executables_.front();
-  ready_executables_.pop_front();
-
-  if (any_executable.callback_group &&
-    any_executable.callback_group->type() == CallbackGroupType::MutuallyExclusive)
-  {
-    assert(any_executable.callback_group->can_be_taken_from().load());
-    any_executable.callback_group->can_be_taken_from().store(false);
+  if (success) {
+    // If it is valid, check to see if the group is mutually exclusive or
+    // not, then mark it accordingly ..Check if the callback_group belongs to this executor
+    if (any_executable.callback_group && any_executable.callback_group->type() == \
+      CallbackGroupType::MutuallyExclusive)
+    {
+      // It should not have been taken otherwise
+      assert(any_executable.callback_group->can_be_taken_from().load());
+      // Set to false to indicate something is being run from this group
+      // This is reset to true either when the any_executable is executed or when the
+      // any_executable is destructued
+      any_executable.callback_group->can_be_taken_from().store(false);
+    }
   }
-
-  return true;
+  // If there is no ready executable, return false
+  return success;
 }
 
 bool
@@ -697,6 +972,22 @@ Executor::get_next_executable(AnyExecutable & any_executable, std::chrono::nanos
   return success;
 }
 
+// Returns true iff the weak_groups_to_nodes map has node_ptr as the value in any of its entry.
+bool
+Executor::has_node(
+  const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
+  weak_groups_to_nodes) const
+{
+  return std::find_if(
+    weak_groups_to_nodes.begin(),
+    weak_groups_to_nodes.end(),
+    [&](const WeakCallbackGroupsToNodesMap::value_type & other) -> bool {
+      auto other_ptr = other.second.lock();
+      return other_ptr == node_ptr;
+    }) != weak_groups_to_nodes.end();
+}
+
 bool
 Executor::is_spinning()
 {

rclcpp/src/rclcpp/executors.cpp

@@ -17,7 +17,9 @@
 void
 rclcpp::spin_some(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr)
 {
-  rclcpp::executors::SingleThreadedExecutor exec;
+  rclcpp::ExecutorOptions options;
+  options.context = node_ptr->get_context();
+  rclcpp::executors::SingleThreadedExecutor exec(options);
   exec.spin_node_some(node_ptr);
 }
 
@@ -30,7 +32,9 @@ rclcpp::spin_some(rclcpp::Node::SharedPtr node_ptr)
 void
 rclcpp::spin(rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr)
 {
-  rclcpp::executors::SingleThreadedExecutor exec;
+  rclcpp::ExecutorOptions options;
+  options.context = node_ptr->get_context();
+  rclcpp::executors::SingleThreadedExecutor exec(options);
   exec.add_node(node_ptr);
   exec.spin();
   exec.remove_node(node_ptr);

rclcpp/src/rclcpp/executors/executor_entities_collection.cpp

@@ -39,30 +39,6 @@ void ExecutorEntitiesCollection::clear()
   waitables.clear();
 }
 
-size_t ExecutorEntitiesCollection::remove_expired_entities()
-{
-  auto remove_entities = [](auto & collection) -> size_t {
-      size_t removed = 0;
-      for (auto it = collection.begin(); it != collection.end(); ) {
-        if (it->second.entity.expired()) {
-          ++removed;
-          it = collection.erase(it);
-        } else {
-          ++it;
-        }
-      }
-      return removed;
-    };
-
-  return
-    remove_entities(subscriptions) +
-    remove_entities(timers) +
-    remove_entities(guard_conditions) +
-    remove_entities(clients) +
-    remove_entities(services) +
-    remove_entities(waitables);
-}
-
 void
 build_entities_collection(
   const std::vector<rclcpp::CallbackGroup::WeakPtr> & callback_groups,
@@ -246,7 +222,9 @@ ready_executables(
     executables.push_back(exec);
     added++;
   }
+
   return added;
 }
+
 }  // namespace executors
 }  // namespace rclcpp

rclcpp/src/rclcpp/executors/executor_entities_collector.cpp

@@ -105,8 +105,7 @@ void
 ExecutorEntitiesCollector::remove_node(
   rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr)
 {
-  std::atomic_bool & has_executor = node_ptr->get_associated_with_executor_atomic();
-  if (!has_executor.exchange(false)) {
+  if (!node_ptr->get_associated_with_executor_atomic().load()) {
     throw std::runtime_error(
             std::string("Node '") + node_ptr->get_fully_qualified_name() +
             "' needs to be associated with an executor.");
@@ -162,6 +161,7 @@ ExecutorEntitiesCollector::remove_callback_group(rclcpp::CallbackGroup::SharedPt
     throw std::runtime_error("Node must not be deleted before its callback group(s).");
   }
   */
+
   auto weak_group_ptr = rclcpp::CallbackGroup::WeakPtr(group_ptr);
   std::lock_guard<std::mutex> lock(mutex_);
   bool associated = manually_added_groups_.count(group_ptr) != 0;
@@ -314,11 +314,7 @@ ExecutorEntitiesCollector::process_queues()
     if (node_it != weak_nodes_.end()) {
       remove_weak_node(node_it);
     } else {
-      // The node may have been destroyed and removed from the colletion before
-      // we processed the queues.  Don't throw if the pointer is already expired.
-      if (!weak_node_ptr.expired()) {
-        throw std::runtime_error("Node needs to be associated with this executor.");
-      }
+      throw std::runtime_error("Node needs to be associated with this executor.");
     }
 
     auto node_ptr = weak_node_ptr.lock();

rclcpp/src/rclcpp/executors/executor_notify_waitable.cpp

@@ -46,18 +46,20 @@ void
 ExecutorNotifyWaitable::add_to_wait_set(rcl_wait_set_t * wait_set)
 {
   std::lock_guard<std::mutex> lock(guard_condition_mutex_);
+
   for (auto weak_guard_condition : this->notify_guard_conditions_) {
     auto guard_condition = weak_guard_condition.lock();
-    if (!guard_condition) {continue;}
+    if (guard_condition) {
+      auto rcl_guard_condition = &guard_condition->get_rcl_guard_condition();
 
-    rcl_guard_condition_t * cond = &guard_condition->get_rcl_guard_condition();
+      rcl_ret_t ret = rcl_wait_set_add_guard_condition(
+        wait_set,
+        rcl_guard_condition, NULL);
 
-    rcl_ret_t ret = rcl_wait_set_add_guard_condition(
-      wait_set, cond, NULL);
-
-    if (RCL_RET_OK != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(
-        ret, "failed to add guard condition to wait set");
+      if (RCL_RET_OK != ret) {
+        rclcpp::exceptions::throw_from_rcl_error(
+          ret, "failed to add guard condition to wait set");
+      }
     }
   }
 }

rclcpp/src/rclcpp/executors/multi_threaded_executor.cpp

@@ -99,19 +99,6 @@ MultiThreadedExecutor::run(size_t this_thread_number)
 
     execute_any_executable(any_exec);
 
-    if (any_exec.callback_group &&
-      any_exec.callback_group->type() == CallbackGroupType::MutuallyExclusive &&
-      any_exec.callback_group->size() > 1)
-    {
-      try {
-        interrupt_guard_condition_->trigger();
-      } catch (const rclcpp::exceptions::RCLError & ex) {
-        throw std::runtime_error(
-                std::string(
-                  "Failed to trigger guard condition on callback group change: ") + ex.what());
-      }
-    }
-
     // Clear the callback_group to prevent the AnyExecutable destructor from
     // resetting the callback group `can_be_taken_from`
     any_exec.callback_group.reset();

rclcpp/src/rclcpp/executors/static_executor_entities_collector.cpp

@@ -0,0 +1,524 @@
+// Copyright 2020 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "rclcpp/executors/static_executor_entities_collector.hpp"
+
+#include <algorithm>
+#include <memory>
+#include <stdexcept>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "rclcpp/memory_strategy.hpp"
+#include "rclcpp/executors/static_single_threaded_executor.hpp"
+#include "rclcpp/detail/add_guard_condition_to_rcl_wait_set.hpp"
+
+using rclcpp::executors::StaticExecutorEntitiesCollector;
+
+StaticExecutorEntitiesCollector::~StaticExecutorEntitiesCollector()
+{
+  // Disassociate all callback groups and thus nodes.
+  for (const auto & pair : weak_groups_associated_with_executor_to_nodes_) {
+    auto group = pair.first.lock();
+    if (group) {
+      std::atomic_bool & has_executor = group->get_associated_with_executor_atomic();
+      has_executor.store(false);
+    }
+  }
+  for (const auto & pair : weak_groups_to_nodes_associated_with_executor_) {
+    auto group = pair.first.lock();
+    if (group) {
+      std::atomic_bool & has_executor = group->get_associated_with_executor_atomic();
+      has_executor.store(false);
+    }
+  }
+  // Disassociate all nodes
+  for (const auto & weak_node : weak_nodes_) {
+    auto node = weak_node.lock();
+    if (node) {
+      std::atomic_bool & has_executor = node->get_associated_with_executor_atomic();
+      has_executor.store(false);
+    }
+  }
+  weak_groups_associated_with_executor_to_nodes_.clear();
+  weak_groups_to_nodes_associated_with_executor_.clear();
+  exec_list_.clear();
+  weak_nodes_.clear();
+  weak_nodes_to_guard_conditions_.clear();
+}
+
+void
+StaticExecutorEntitiesCollector::init(
+  rcl_wait_set_t * p_wait_set,
+  rclcpp::memory_strategy::MemoryStrategy::SharedPtr memory_strategy)
+{
+  // Empty initialize executable list
+  exec_list_ = rclcpp::experimental::ExecutableList();
+  // Get executor's wait_set_ pointer
+  p_wait_set_ = p_wait_set;
+  // Get executor's memory strategy ptr
+  if (memory_strategy == nullptr) {
+    throw std::runtime_error("Received NULL memory strategy in executor waitable.");
+  }
+  memory_strategy_ = memory_strategy;
+
+  // Get memory strategy and executable list. Prepare wait_set_
+  std::shared_ptr<void> shared_ptr;
+  execute(shared_ptr);
+
+  // The entities collector is now initialized
+  initialized_ = true;
+}
+
+void
+StaticExecutorEntitiesCollector::fini()
+{
+  memory_strategy_->clear_handles();
+  exec_list_.clear();
+}
+
+std::shared_ptr<void>
+StaticExecutorEntitiesCollector::take_data()
+{
+  return nullptr;
+}
+
+void
+StaticExecutorEntitiesCollector::execute(std::shared_ptr<void> & data)
+{
+  (void) data;
+  // Fill memory strategy with entities coming from weak_nodes_
+  fill_memory_strategy();
+  // Fill exec_list_ with entities coming from weak_nodes_ (same as memory strategy)
+  fill_executable_list();
+  // Resize the wait_set_ based on memory_strategy handles (rcl_wait_set_resize)
+  prepare_wait_set();
+  // Add new nodes guard conditions to map
+  std::lock_guard<std::mutex> guard{new_nodes_mutex_};
+  for (const auto & weak_node : new_nodes_) {
+    if (auto node_ptr = weak_node.lock()) {
+      weak_nodes_to_guard_conditions_[node_ptr] =
+        node_ptr->get_shared_notify_guard_condition().get();
+    }
+  }
+  new_nodes_.clear();
+}
+
+void
+StaticExecutorEntitiesCollector::fill_memory_strategy()
+{
+  memory_strategy_->clear_handles();
+  bool has_invalid_weak_groups_or_nodes =
+    memory_strategy_->collect_entities(weak_groups_to_nodes_associated_with_executor_);
+  // Clean up any invalid nodes, if they were detected
+  if (has_invalid_weak_groups_or_nodes) {
+    std::vector<rclcpp::CallbackGroup::WeakPtr> invalid_group_ptrs;
+    for (const auto & pair : weak_groups_to_nodes_associated_with_executor_) {
+      auto & weak_group_ptr = pair.first;
+      auto & weak_node_ptr = pair.second;
+      if (weak_group_ptr.expired() || weak_node_ptr.expired()) {
+        invalid_group_ptrs.push_back(weak_group_ptr);
+      }
+    }
+    std::for_each(
+      invalid_group_ptrs.begin(), invalid_group_ptrs.end(),
+      [this](rclcpp::CallbackGroup::WeakPtr group_ptr) {
+        weak_groups_to_nodes_associated_with_executor_.erase(group_ptr);
+      });
+  }
+  has_invalid_weak_groups_or_nodes =
+    memory_strategy_->collect_entities(weak_groups_associated_with_executor_to_nodes_);
+  // Clean up any invalid nodes, if they were detected
+  if (has_invalid_weak_groups_or_nodes) {
+    std::vector<rclcpp::CallbackGroup::WeakPtr> invalid_group_ptrs;
+    for (const auto & pair : weak_groups_associated_with_executor_to_nodes_) {
+      auto & weak_group_ptr = pair.first;
+      const auto & weak_node_ptr = pair.second;
+      if (weak_group_ptr.expired() || weak_node_ptr.expired()) {
+        invalid_group_ptrs.push_back(weak_group_ptr);
+      }
+    }
+    std::for_each(
+      invalid_group_ptrs.begin(), invalid_group_ptrs.end(),
+      [this](rclcpp::CallbackGroup::WeakPtr group_ptr) {
+        weak_groups_associated_with_executor_to_nodes_.erase(group_ptr);
+      });
+  }
+
+  // Add the static executor waitable to the memory strategy
+  memory_strategy_->add_waitable_handle(this->shared_from_this());
+}
+
+void
+StaticExecutorEntitiesCollector::fill_executable_list()
+{
+  exec_list_.clear();
+  add_callback_groups_from_nodes_associated_to_executor();
+  fill_executable_list_from_map(weak_groups_associated_with_executor_to_nodes_);
+  fill_executable_list_from_map(weak_groups_to_nodes_associated_with_executor_);
+  // Add the executor's waitable to the executable list
+  exec_list_.add_waitable(shared_from_this());
+}
+void
+StaticExecutorEntitiesCollector::fill_executable_list_from_map(
+  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
+  weak_groups_to_nodes)
+{
+  for (const auto & pair : weak_groups_to_nodes) {
+    auto group = pair.first.lock();
+    auto node = pair.second.lock();
+    if (!node || !group || !group->can_be_taken_from().load()) {
+      continue;
+    }
+    group->find_timer_ptrs_if(
+      [this](const rclcpp::TimerBase::SharedPtr & timer) {
+        if (timer) {
+          exec_list_.add_timer(timer);
+        }
+        return false;
+      });
+    group->find_subscription_ptrs_if(
+      [this](const rclcpp::SubscriptionBase::SharedPtr & subscription) {
+        if (subscription) {
+          exec_list_.add_subscription(subscription);
+        }
+        return false;
+      });
+    group->find_service_ptrs_if(
+      [this](const rclcpp::ServiceBase::SharedPtr & service) {
+        if (service) {
+          exec_list_.add_service(service);
+        }
+        return false;
+      });
+    group->find_client_ptrs_if(
+      [this](const rclcpp::ClientBase::SharedPtr & client) {
+        if (client) {
+          exec_list_.add_client(client);
+        }
+        return false;
+      });
+    group->find_waitable_ptrs_if(
+      [this](const rclcpp::Waitable::SharedPtr & waitable) {
+        if (waitable) {
+          exec_list_.add_waitable(waitable);
+        }
+        return false;
+      });
+  }
+}
+
+void
+StaticExecutorEntitiesCollector::prepare_wait_set()
+{
+  // clear wait set
+  if (rcl_wait_set_clear(p_wait_set_) != RCL_RET_OK) {
+    throw std::runtime_error("Couldn't clear wait set");
+  }
+
+  // The size of waitables are accounted for in size of the other entities
+  rcl_ret_t ret = rcl_wait_set_resize(
+    p_wait_set_, memory_strategy_->number_of_ready_subscriptions(),
+    memory_strategy_->number_of_guard_conditions(), memory_strategy_->number_of_ready_timers(),
+    memory_strategy_->number_of_ready_clients(), memory_strategy_->number_of_ready_services(),
+    memory_strategy_->number_of_ready_events());
+
+  if (RCL_RET_OK != ret) {
+    throw std::runtime_error(
+            std::string("Couldn't resize the wait set: ") + rcl_get_error_string().str);
+  }
+}
+
+void
+StaticExecutorEntitiesCollector::refresh_wait_set(std::chrono::nanoseconds timeout)
+{
+  // clear wait set (memset to '0' all wait_set_ entities
+  // but keeps the wait_set_ number of entities)
+  if (rcl_wait_set_clear(p_wait_set_) != RCL_RET_OK) {
+    throw std::runtime_error("Couldn't clear wait set");
+  }
+
+  if (!memory_strategy_->add_handles_to_wait_set(p_wait_set_)) {
+    throw std::runtime_error("Couldn't fill wait set");
+  }
+
+  rcl_ret_t status =
+    rcl_wait(p_wait_set_, std::chrono::duration_cast<std::chrono::nanoseconds>(timeout).count());
+
+  if (status == RCL_RET_WAIT_SET_EMPTY) {
+    RCUTILS_LOG_WARN_NAMED(
+      "rclcpp",
+      "empty wait set received in rcl_wait(). This should never happen.");
+  } else if (status != RCL_RET_OK && status != RCL_RET_TIMEOUT) {
+    using rclcpp::exceptions::throw_from_rcl_error;
+    throw_from_rcl_error(status, "rcl_wait() failed");
+  }
+}
+
+void
+StaticExecutorEntitiesCollector::add_to_wait_set(rcl_wait_set_t * wait_set)
+{
+  // Add waitable guard conditions (one for each registered node) into the wait set.
+  for (const auto & pair : weak_nodes_to_guard_conditions_) {
+    auto & gc = pair.second;
+    detail::add_guard_condition_to_rcl_wait_set(*wait_set, *gc);
+  }
+}
+
+size_t StaticExecutorEntitiesCollector::get_number_of_ready_guard_conditions()
+{
+  std::lock_guard<std::mutex> guard{new_nodes_mutex_};
+  return weak_nodes_to_guard_conditions_.size() + new_nodes_.size();
+}
+
+bool
+StaticExecutorEntitiesCollector::add_node(
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr)
+{
+  bool is_new_node = false;
+  // If the node already has an executor
+  std::atomic_bool & has_executor = node_ptr->get_associated_with_executor_atomic();
+  if (has_executor.exchange(true)) {
+    throw std::runtime_error("Node has already been added to an executor.");
+  }
+  node_ptr->for_each_callback_group(
+    [this, node_ptr, &is_new_node](rclcpp::CallbackGroup::SharedPtr group_ptr)
+    {
+      if (
+        !group_ptr->get_associated_with_executor_atomic().load() &&
+        group_ptr->automatically_add_to_executor_with_node())
+      {
+        is_new_node = (add_callback_group(
+          group_ptr,
+          node_ptr,
+          weak_groups_to_nodes_associated_with_executor_) ||
+        is_new_node);
+      }
+    });
+  weak_nodes_.push_back(node_ptr);
+  return is_new_node;
+}
+
+bool
+StaticExecutorEntitiesCollector::add_callback_group(
+  rclcpp::CallbackGroup::SharedPtr group_ptr,
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+  rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap & weak_groups_to_nodes)
+{
+  // If the callback_group already has an executor
+  std::atomic_bool & has_executor = group_ptr->get_associated_with_executor_atomic();
+  if (has_executor.exchange(true)) {
+    throw std::runtime_error("Callback group has already been added to an executor.");
+  }
+  bool is_new_node = !has_node(node_ptr, weak_groups_associated_with_executor_to_nodes_) &&
+    !has_node(node_ptr, weak_groups_to_nodes_associated_with_executor_);
+  rclcpp::CallbackGroup::WeakPtr weak_group_ptr = group_ptr;
+  auto insert_info = weak_groups_to_nodes.insert(
+    std::make_pair(weak_group_ptr, node_ptr));
+  bool was_inserted = insert_info.second;
+  if (!was_inserted) {
+    throw std::runtime_error("Callback group was already added to executor.");
+  }
+  if (is_new_node) {
+    std::lock_guard<std::mutex> guard{new_nodes_mutex_};
+    new_nodes_.push_back(node_ptr);
+    return true;
+  }
+  return false;
+}
+
+bool
+StaticExecutorEntitiesCollector::add_callback_group(
+  rclcpp::CallbackGroup::SharedPtr group_ptr,
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr)
+{
+  return add_callback_group(group_ptr, node_ptr, weak_groups_associated_with_executor_to_nodes_);
+}
+
+bool
+StaticExecutorEntitiesCollector::remove_callback_group(
+  rclcpp::CallbackGroup::SharedPtr group_ptr)
+{
+  return this->remove_callback_group_from_map(
+    group_ptr,
+    weak_groups_associated_with_executor_to_nodes_);
+}
+
+bool
+StaticExecutorEntitiesCollector::remove_callback_group_from_map(
+  rclcpp::CallbackGroup::SharedPtr group_ptr,
+  rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap & weak_groups_to_nodes)
+{
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr;
+  rclcpp::CallbackGroup::WeakPtr weak_group_ptr = group_ptr;
+  auto iter = weak_groups_to_nodes.find(weak_group_ptr);
+  if (iter != weak_groups_to_nodes.end()) {
+    node_ptr = iter->second.lock();
+    if (node_ptr == nullptr) {
+      throw std::runtime_error("Node must not be deleted before its callback group(s).");
+    }
+    weak_groups_to_nodes.erase(iter);
+  } else {
+    throw std::runtime_error("Callback group needs to be associated with executor.");
+  }
+  // If the node was matched and removed, interrupt waiting.
+  if (!has_node(node_ptr, weak_groups_associated_with_executor_to_nodes_) &&
+    !has_node(node_ptr, weak_groups_to_nodes_associated_with_executor_))
+  {
+    rclcpp::node_interfaces::NodeBaseInterface::WeakPtr node_weak_ptr(node_ptr);
+    weak_nodes_to_guard_conditions_.erase(node_weak_ptr);
+    return true;
+  }
+  return false;
+}
+
+bool
+StaticExecutorEntitiesCollector::remove_node(
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr)
+{
+  if (!node_ptr->get_associated_with_executor_atomic().load()) {
+    return false;
+  }
+  bool node_found = false;
+  auto node_it = weak_nodes_.begin();
+  while (node_it != weak_nodes_.end()) {
+    bool matched = (node_it->lock() == node_ptr);
+    if (matched) {
+      weak_nodes_.erase(node_it);
+      node_found = true;
+      break;
+    }
+    ++node_it;
+  }
+  if (!node_found) {
+    return false;
+  }
+  std::vector<rclcpp::CallbackGroup::SharedPtr> found_group_ptrs;
+  std::for_each(
+    weak_groups_to_nodes_associated_with_executor_.begin(),
+    weak_groups_to_nodes_associated_with_executor_.end(),
+    [&found_group_ptrs, node_ptr](std::pair<rclcpp::CallbackGroup::WeakPtr,
+    rclcpp::node_interfaces::NodeBaseInterface::WeakPtr> key_value_pair) {
+      auto & weak_node_ptr = key_value_pair.second;
+      auto shared_node_ptr = weak_node_ptr.lock();
+      auto group_ptr = key_value_pair.first.lock();
+      if (shared_node_ptr == node_ptr) {
+        found_group_ptrs.push_back(group_ptr);
+      }
+    });
+  std::for_each(
+    found_group_ptrs.begin(), found_group_ptrs.end(), [this]
+      (rclcpp::CallbackGroup::SharedPtr group_ptr) {
+      this->remove_callback_group_from_map(
+        group_ptr,
+        weak_groups_to_nodes_associated_with_executor_);
+    });
+  std::atomic_bool & has_executor = node_ptr->get_associated_with_executor_atomic();
+  has_executor.store(false);
+  return true;
+}
+
+bool
+StaticExecutorEntitiesCollector::is_ready(rcl_wait_set_t * p_wait_set)
+{
+  // Check wait_set guard_conditions for added/removed entities to/from a node
+  for (size_t i = 0; i < p_wait_set->size_of_guard_conditions; ++i) {
+    if (p_wait_set->guard_conditions[i] != NULL) {
+      auto found_guard_condition = std::find_if(
+        weak_nodes_to_guard_conditions_.begin(), weak_nodes_to_guard_conditions_.end(),
+        [&](std::pair<rclcpp::node_interfaces::NodeBaseInterface::WeakPtr,
+        const GuardCondition *> pair) -> bool {
+          const rcl_guard_condition_t & rcl_gc = pair.second->get_rcl_guard_condition();
+          return &rcl_gc == p_wait_set->guard_conditions[i];
+        });
+      if (found_guard_condition != weak_nodes_to_guard_conditions_.end()) {
+        return true;
+      }
+    }
+  }
+  // None of the guard conditions triggered belong to a registered node
+  return false;
+}
+
+// Returns true iff the weak_groups_to_nodes map has node_ptr as the value in any of its entry.
+bool
+StaticExecutorEntitiesCollector::has_node(
+  const rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+  const rclcpp::memory_strategy::MemoryStrategy::WeakCallbackGroupsToNodesMap &
+  weak_groups_to_nodes) const
+{
+  return std::find_if(
+    weak_groups_to_nodes.begin(),
+    weak_groups_to_nodes.end(),
+    [&](const WeakCallbackGroupsToNodesMap::value_type & other) -> bool {
+      auto other_ptr = other.second.lock();
+      return other_ptr == node_ptr;
+    }) != weak_groups_to_nodes.end();
+}
+
+void
+StaticExecutorEntitiesCollector::add_callback_groups_from_nodes_associated_to_executor()
+{
+  for (const auto & weak_node : weak_nodes_) {
+    auto node = weak_node.lock();
+    if (node) {
+      node->for_each_callback_group(
+        [this, node](rclcpp::CallbackGroup::SharedPtr shared_group_ptr)
+        {
+          if (shared_group_ptr->automatically_add_to_executor_with_node() &&
+          !shared_group_ptr->get_associated_with_executor_atomic().load())
+          {
+            add_callback_group(
+              shared_group_ptr,
+              node,
+              weak_groups_to_nodes_associated_with_executor_);
+          }
+        });
+    }
+  }
+}
+
+std::vector<rclcpp::CallbackGroup::WeakPtr>
+StaticExecutorEntitiesCollector::get_all_callback_groups()
+{
+  std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
+  for (const auto & group_node_ptr : weak_groups_associated_with_executor_to_nodes_) {
+    groups.push_back(group_node_ptr.first);
+  }
+  for (const auto & group_node_ptr : weak_groups_to_nodes_associated_with_executor_) {
+    groups.push_back(group_node_ptr.first);
+  }
+  return groups;
+}
+
+std::vector<rclcpp::CallbackGroup::WeakPtr>
+StaticExecutorEntitiesCollector::get_manually_added_callback_groups()
+{
+  std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
+  for (const auto & group_node_ptr : weak_groups_associated_with_executor_to_nodes_) {
+    groups.push_back(group_node_ptr.first);
+  }
+  return groups;
+}
+
+std::vector<rclcpp::CallbackGroup::WeakPtr>
+StaticExecutorEntitiesCollector::get_automatically_added_callback_groups_from_nodes()
+{
+  std::vector<rclcpp::CallbackGroup::WeakPtr> groups;
+  for (const auto & group_node_ptr : weak_groups_to_nodes_associated_with_executor_) {
+    groups.push_back(group_node_ptr.first);
+  }
+  return groups;
+}

rclcpp/src/rclcpp/executors/static_single_threaded_executor.cpp

@@ -12,21 +12,31 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "rclcpp/executors/executor_entities_collection.hpp"
-#include "rclcpp/executors/executor_notify_waitable.hpp"
+#include "rclcpp/executors/static_single_threaded_executor.hpp"
+
+#include <chrono>
+#include <memory>
+#include <utility>
+#include <vector>
+
 #include "rcpputils/scope_exit.hpp"
 
-#include "rclcpp/executors/static_single_threaded_executor.hpp"
-#include "rclcpp/any_executable.hpp"
-
 using rclcpp::executors::StaticSingleThreadedExecutor;
+using rclcpp::experimental::ExecutableList;
 
-StaticSingleThreadedExecutor::StaticSingleThreadedExecutor(const rclcpp::ExecutorOptions & options)
+StaticSingleThreadedExecutor::StaticSingleThreadedExecutor(
+  const rclcpp::ExecutorOptions & options)
 : rclcpp::Executor(options)
 {
+  entities_collector_ = std::make_shared<StaticExecutorEntitiesCollector>();
 }
 
-StaticSingleThreadedExecutor::~StaticSingleThreadedExecutor() {}
+StaticSingleThreadedExecutor::~StaticSingleThreadedExecutor()
+{
+  if (entities_collector_->is_init()) {
+    entities_collector_->fini();
+  }
+}
 
 void
 StaticSingleThreadedExecutor::spin()
@@ -36,25 +46,14 @@ StaticSingleThreadedExecutor::spin()
   }
   RCPPUTILS_SCOPE_EXIT(this->spinning.store(false); );
 
-  // This is essentially the contents of the rclcpp::Executor::wait_for_work method,
-  // except we need to keep the wait result to reproduce the StaticSingleThreadedExecutor
-  // behavior.
+  // Set memory_strategy_ and exec_list_ based on weak_nodes_
+  // Prepare wait_set_ based on memory_strategy_
+  entities_collector_->init(&wait_set_, memory_strategy_);
+
   while (rclcpp::ok(this->context_) && spinning.load()) {
-    std::deque<rclcpp::AnyExecutable> to_exec;
-
-    std::lock_guard<std::mutex> guard(mutex_);
-    if (current_collection_.empty() || this->entities_need_rebuild_.load()) {
-      this->collect_entities();
-    }
-
-    auto wait_result = wait_set_.wait(std::chrono::nanoseconds(-1));
-    if (wait_result.kind() == WaitResultKind::Empty) {
-      RCUTILS_LOG_WARN_NAMED(
-        "rclcpp",
-        "empty wait set received in wait(). This should never happen.");
-      continue;
-    }
-    execute_ready_executables(current_collection_, wait_result, false);
+    // Refresh wait set and wait for work
+    entities_collector_->refresh_wait_set();
+    execute_ready_executables();
   }
 }
 
@@ -81,6 +80,11 @@ StaticSingleThreadedExecutor::spin_all(std::chrono::nanoseconds max_duration)
 void
 StaticSingleThreadedExecutor::spin_some_impl(std::chrono::nanoseconds max_duration, bool exhaustive)
 {
+  // Make sure the entities collector has been initialized
+  if (!entities_collector_->is_init()) {
+    entities_collector_->init(&wait_set_, memory_strategy_);
+  }
+
   auto start = std::chrono::steady_clock::now();
   auto max_duration_not_elapsed = [max_duration, start]() {
       if (std::chrono::nanoseconds(0) == max_duration) {
@@ -101,21 +105,9 @@ StaticSingleThreadedExecutor::spin_some_impl(std::chrono::nanoseconds max_durati
 
   while (rclcpp::ok(context_) && spinning.load() && max_duration_not_elapsed()) {
     // Get executables that are ready now
-    std::lock_guard<std::mutex> guard(mutex_);
-    if (current_collection_.empty() || this->entities_need_rebuild_.load()) {
-      this->collect_entities();
-    }
-
-    auto wait_result = wait_set_.wait(std::chrono::nanoseconds(0));
-    if (wait_result.kind() == WaitResultKind::Empty) {
-      RCUTILS_LOG_WARN_NAMED(
-        "rclcpp",
-        "empty wait set received in wait(). This should never happen.");
-      continue;
-    }
-
+    entities_collector_->refresh_wait_set(std::chrono::milliseconds::zero());
     // Execute ready executables
-    bool work_available = execute_ready_executables(current_collection_, wait_result, false);
+    bool work_available = execute_ready_executables();
     if (!work_available || !exhaustive) {
       break;
     }
@@ -125,122 +117,164 @@ StaticSingleThreadedExecutor::spin_some_impl(std::chrono::nanoseconds max_durati
 void
 StaticSingleThreadedExecutor::spin_once_impl(std::chrono::nanoseconds timeout)
 {
+  // Make sure the entities collector has been initialized
+  if (!entities_collector_->is_init()) {
+    entities_collector_->init(&wait_set_, memory_strategy_);
+  }
+
   if (rclcpp::ok(context_) && spinning.load()) {
-    std::lock_guard<std::mutex> guard(mutex_);
-    if (current_collection_.empty() || this->entities_need_rebuild_.load()) {
-      this->collect_entities();
-    }
-
-    auto wait_result = wait_set_.wait(std::chrono::nanoseconds(timeout));
-    if (wait_result.kind() == WaitResultKind::Empty) {
-      RCUTILS_LOG_WARN_NAMED(
-        "rclcpp",
-        "empty wait set received in wait(). This should never happen.");
-      return;
-    }
-
+    // Wait until we have a ready entity or timeout expired
+    entities_collector_->refresh_wait_set(timeout);
     // Execute ready executables
-    execute_ready_executables(current_collection_, wait_result, true);
+    execute_ready_executables(true);
   }
 }
 
-// This preserves the "scheduling semantics" of the StaticSingleThreadedExecutor
-// from the original implementation.
-bool StaticSingleThreadedExecutor::execute_ready_executables(
-  const rclcpp::executors::ExecutorEntitiesCollection & collection,
-  rclcpp::WaitResult<rclcpp::WaitSet> & wait_result,
-  bool spin_once)
+void
+StaticSingleThreadedExecutor::add_callback_group(
+  rclcpp::CallbackGroup::SharedPtr group_ptr,
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr,
+  bool notify)
+{
+  bool is_new_node = entities_collector_->add_callback_group(group_ptr, node_ptr);
+  if (is_new_node && notify) {
+    // Interrupt waiting to handle new node
+    interrupt_guard_condition_->trigger();
+  }
+}
+
+void
+StaticSingleThreadedExecutor::add_node(
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, bool notify)
+{
+  bool is_new_node = entities_collector_->add_node(node_ptr);
+  if (is_new_node && notify) {
+    // Interrupt waiting to handle new node
+    interrupt_guard_condition_->trigger();
+  }
+}
+
+void
+StaticSingleThreadedExecutor::add_node(std::shared_ptr<rclcpp::Node> node_ptr, bool notify)
+{
+  this->add_node(node_ptr->get_node_base_interface(), notify);
+}
+
+void
+StaticSingleThreadedExecutor::remove_callback_group(
+  rclcpp::CallbackGroup::SharedPtr group_ptr, bool notify)
+{
+  bool node_removed = entities_collector_->remove_callback_group(group_ptr);
+  // If the node was matched and removed, interrupt waiting
+  if (node_removed && notify) {
+    interrupt_guard_condition_->trigger();
+  }
+}
+
+void
+StaticSingleThreadedExecutor::remove_node(
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_ptr, bool notify)
+{
+  bool node_removed = entities_collector_->remove_node(node_ptr);
+  if (!node_removed) {
+    throw std::runtime_error("Node needs to be associated with this executor.");
+  }
+  // If the node was matched and removed, interrupt waiting
+  if (notify) {
+    interrupt_guard_condition_->trigger();
+  }
+}
+
+std::vector<rclcpp::CallbackGroup::WeakPtr>
+StaticSingleThreadedExecutor::get_all_callback_groups()
+{
+  return entities_collector_->get_all_callback_groups();
+}
+
+std::vector<rclcpp::CallbackGroup::WeakPtr>
+StaticSingleThreadedExecutor::get_manually_added_callback_groups()
+{
+  return entities_collector_->get_manually_added_callback_groups();
+}
+
+std::vector<rclcpp::CallbackGroup::WeakPtr>
+StaticSingleThreadedExecutor::get_automatically_added_callback_groups_from_nodes()
+{
+  return entities_collector_->get_automatically_added_callback_groups_from_nodes();
+}
+
+void
+StaticSingleThreadedExecutor::remove_node(std::shared_ptr<rclcpp::Node> node_ptr, bool notify)
+{
+  this->remove_node(node_ptr->get_node_base_interface(), notify);
+}
+
+bool
+StaticSingleThreadedExecutor::execute_ready_executables(bool spin_once)
 {
   bool any_ready_executable = false;
 
-  if (wait_result.kind() != rclcpp::WaitResultKind::Ready) {
-    return any_ready_executable;
+  // Execute all the ready subscriptions
+  for (size_t i = 0; i < wait_set_.size_of_subscriptions; ++i) {
+    if (i < entities_collector_->get_number_of_subscriptions()) {
+      if (wait_set_.subscriptions[i]) {
+        execute_subscription(entities_collector_->get_subscription(i));
+        if (spin_once) {
+          return true;
+        }
+        any_ready_executable = true;
+      }
+    }
   }
-
-  auto rcl_wait_set = wait_result.get_wait_set().get_rcl_wait_set();
-
-  for (size_t ii = 0; ii < rcl_wait_set.size_of_timers; ++ii) {
-    if (nullptr == rcl_wait_set.timers[ii]) {continue;}
-    auto entity_iter = collection.timers.find(rcl_wait_set.timers[ii]);
-    if (entity_iter != collection.timers.end()) {
-      auto entity = entity_iter->second.entity.lock();
-      if (!entity) {
-        continue;
+  // Execute all the ready timers
+  for (size_t i = 0; i < wait_set_.size_of_timers; ++i) {
+    if (i < entities_collector_->get_number_of_timers()) {
+      if (wait_set_.timers[i] && entities_collector_->get_timer(i)->is_ready()) {
+        auto timer = entities_collector_->get_timer(i);
+        timer->call();
+        execute_timer(std::move(timer));
+        if (spin_once) {
+          return true;
+        }
+        any_ready_executable = true;
       }
-      if (!entity->call()) {
-        continue;
+    }
+  }
+  // Execute all the ready services
+  for (size_t i = 0; i < wait_set_.size_of_services; ++i) {
+    if (i < entities_collector_->get_number_of_services()) {
+      if (wait_set_.services[i]) {
+        execute_service(entities_collector_->get_service(i));
+        if (spin_once) {
+          return true;
+        }
+        any_ready_executable = true;
       }
-      execute_timer(entity);
+    }
+  }
+  // Execute all the ready clients
+  for (size_t i = 0; i < wait_set_.size_of_clients; ++i) {
+    if (i < entities_collector_->get_number_of_clients()) {
+      if (wait_set_.clients[i]) {
+        execute_client(entities_collector_->get_client(i));
+        if (spin_once) {
+          return true;
+        }
+        any_ready_executable = true;
+      }
+    }
+  }
+  // Execute all the ready waitables
+  for (size_t i = 0; i < entities_collector_->get_number_of_waitables(); ++i) {
+    auto waitable = entities_collector_->get_waitable(i);
+    if (waitable->is_ready(&wait_set_)) {
+      auto data = waitable->take_data();
+      waitable->execute(data);
       if (spin_once) {
         return true;
       }
       any_ready_executable = true;
     }
   }
-
-  for (size_t ii = 0; ii < rcl_wait_set.size_of_subscriptions; ++ii) {
-    if (nullptr == rcl_wait_set.subscriptions[ii]) {continue;}
-    auto entity_iter = collection.subscriptions.find(rcl_wait_set.subscriptions[ii]);
-    if (entity_iter != collection.subscriptions.end()) {
-      auto entity = entity_iter->second.entity.lock();
-      if (!entity) {
-        continue;
-      }
-      execute_subscription(entity);
-      if (spin_once) {
-        return true;
-      }
-      any_ready_executable = true;
-    }
-  }
-
-  for (size_t ii = 0; ii < rcl_wait_set.size_of_services; ++ii) {
-    if (nullptr == rcl_wait_set.services[ii]) {continue;}
-    auto entity_iter = collection.services.find(rcl_wait_set.services[ii]);
-    if (entity_iter != collection.services.end()) {
-      auto entity = entity_iter->second.entity.lock();
-      if (!entity) {
-        continue;
-      }
-      execute_service(entity);
-      if (spin_once) {
-        return true;
-      }
-      any_ready_executable = true;
-    }
-  }
-
-  for (size_t ii = 0; ii < rcl_wait_set.size_of_clients; ++ii) {
-    if (nullptr == rcl_wait_set.clients[ii]) {continue;}
-    auto entity_iter = collection.clients.find(rcl_wait_set.clients[ii]);
-    if (entity_iter != collection.clients.end()) {
-      auto entity = entity_iter->second.entity.lock();
-      if (!entity) {
-        continue;
-      }
-      execute_client(entity);
-      if (spin_once) {
-        return true;
-      }
-      any_ready_executable = true;
-    }
-  }
-
-  for (auto & [handle, entry] : collection.waitables) {
-    auto waitable = entry.entity.lock();
-    if (!waitable) {
-      continue;
-    }
-    if (!waitable->is_ready(&rcl_wait_set)) {
-      continue;
-    }
-
-    auto data = waitable->take_data();
-    waitable->execute(data);
-    if (spin_once) {
-      return true;
-    }
-    any_ready_executable = true;
-  }
   return any_ready_executable;
 }

rclcpp/src/rclcpp/experimental/executors/events_executor/events_executor.cpp

@@ -273,13 +273,10 @@ EventsExecutor::execute_event(const ExecutorEvent & event)
   switch (event.type) {
     case ExecutorEventType::CLIENT_EVENT:
       {
-        rclcpp::ClientBase::SharedPtr client;
-        {
-          std::lock_guard<std::recursive_mutex> lock(collection_mutex_);
-          client = this->retrieve_entity(
-            static_cast<const rcl_client_t *>(event.entity_key),
-            current_entities_collection_->clients);
-        }
+        auto client = this->retrieve_entity(
+          static_cast<const rcl_client_t *>(event.entity_key),
+          current_entities_collection_->clients);
+
         if (client) {
           for (size_t i = 0; i < event.num_events; i++) {
             execute_client(client);
@@ -290,13 +287,9 @@ EventsExecutor::execute_event(const ExecutorEvent & event)
       }
     case ExecutorEventType::SUBSCRIPTION_EVENT:
       {
-        rclcpp::SubscriptionBase::SharedPtr subscription;
-        {
-          std::lock_guard<std::recursive_mutex> lock(collection_mutex_);
-          subscription = this->retrieve_entity(
-            static_cast<const rcl_subscription_t *>(event.entity_key),
-            current_entities_collection_->subscriptions);
-        }
+        auto subscription = this->retrieve_entity(
+          static_cast<const rcl_subscription_t *>(event.entity_key),
+          current_entities_collection_->subscriptions);
         if (subscription) {
           for (size_t i = 0; i < event.num_events; i++) {
             execute_subscription(subscription);
@@ -306,13 +299,10 @@ EventsExecutor::execute_event(const ExecutorEvent & event)
       }
     case ExecutorEventType::SERVICE_EVENT:
       {
-        rclcpp::ServiceBase::SharedPtr service;
-        {
-          std::lock_guard<std::recursive_mutex> lock(collection_mutex_);
-          service = this->retrieve_entity(
-            static_cast<const rcl_service_t *>(event.entity_key),
-            current_entities_collection_->services);
-        }
+        auto service = this->retrieve_entity(
+          static_cast<const rcl_service_t *>(event.entity_key),
+          current_entities_collection_->services);
+
         if (service) {
           for (size_t i = 0; i < event.num_events; i++) {
             execute_service(service);
@@ -329,13 +319,9 @@ EventsExecutor::execute_event(const ExecutorEvent & event)
       }
     case ExecutorEventType::WAITABLE_EVENT:
       {
-        rclcpp::Waitable::SharedPtr waitable;
-        {
-          std::lock_guard<std::recursive_mutex> lock(collection_mutex_);
-          waitable = this->retrieve_entity(
-            static_cast<const rclcpp::Waitable *>(event.entity_key),
-            current_entities_collection_->waitables);
-        }
+        auto waitable = this->retrieve_entity(
+          static_cast<const rclcpp::Waitable *>(event.entity_key),
+          current_entities_collection_->waitables);
         if (waitable) {
           for (size_t i = 0; i < event.num_events; i++) {
             auto data = waitable->take_data_by_entity_id(event.waitable_data);
@@ -400,7 +386,6 @@ EventsExecutor::get_automatically_added_callback_groups_from_nodes()
 void
 EventsExecutor::refresh_current_collection_from_callback_groups()
 {
-  // Build the new collection
   this->entities_collector_->update_collections();
   auto callback_groups = this->entities_collector_->get_all_callback_groups();
   rclcpp::executors::ExecutorEntitiesCollection new_collection;
@@ -415,9 +400,6 @@ EventsExecutor::refresh_current_collection_from_callback_groups()
   // To do it, we need to add the notify waitable as an entry in both the new and
   // current collections such that it's neither added or removed.
   this->add_notify_waitable_to_collection(new_collection.waitables);
-
-  // Acquire lock before modifying the current collection
-  std::lock_guard<std::recursive_mutex> lock(collection_mutex_);
   this->add_notify_waitable_to_collection(current_entities_collection_->waitables);
 
   this->refresh_current_collection(new_collection);
@@ -427,9 +409,6 @@ void
 EventsExecutor::refresh_current_collection(
   const rclcpp::executors::ExecutorEntitiesCollection & new_collection)
 {
-  // Acquire lock before modifying the current collection
-  std::lock_guard<std::recursive_mutex> lock(collection_mutex_);
-
   current_entities_collection_->timers.update(
     new_collection.timers,
     [this](rclcpp::TimerBase::SharedPtr timer) {timers_manager_->add_timer(timer);},

rclcpp/src/rclcpp/intra_process_manager.cpp

@@ -225,52 +225,5 @@ IntraProcessManager::can_communicate(
   return true;
 }
 
-size_t
-IntraProcessManager::lowest_available_capacity(const uint64_t intra_process_publisher_id) const
-{
-  size_t capacity = std::numeric_limits<size_t>::max();
-
-  auto publisher_it = pub_to_subs_.find(intra_process_publisher_id);
-  if (publisher_it == pub_to_subs_.end()) {
-    // Publisher is either invalid or no longer exists.
-    RCLCPP_WARN(
-      rclcpp::get_logger("rclcpp"),
-      "Calling lowest_available_capacity for invalid or no longer existing publisher id");
-    return 0u;
-  }
-
-  if (publisher_it->second.take_shared_subscriptions.empty() &&
-    publisher_it->second.take_ownership_subscriptions.empty())
-  {
-    // no subscriptions available
-    return 0u;
-  }
-
-  auto available_capacity = [this, &capacity](const uint64_t intra_process_subscription_id)
-    {
-      auto subscription_it = subscriptions_.find(intra_process_subscription_id);
-      if (subscription_it != subscriptions_.end()) {
-        auto subscription = subscription_it->second.lock();
-        if (subscription) {
-          capacity = std::min(capacity, subscription->available_capacity());
-        }
-      } else {
-        // Subscription is either invalid or no longer exists.
-        RCLCPP_WARN(
-          rclcpp::get_logger("rclcpp"),
-          "Calling available_capacity for invalid or no longer existing subscription id");
-      }
-    };
-
-  for (const auto sub_id : publisher_it->second.take_shared_subscriptions) {
-    available_capacity(sub_id);
-  }
-
-  for (const auto sub_id : publisher_it->second.take_ownership_subscriptions) {
-    available_capacity(sub_id);
-  }
-
-  return capacity;
-}
 }  // namespace experimental
 }  // namespace rclcpp

rclcpp/src/rclcpp/node.cpp

@@ -17,7 +17,10 @@
 #include <limits>
 #include <map>
 #include <memory>
+#include <mutex>
+#include <shared_mutex>
 #include <string>
+#include <unordered_map>
 #include <utility>
 #include <vector>
 
@@ -36,6 +39,7 @@
 #include "rclcpp/node_interfaces/node_time_source.hpp"
 #include "rclcpp/node_interfaces/node_timers.hpp"
 #include "rclcpp/node_interfaces/node_topics.hpp"
+#include "rclcpp/node_interfaces/node_type_descriptions.hpp"
 #include "rclcpp/node_interfaces/node_waitables.hpp"
 #include "rclcpp/qos_overriding_options.hpp"
 
@@ -109,6 +113,72 @@ create_effective_namespace(const std::string & node_namespace, const std::string
 
 }  // namespace
 
+/// \brief Associate new extra member variables with instances of Node without changing ABI.
+/**
+ * It is used only for bugfixes or backported features that require new members.
+ * Atomically constructs/destroys all extra members.
+ * Node instance will register and remove itself, and use its methods to retrieve members.
+ * Note for performance consideration that accessing these members uses a map lookup.
+ */
+class Node::BackportMembers
+{
+public:
+  BackportMembers() = default;
+  ~BackportMembers() = default;
+
+  /// \brief Add all backported members for a new Node.
+  /**
+   * \param[in] key Raw pointer to the Node instance that will use new members.
+   */
+  void add(Node * key)
+  {
+    // Adding a new instance to the maps requires exclusive access
+    std::unique_lock lock(map_access_mutex_);
+    type_descriptions_map_.emplace(
+      key,
+      std::make_shared<rclcpp::node_interfaces::NodeTypeDescriptions>(
+        key->get_node_base_interface(),
+        key->get_node_logging_interface(),
+        key->get_node_parameters_interface(),
+        key->get_node_services_interface()));
+  }
+
+  /// \brief Remove the members for an instance of Node
+  /**
+   * \param[in] key Raw pointer to the Node
+   */
+  void remove(const Node * key)
+  {
+    // Removing an instance from the maps requires exclusive access
+    std::unique_lock lock(map_access_mutex_);
+    type_descriptions_map_.erase(key);
+  }
+
+  /// \brief Retrieve the NodeTypeDescriptionsInterface for a Node.
+  /**
+   * \param[in] key Raw pointer to an instance of Node.
+   * \return A shared ptr to this Node's NodeTypeDescriptionsInterface instance.
+   */
+  rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
+  get_node_type_descriptions_interface(const Node * key) const
+  {
+    // Multiple threads can retrieve from the maps at the same time
+    std::shared_lock lock(map_access_mutex_);
+    return type_descriptions_map_.at(key);
+  }
+
+private:
+  /// \brief Map that stored TypeDescriptionsInterface members
+  std::unordered_map<
+    const Node *, rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
+  > type_descriptions_map_;
+
+  /// \brief Controls access to all private maps
+  mutable std::shared_mutex map_access_mutex_;
+};
+// Definition of static member declaration
+Node::BackportMembers Node::backport_members_;
+
 Node::Node(
   const std::string & node_name,
   const NodeOptions & options)
@@ -211,6 +281,8 @@ Node::Node(
   sub_namespace_(""),
   effective_namespace_(create_effective_namespace(this->get_namespace(), sub_namespace_))
 {
+  backport_members_.add(this);
+
   // we have got what we wanted directly from the overrides,
   // but declare the parameters anyway so they are visible.
   rclcpp::detail::declare_qos_parameters(
@@ -272,6 +344,7 @@ Node::Node(
 Node::~Node()
 {
   // release sub-interfaces in an order that allows them to consult with node_base during tear-down
+  backport_members_.remove(this);
   node_waitables_.reset();
   node_time_source_.reset();
   node_parameters_.reset();
@@ -591,6 +664,12 @@ Node::get_node_topics_interface()
   return node_topics_;
 }
 
+rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
+Node::get_node_type_descriptions_interface()
+{
+  return backport_members_.get_node_type_descriptions_interface(this);
+}
+
 rclcpp::node_interfaces::NodeServicesInterface::SharedPtr
 Node::get_node_services_interface()
 {

rclcpp/src/rclcpp/node_interfaces/node_timers.cpp

@@ -50,7 +50,7 @@ NodeTimers::add_timer(
             std::string("failed to notify wait set on timer creation: ") + ex.what());
   }
 
-  TRACETOOLS_TRACEPOINT(
+  TRACEPOINT(
     rclcpp_timer_link_node,
     static_cast<const void *>(timer->get_timer_handle().get()),
     static_cast<const void *>(node_base_->get_rcl_node_handle()));

rclcpp/src/rclcpp/node_interfaces/node_type_descriptions.cpp

@@ -0,0 +1,157 @@
+// Copyright 2023 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <memory>
+#include <string>
+#include <thread>
+
+#include "rclcpp/node_interfaces/node_type_descriptions.hpp"
+#include "rclcpp/parameter_client.hpp"
+
+#include "type_description_interfaces/srv/get_type_description.h"
+
+namespace
+{
+// Helper wrapper for rclcpp::Service to access ::Request and ::Response types for allocation.
+struct GetTypeDescription__C
+{
+  using Request = type_description_interfaces__srv__GetTypeDescription_Request;
+  using Response = type_description_interfaces__srv__GetTypeDescription_Response;
+  using Event = type_description_interfaces__srv__GetTypeDescription_Event;
+};
+}  // namespace
+
+// Helper function for C typesupport.
+namespace rosidl_typesupport_cpp
+{
+template<>
+rosidl_service_type_support_t const *
+get_service_type_support_handle<GetTypeDescription__C>()
+{
+  return ROSIDL_GET_SRV_TYPE_SUPPORT(type_description_interfaces, srv, GetTypeDescription);
+}
+}  // namespace rosidl_typesupport_cpp
+
+namespace rclcpp
+{
+namespace node_interfaces
+{
+
+class NodeTypeDescriptions::NodeTypeDescriptionsImpl
+{
+public:
+  using ServiceT = GetTypeDescription__C;
+
+  rclcpp::Logger logger_;
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base_;
+  rclcpp::Service<ServiceT>::SharedPtr type_description_srv_;
+
+  NodeTypeDescriptionsImpl(
+    rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base,
+    rclcpp::node_interfaces::NodeLoggingInterface::SharedPtr node_logging,
+    rclcpp::node_interfaces::NodeParametersInterface::SharedPtr node_parameters,
+    rclcpp::node_interfaces::NodeServicesInterface::SharedPtr node_services)
+  : logger_(node_logging->get_logger()),
+    node_base_(node_base)
+  {
+    const std::string enable_param_name = "start_type_description_service";
+
+    bool enabled = false;
+    try {
+      auto enable_param = node_parameters->declare_parameter(
+        enable_param_name,
+        rclcpp::ParameterValue(true),
+        rcl_interfaces::msg::ParameterDescriptor()
+        .set__name(enable_param_name)
+        .set__type(rclcpp::PARAMETER_BOOL)
+        .set__description("Start the ~/get_type_description service for this node.")
+        .set__read_only(true));
+      enabled = enable_param.get<bool>();
+    } catch (const rclcpp::exceptions::InvalidParameterTypeException & exc) {
+      RCLCPP_ERROR(logger_, "%s", exc.what());
+      throw;
+    }
+
+    if (enabled) {
+      auto rcl_node = node_base->get_rcl_node_handle();
+      rcl_ret_t rcl_ret = rcl_node_type_description_service_init(rcl_node);
+      if (rcl_ret != RCL_RET_OK) {
+        RCLCPP_ERROR(
+          logger_, "Failed to initialize ~/get_type_description_service: %s",
+          rcl_get_error_string().str);
+        throw std::runtime_error(
+                "Failed to initialize ~/get_type_description service.");
+      }
+
+      rcl_service_t * rcl_srv = nullptr;
+      rcl_ret = rcl_node_get_type_description_service(rcl_node, &rcl_srv);
+      if (rcl_ret != RCL_RET_OK) {
+        throw std::runtime_error(
+                "Failed to get initialized ~/get_type_description service from rcl.");
+      }
+
+      rclcpp::AnyServiceCallback<ServiceT> cb;
+      cb.set(
+        [this](
+          std::shared_ptr<rmw_request_id_t> header,
+          std::shared_ptr<ServiceT::Request> request,
+          std::shared_ptr<ServiceT::Response> response
+        ) {
+          rcl_node_type_description_service_handle_request(
+            node_base_->get_rcl_node_handle(),
+            header.get(),
+            request.get(),
+            response.get());
+        });
+
+      type_description_srv_ = std::make_shared<Service<ServiceT>>(
+        node_base_->get_shared_rcl_node_handle(),
+        rcl_srv,
+        cb);
+      node_services->add_service(
+        std::dynamic_pointer_cast<ServiceBase>(type_description_srv_),
+        nullptr);
+    }
+  }
+
+  ~NodeTypeDescriptionsImpl()
+  {
+    if (
+      type_description_srv_ &&
+      RCL_RET_OK != rcl_node_type_description_service_fini(node_base_->get_rcl_node_handle()))
+    {
+      RCLCPP_ERROR(
+        logger_,
+        "Error in shutdown of get_type_description service: %s", rcl_get_error_string().str);
+    }
+  }
+};
+
+NodeTypeDescriptions::NodeTypeDescriptions(
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr node_base,
+  rclcpp::node_interfaces::NodeLoggingInterface::SharedPtr node_logging,
+  rclcpp::node_interfaces::NodeParametersInterface::SharedPtr node_parameters,
+  rclcpp::node_interfaces::NodeServicesInterface::SharedPtr node_services)
+: impl_(new NodeTypeDescriptionsImpl(
+      node_base,
+      node_logging,
+      node_parameters,
+      node_services))
+{}
+
+NodeTypeDescriptions::~NodeTypeDescriptions()
+{}
+
+}  // namespace node_interfaces
+}  // namespace rclcpp

rclcpp/src/rclcpp/publisher_base.cpp

@@ -384,22 +384,3 @@ std::vector<rclcpp::NetworkFlowEndpoint> PublisherBase::get_network_flow_endpoin
 
   return network_flow_endpoint_vector;
 }
-
-size_t PublisherBase::lowest_available_ipm_capacity() const
-{
-  if (!intra_process_is_enabled_) {
-    return 0u;
-  }
-
-  auto ipm = weak_ipm_.lock();
-
-  if (!ipm) {
-    // TODO(ivanpauno): should this raise an error?
-    RCLCPP_WARN(
-      rclcpp::get_logger("rclcpp"),
-      "Intra process manager died for a publisher.");
-    return 0u;
-  }
-
-  return ipm->lowest_available_capacity(intra_process_publisher_id_);
-}

rclcpp/src/rclcpp/signal_handler.cpp

@@ -113,7 +113,7 @@ SignalHandler::get_logger()
 SignalHandler &
 SignalHandler::get_global_signal_handler()
 {
-  static SignalHandler signal_handler;
+  static SignalHandler & signal_handler = *new SignalHandler();
   return signal_handler;
 }
 

rclcpp/src/rclcpp/subscription_base.cpp

@@ -218,7 +218,7 @@ SubscriptionBase::take_type_erased(void * message_out, rclcpp::MessageInfo & mes
     &message_info_out.get_rmw_message_info(),
     nullptr  // rmw_subscription_allocation_t is unused here
   );
-  TRACETOOLS_TRACEPOINT(rclcpp_take, static_cast<const void *>(message_out));
+  TRACEPOINT(rclcpp_take, static_cast<const void *>(message_out));
   if (RCL_RET_SUBSCRIPTION_TAKE_FAILED == ret) {
     return false;
   } else if (RCL_RET_OK != ret) {
@@ -298,7 +298,20 @@ SubscriptionBase::setup_intra_process(
 bool
 SubscriptionBase::can_loan_messages() const
 {
-  return rcl_subscription_can_loan_messages(subscription_handle_.get());
+  bool retval = rcl_subscription_can_loan_messages(subscription_handle_.get());
+  if (retval) {
+    // TODO(clalancette): The loaned message interface is currently not safe to use with
+    // shared_ptr callbacks.  If a user takes a copy of the shared_ptr, it can get freed from
+    // underneath them via rcl_return_loaned_message_from_subscription().  The correct solution is
+    // to return the loaned message in a custom deleter, but that needs to be carefully handled
+    // with locking.  Warn the user about this until we fix it.
+    RCLCPP_WARN_ONCE(
+      this->node_logger_,
+      "Loaned messages are only safe with const ref subscription callbacks. "
+      "If you are using any other kind of subscriptions, "
+      "set the ROS_DISABLE_LOANED_MESSAGES environment variable to 1 (the default).");
+  }
+  return retval;
 }
 
 rclcpp::Waitable::SharedPtr

rclcpp/src/rclcpp/time_source.cpp

@@ -54,9 +54,7 @@ public:
     ros_time_active_ = true;
 
     // Update all attached clocks to zero or last recorded time
-    for (auto it = associated_clocks_.begin(); it != associated_clocks_.end(); ++it) {
-      set_clock(last_time_msg_, true, *it);
-    }
+    set_all_clocks(last_time_msg_, true);
   }
 
   // An internal method to use in the clock callback that iterates and disables all clocks
@@ -71,11 +69,8 @@ public:
     ros_time_active_ = false;
 
     // Update all attached clocks
-    std::lock_guard<std::mutex> guard(clock_list_lock_);
-    for (auto it = associated_clocks_.begin(); it != associated_clocks_.end(); ++it) {
-      auto msg = std::make_shared<builtin_interfaces::msg::Time>();
-      set_clock(msg, false, *it);
-    }
+    auto msg = std::make_shared<builtin_interfaces::msg::Time>();
+    set_all_clocks(msg, false);
   }
 
   // Check if ROS time is active

rclcpp/test/benchmark/benchmark_executor.cpp

@@ -362,3 +362,42 @@ BENCHMARK_F(PerformanceTestExecutorSimple, spin_until_future_complete)(benchmark
     }
   }
 }
+
+BENCHMARK_F(
+  PerformanceTestExecutorSimple,
+  static_executor_entities_collector_execute)(benchmark::State & st)
+{
+  rclcpp::executors::StaticExecutorEntitiesCollector::SharedPtr entities_collector_ =
+    std::make_shared<rclcpp::executors::StaticExecutorEntitiesCollector>();
+  entities_collector_->add_node(node->get_node_base_interface());
+
+  rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
+  rcl_allocator_t allocator = rcl_get_default_allocator();
+  auto shared_context = node->get_node_base_interface()->get_context();
+  rcl_context_t * context = shared_context->get_rcl_context().get();
+  rcl_ret_t ret = rcl_wait_set_init(&wait_set, 100, 100, 100, 100, 100, 100, context, allocator);
+  if (ret != RCL_RET_OK) {
+    st.SkipWithError(rcutils_get_error_string().str);
+  }
+  RCPPUTILS_SCOPE_EXIT(
+  {
+    rcl_ret_t ret = rcl_wait_set_fini(&wait_set);
+    if (ret != RCL_RET_OK) {
+      st.SkipWithError(rcutils_get_error_string().str);
+    }
+  });
+
+  auto memory_strategy = rclcpp::memory_strategies::create_default_strategy();
+  rclcpp::GuardCondition guard_condition(shared_context);
+
+  entities_collector_->init(&wait_set, memory_strategy);
+  RCPPUTILS_SCOPE_EXIT(entities_collector_->fini());
+
+  reset_heap_counters();
+
+  for (auto _ : st) {
+    (void)_;
+    std::shared_ptr<void> data = entities_collector_->take_data();
+    entities_collector_->execute(data);
+  }
+}

rclcpp/test/rclcpp/CMakeLists.txt

@@ -262,6 +262,11 @@ if(TARGET test_node_interfaces__node_topics)
     "test_msgs")
   target_link_libraries(test_node_interfaces__node_topics ${PROJECT_NAME} mimick)
 endif()
+ament_add_gtest(test_node_interfaces__node_type_descriptions
+  node_interfaces/test_node_type_descriptions.cpp)
+if(TARGET test_node_interfaces__node_type_descriptions)
+  target_link_libraries(test_node_interfaces__node_type_descriptions ${PROJECT_NAME} mimick)
+endif()
 ament_add_gtest(test_node_interfaces__node_waitables
   node_interfaces/test_node_waitables.cpp)
 if(TARGET test_node_interfaces__node_waitables)
@@ -678,6 +683,14 @@ if(TARGET test_executors)
   target_link_libraries(test_executors ${PROJECT_NAME})
 endif()
 
+ament_add_gtest(test_static_single_threaded_executor executors/test_static_single_threaded_executor.cpp
+  APPEND_LIBRARY_DIRS "${append_library_dirs}")
+if(TARGET test_static_single_threaded_executor)
+  ament_target_dependencies(test_static_single_threaded_executor
+    "test_msgs")
+  target_link_libraries(test_static_single_threaded_executor ${PROJECT_NAME} mimick)
+endif()
+
 ament_add_gtest(test_multi_threaded_executor executors/test_multi_threaded_executor.cpp
   APPEND_LIBRARY_DIRS "${append_library_dirs}")
 if(TARGET test_multi_threaded_executor)
@@ -686,6 +699,15 @@ if(TARGET test_multi_threaded_executor)
   target_link_libraries(test_multi_threaded_executor ${PROJECT_NAME})
 endif()
 
+ament_add_gtest(test_static_executor_entities_collector executors/test_static_executor_entities_collector.cpp
+  APPEND_LIBRARY_DIRS "${append_library_dirs}" TIMEOUT 120)
+if(TARGET test_static_executor_entities_collector)
+  ament_target_dependencies(test_static_executor_entities_collector
+    "rcl"
+    "test_msgs")
+  target_link_libraries(test_static_executor_entities_collector ${PROJECT_NAME} mimick)
+endif()
+
 ament_add_gtest(test_entities_collector executors/test_entities_collector.cpp
   APPEND_LIBRARY_DIRS "${append_library_dirs}" TIMEOUT 120)
 if(TARGET test_entities_collector)

rclcpp/test/rclcpp/executors/test_executors.cpp

@@ -22,6 +22,7 @@
 #include <algorithm>
 #include <atomic>
 #include <chrono>
+#include <future>
 #include <limits>
 #include <memory>
 #include <string>
@@ -79,6 +80,8 @@ public:
   int callback_count;
 };
 
+// spin_all and spin_some are not implemented correctly in StaticSingleThreadedExecutor, see:
+// https://github.com/ros2/rclcpp/issues/1219 for tracking
 template<typename T>
 class TestExecutorsStable : public TestExecutors<T> {};
 
@@ -120,6 +123,15 @@ public:
 // is updated.
 TYPED_TEST_SUITE(TestExecutors, ExecutorTypes, ExecutorTypeNames);
 
+// StaticSingleThreadedExecutor is not included in these tests for now, due to:
+// https://github.com/ros2/rclcpp/issues/1219
+using StandardExecutors =
+  ::testing::Types<
+  rclcpp::executors::SingleThreadedExecutor,
+  rclcpp::executors::MultiThreadedExecutor,
+  rclcpp::experimental::executors::EventsExecutor>;
+TYPED_TEST_SUITE(TestExecutorsStable, StandardExecutors, ExecutorTypeNames);
+
 // Make sure that executors detach from nodes when destructing
 TYPED_TEST(TestExecutors, detachOnDestruction)
 {
@@ -143,7 +155,10 @@ TYPED_TEST(TestExecutors, detachOnDestruction)
 }
 
 // Make sure that the executor can automatically remove expired nodes correctly
-TYPED_TEST(TestExecutors, addTemporaryNode) {
+// Currently fails for StaticSingleThreadedExecutor so it is being skipped, see:
+// https://github.com/ros2/rclcpp/issues/1231
+TYPED_TEST(TestExecutorsStable, addTemporaryNode)
+{
   using ExecutorType = TypeParam;
   // rmw_connextdds doesn't support events-executor
   if (
@@ -253,17 +268,14 @@ TYPED_TEST(TestExecutors, spinWhileAlreadySpinning)
   ExecutorType executor;
   executor.add_node(this->node);
 
-  std::atomic_bool timer_completed = false;
-  auto timer = this->node->create_wall_timer(
-    1ms, [&]() {
-      timer_completed.store(true);
-    });
+  bool timer_completed = false;
+  auto timer = this->node->create_wall_timer(1ms, [&]() {timer_completed = true;});
 
   std::thread spinner([&]() {executor.spin();});
   // Sleep for a short time to verify executor.spin() is going, and didn't throw.
 
   auto start = std::chrono::steady_clock::now();
-  while (!timer_completed.load() && (std::chrono::steady_clock::now() - start) < 10s) {
+  while (!timer_completed && (std::chrono::steady_clock::now() - start) < 10s) {
     std::this_thread::sleep_for(1ms);
   }
 
@@ -847,7 +859,7 @@ public:
     test_name << test_info->test_case_name() << "_" << test_info->name();
     node = std::make_shared<rclcpp::Node>("node", test_name.str());
 
-    callback_count = 0;
+    callback_count = 0u;
 
     const std::string topic_name = std::string("topic_") + test_name.str();
 
@@ -856,7 +868,7 @@ public:
     publisher = node->create_publisher<test_msgs::msg::Empty>(topic_name, rclcpp::QoS(1), po);
 
     auto callback = [this](test_msgs::msg::Empty::ConstSharedPtr) {
-        this->callback_count.fetch_add(1);
+        this->callback_count.fetch_add(1u);
       };
 
     rclcpp::SubscriptionOptions so;
@@ -878,7 +890,7 @@ public:
   rclcpp::Node::SharedPtr node;
   rclcpp::Publisher<test_msgs::msg::Empty>::SharedPtr publisher;
   rclcpp::Subscription<test_msgs::msg::Empty>::SharedPtr subscription;
-  std::atomic_int callback_count;
+  std::atomic_size_t callback_count;
 };
 
 TYPED_TEST_SUITE(TestIntraprocessExecutors, ExecutorTypes, ExecutorTypeNames);
@@ -894,7 +906,7 @@ TYPED_TEST(TestIntraprocessExecutors, testIntraprocessRetrigger) {
   ExecutorType executor;
   executor.add_node(this->node);
 
-  EXPECT_EQ(0, this->callback_count.load());
+  EXPECT_EQ(0u, this->callback_count.load());
   this->publisher->publish(test_msgs::msg::Empty());
 
   // Wait for up to 5 seconds for the first message to come available.
@@ -908,7 +920,7 @@ TYPED_TEST(TestIntraprocessExecutors, testIntraprocessRetrigger) {
   EXPECT_EQ(1u, this->callback_count.load());
 
   // reset counter
-  this->callback_count.store(0);
+  this->callback_count.store(0u);
 
   for (size_t ii = 0; ii < kNumMessages; ++ii) {
     this->publisher->publish(test_msgs::msg::Empty());
@@ -928,3 +940,31 @@ TYPED_TEST(TestIntraprocessExecutors, testIntraprocessRetrigger) {
   executor.spin();
   EXPECT_EQ(kNumMessages, this->callback_count.load());
 }
+
+// Check spin functions with non default context
+TEST(TestExecutors, testSpinWithNonDefaultContext)
+{
+  auto non_default_context = std::make_shared<rclcpp::Context>();
+  non_default_context->init(0, nullptr);
+
+  {
+    auto node =
+      std::make_unique<rclcpp::Node>("node", rclcpp::NodeOptions().context(non_default_context));
+
+    EXPECT_NO_THROW(rclcpp::spin_some(node->get_node_base_interface()));
+
+    auto check_spin_until_future_complete = [&]() {
+        std::promise<bool> promise;
+        std::future<bool> future = promise.get_future();
+        promise.set_value(true);
+
+        auto shared_future = future.share();
+        auto ret = rclcpp::spin_until_future_complete(
+          node->get_node_base_interface(), shared_future, 1s);
+        EXPECT_EQ(rclcpp::FutureReturnCode::SUCCESS, ret);
+      };
+    EXPECT_NO_THROW(check_spin_until_future_complete());
+  }
+
+  rclcpp::shutdown(non_default_context);
+}

rclcpp/test/rclcpp/node_interfaces/test_node_parameters.cpp

@@ -77,9 +77,9 @@ TEST_F(TestNodeParameters, list_parameters)
   std::vector<std::string> prefixes;
   const auto list_result = node_parameters->list_parameters(prefixes, 1u);
 
-  // Currently the only default parameter is 'use_sim_time', but that may change.
+  // Currently the default parameters are 'use_sim_time' and 'start_type_description_service'
   size_t number_of_parameters = list_result.names.size();
-  EXPECT_GE(1u, number_of_parameters);
+  EXPECT_GE(2u, number_of_parameters);
 
   const std::string parameter_name = "new_parameter";
   const rclcpp::ParameterValue value(true);

rclcpp/test/rclcpp/node_interfaces/test_node_type_descriptions.cpp

@@ -0,0 +1,63 @@
+// Copyright 2023 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <gtest/gtest.h>
+
+#include "rclcpp/node.hpp"
+#include "rclcpp/node_interfaces/node_type_descriptions.hpp"
+
+class TestNodeTypeDescriptions : public ::testing::Test
+{
+public:
+  void SetUp()
+  {
+    rclcpp::init(0, nullptr);
+  }
+
+  void TearDown()
+  {
+    rclcpp::shutdown();
+  }
+};
+
+TEST_F(TestNodeTypeDescriptions, interface_created)
+{
+  rclcpp::Node node{"node", "ns"};
+  ASSERT_NE(nullptr, node.get_node_type_descriptions_interface());
+}
+
+TEST_F(TestNodeTypeDescriptions, disabled_no_service)
+{
+  rclcpp::NodeOptions node_options;
+  node_options.append_parameter_override("start_type_description_service", false);
+  rclcpp::Node node{"node", "ns", node_options};
+
+  rcl_node_t * rcl_node = node.get_node_base_interface()->get_rcl_node_handle();
+  rcl_service_t * srv = nullptr;
+  rcl_ret_t ret = rcl_node_get_type_description_service(rcl_node, &srv);
+  ASSERT_EQ(RCL_RET_NOT_INIT, ret);
+}
+
+TEST_F(TestNodeTypeDescriptions, enabled_creates_service)
+{
+  rclcpp::NodeOptions node_options;
+  node_options.append_parameter_override("start_type_description_service", true);
+  rclcpp::Node node{"node", "ns", node_options};
+
+  rcl_node_t * rcl_node = node.get_node_base_interface()->get_rcl_node_handle();
+  rcl_service_t * srv = nullptr;
+  rcl_ret_t ret = rcl_node_get_type_description_service(rcl_node, &srv);
+  ASSERT_EQ(RCL_RET_OK, ret);
+  ASSERT_NE(nullptr, srv);
+}

rclcpp/test/rclcpp/test_client.cpp

@@ -521,6 +521,10 @@ TEST_F(TestClient, rcl_client_response_subscription_get_actual_qos_error) {
 }
 
 TEST_F(TestClient, client_qos) {
+  if (std::string(rmw_get_implementation_identifier()).find("rmw_connextdds") == 0) {
+    GTEST_SKIP() << "Skipping. The tests is known to be flaky in the rmw_connextdds.";
+  }
+
   rclcpp::ServicesQoS qos_profile;
   qos_profile.liveliness(rclcpp::LivelinessPolicy::Automatic);
   rclcpp::Duration duration(std::chrono::nanoseconds(1));

rclcpp/test/rclcpp/test_executor.cpp

@@ -46,6 +46,23 @@ public:
   {
     spin_node_once_nanoseconds(node, std::chrono::milliseconds(100));
   }
+
+  rclcpp::memory_strategy::MemoryStrategy * memory_strategy_ptr()
+  {
+    return memory_strategy_.get();
+  }
+
+  rclcpp::node_interfaces::NodeBaseInterface::SharedPtr local_get_node_by_group(
+    rclcpp::CallbackGroup::SharedPtr group)
+  {
+    std::lock_guard<std::mutex> guard_{mutex_};  // only to make the TSA happy
+    return get_node_by_group(weak_groups_to_nodes_, group);
+  }
+
+  rclcpp::CallbackGroup::SharedPtr local_get_group_by_timer(rclcpp::TimerBase::SharedPtr timer)
+  {
+    return get_group_by_timer(timer);
+  }
 };
 
 class TestExecutor : public ::testing::Test
@@ -113,7 +130,7 @@ TEST_F(TestExecutor, constructor_bad_wait_set_init) {
   auto mock = mocking_utils::patch_and_return("lib:rclcpp", rcl_wait_set_init, RCL_RET_ERROR);
   RCLCPP_EXPECT_THROW_EQ(
     static_cast<void>(std::make_unique<DummyExecutor>()),
-    std::runtime_error("Failed to create wait set: error not set"));
+    std::runtime_error("Failed to create wait set in Executor constructor: error not set"));
 }
 
 TEST_F(TestExecutor, add_callback_group_twice) {
@@ -125,7 +142,7 @@ TEST_F(TestExecutor, add_callback_group_twice) {
   cb_group->get_associated_with_executor_atomic().exchange(false);
   RCLCPP_EXPECT_THROW_EQ(
     dummy.add_callback_group(cb_group, node->get_node_base_interface(), false),
-    std::runtime_error("Callback group has already been added to this executor."));
+    std::runtime_error("Callback group was already added to executor."));
 }
 
 TEST_F(TestExecutor, add_callback_group_failed_trigger_guard_condition) {
@@ -151,15 +168,9 @@ TEST_F(TestExecutor, remove_callback_group_null_node) {
 
   node.reset();
 
-
-  /**
-   * TODO(mjcarroll): Assert this when we are enforcing that nodes must be destroyed
-   * after their created callback groups.
   RCLCPP_EXPECT_THROW_EQ(
     dummy.remove_callback_group(cb_group, false),
     std::runtime_error("Node must not be deleted before its callback group(s)."));
-   */
-  EXPECT_NO_THROW(dummy.remove_callback_group(cb_group, false));
 }
 
 TEST_F(TestExecutor, remove_callback_group_failed_trigger_guard_condition) {
@@ -186,7 +197,7 @@ TEST_F(TestExecutor, remove_node_not_associated) {
 
   RCLCPP_EXPECT_THROW_EQ(
     dummy.remove_node(node->get_node_base_interface(), false),
-    std::runtime_error("Node '/ns/node' needs to be associated with an executor."));
+    std::runtime_error("Node needs to be associated with an executor."));
 }
 
 TEST_F(TestExecutor, remove_node_associated_with_different_executor) {
@@ -200,7 +211,7 @@ TEST_F(TestExecutor, remove_node_associated_with_different_executor) {
 
   RCLCPP_EXPECT_THROW_EQ(
     dummy2.remove_node(node1->get_node_base_interface(), false),
-    std::runtime_error("Node '/ns/node1' needs to be associated with this executor."));
+    std::runtime_error("Node needs to be associated with this executor."));
 }
 
 TEST_F(TestExecutor, spin_node_once_nanoseconds) {
@@ -317,14 +328,42 @@ TEST_F(TestExecutor, cancel_failed_trigger_guard_condition) {
     std::runtime_error("Failed to trigger guard condition in cancel: error not set"));
 }
 
-TEST_F(TestExecutor, create_executor_fail_wait_set_clear) {
-  auto mock = mocking_utils::patch_and_return("lib:rclcpp", rcl_wait_set_clear, RCL_RET_ERROR);
+TEST_F(TestExecutor, set_memory_strategy_nullptr) {
+  DummyExecutor dummy;
+
   RCLCPP_EXPECT_THROW_EQ(
-    DummyExecutor dummy,
-    std::runtime_error("Couldn't clear the wait set: error not set"));
+    dummy.set_memory_strategy(nullptr),
+    std::runtime_error("Received NULL memory strategy in executor."));
 }
 
-TEST_F(TestExecutor, spin_all_fail_wait_set_clear) {
+TEST_F(TestExecutor, set_memory_strategy) {
+  DummyExecutor dummy;
+  rclcpp::memory_strategy::MemoryStrategy::SharedPtr strategy =
+    std::make_shared<
+    rclcpp::memory_strategies::allocator_memory_strategy::AllocatorMemoryStrategy<>>();
+
+  dummy.set_memory_strategy(strategy);
+  EXPECT_EQ(dummy.memory_strategy_ptr(), strategy.get());
+}
+
+TEST_F(TestExecutor, spin_once_failed_trigger_guard_condition) {
+  DummyExecutor dummy;
+  auto node = std::make_shared<rclcpp::Node>("node", "ns");
+  auto timer =
+    node->create_wall_timer(std::chrono::milliseconds(1), [&]() {});
+
+  dummy.add_node(node);
+  // Wait for the wall timer to have expired.
+  std::this_thread::sleep_for(std::chrono::milliseconds(50));
+  auto mock = mocking_utils::patch_and_return(
+    "lib:rclcpp", rcl_trigger_guard_condition, RCL_RET_ERROR);
+  RCLCPP_EXPECT_THROW_EQ(
+    dummy.spin_once(std::chrono::milliseconds(1)),
+    std::runtime_error(
+      "Failed to trigger guard condition from execute_any_executable: error not set"));
+}
+
+TEST_F(TestExecutor, spin_some_fail_wait_set_clear) {
   DummyExecutor dummy;
   auto node = std::make_shared<rclcpp::Node>("node", "ns");
   auto timer =
@@ -332,10 +371,9 @@ TEST_F(TestExecutor, spin_all_fail_wait_set_clear) {
 
   dummy.add_node(node);
   auto mock = mocking_utils::patch_and_return("lib:rclcpp", rcl_wait_set_clear, RCL_RET_ERROR);
-
   RCLCPP_EXPECT_THROW_EQ(
-    dummy.spin_all(std::chrono::milliseconds(1)),
-    std::runtime_error("Couldn't clear the wait set: error not set"));
+    dummy.spin_some(std::chrono::milliseconds(1)),
+    std::runtime_error("Couldn't clear wait set: error not set"));
 }
 
 TEST_F(TestExecutor, spin_some_fail_wait_set_resize) {
@@ -363,7 +401,7 @@ TEST_F(TestExecutor, spin_some_fail_add_handles_to_wait_set) {
     RCL_RET_ERROR);
   RCLCPP_EXPECT_THROW_EQ(
     dummy.spin_some(std::chrono::milliseconds(1)),
-    std::runtime_error("Couldn't fill wait set: error not set"));
+    std::runtime_error("Couldn't fill wait set"));
 }
 
 TEST_F(TestExecutor, spin_some_fail_wait) {
@@ -379,6 +417,71 @@ TEST_F(TestExecutor, spin_some_fail_wait) {
     std::runtime_error("rcl_wait() failed: error not set"));
 }
 
+TEST_F(TestExecutor, get_node_by_group_null_group) {
+  DummyExecutor dummy;
+  ASSERT_EQ(nullptr, dummy.local_get_node_by_group(nullptr));
+}
+
+TEST_F(TestExecutor, get_node_by_group) {
+  DummyExecutor dummy;
+  auto node = std::make_shared<rclcpp::Node>("node", "ns");
+  rclcpp::CallbackGroup::SharedPtr cb_group = node->create_callback_group(
+    rclcpp::CallbackGroupType::MutuallyExclusive);
+  dummy.add_callback_group(cb_group, node->get_node_base_interface(), false);
+  ASSERT_EQ(node->get_node_base_interface().get(), dummy.local_get_node_by_group(cb_group).get());
+}
+
+TEST_F(TestExecutor, get_node_by_group_not_found) {
+  DummyExecutor dummy;
+  auto node = std::make_shared<rclcpp::Node>("node", "ns");
+  rclcpp::CallbackGroup::SharedPtr cb_group = node->create_callback_group(
+    rclcpp::CallbackGroupType::MutuallyExclusive);
+  ASSERT_EQ(nullptr, dummy.local_get_node_by_group(cb_group).get());
+}
+
+TEST_F(TestExecutor, get_group_by_timer_nullptr) {
+  DummyExecutor dummy;
+  ASSERT_EQ(nullptr, dummy.local_get_group_by_timer(nullptr));
+}
+
+TEST_F(TestExecutor, get_group_by_timer) {
+  DummyExecutor dummy;
+  auto node = std::make_shared<rclcpp::Node>("node", "ns");
+  rclcpp::CallbackGroup::SharedPtr cb_group = node->create_callback_group(
+    rclcpp::CallbackGroupType::MutuallyExclusive);
+  auto timer =
+    node->create_wall_timer(std::chrono::milliseconds(1), [&]() {}, cb_group);
+  dummy.add_node(node);
+
+  ASSERT_EQ(cb_group.get(), dummy.local_get_group_by_timer(timer).get());
+}
+
+TEST_F(TestExecutor, get_group_by_timer_with_deleted_group) {
+  DummyExecutor dummy;
+  auto node = std::make_shared<rclcpp::Node>("node", "ns");
+  rclcpp::CallbackGroup::SharedPtr cb_group = node->create_callback_group(
+    rclcpp::CallbackGroupType::MutuallyExclusive);
+  auto timer =
+    node->create_wall_timer(std::chrono::milliseconds(1), [&]() {}, cb_group);
+  dummy.add_node(node);
+
+  cb_group.reset();
+
+  ASSERT_EQ(nullptr, dummy.local_get_group_by_timer(timer).get());
+}
+
+TEST_F(TestExecutor, get_group_by_timer_add_callback_group) {
+  DummyExecutor dummy;
+  auto node = std::make_shared<rclcpp::Node>("node", "ns");
+  rclcpp::CallbackGroup::SharedPtr cb_group = node->create_callback_group(
+    rclcpp::CallbackGroupType::MutuallyExclusive);
+  auto timer =
+    node->create_wall_timer(std::chrono::milliseconds(1), [&]() {}, cb_group);
+  dummy.add_callback_group(cb_group, node->get_node_base_interface(), false);
+
+  ASSERT_EQ(cb_group.get(), dummy.local_get_group_by_timer(timer).get());
+}
+
 TEST_F(TestExecutor, spin_until_future_complete_in_spin_until_future_complete) {
   DummyExecutor dummy;
   auto node = std::make_shared<rclcpp::Node>("node", "ns");

rclcpp/test/rclcpp/test_intra_process_buffer.cpp

@@ -238,75 +238,3 @@ TEST(TestIntraProcessBuffer, unique_buffer_consume) {
   EXPECT_EQ(original_value, *popped_unique_msg);
   EXPECT_EQ(original_message_pointer, popped_message_pointer);
 }
-
-/*
-  Check the available buffer capacity while storing and consuming data from an intra-process
-  buffer.
-  The initial available buffer capacity should equal the buffer size.
-  Inserting a message should decrease the available buffer capacity by 1.
-  Consuming a message should increase the available buffer capacity by 1.
- */
-TEST(TestIntraProcessBuffer, available_capacity) {
-  using MessageT = char;
-  using Alloc = std::allocator<void>;
-  using Deleter = std::default_delete<MessageT>;
-  using SharedMessageT = std::shared_ptr<const MessageT>;
-  using UniqueMessageT = std::unique_ptr<MessageT, Deleter>;
-  using UniqueIntraProcessBufferT = rclcpp::experimental::buffers::TypedIntraProcessBuffer<
-    MessageT, Alloc, Deleter, UniqueMessageT>;
-
-  constexpr auto history_depth = 5u;
-
-  auto buffer_impl =
-    std::make_unique<rclcpp::experimental::buffers::RingBufferImplementation<UniqueMessageT>>(
-    history_depth);
-
-  UniqueIntraProcessBufferT intra_process_buffer(std::move(buffer_impl));
-
-  EXPECT_EQ(history_depth, intra_process_buffer.available_capacity());
-
-  auto original_unique_msg = std::make_unique<char>('a');
-  auto original_message_pointer = reinterpret_cast<std::uintptr_t>(original_unique_msg.get());
-  auto original_value = *original_unique_msg;
-
-  intra_process_buffer.add_unique(std::move(original_unique_msg));
-
-  EXPECT_EQ(history_depth - 1u, intra_process_buffer.available_capacity());
-
-  SharedMessageT popped_shared_msg;
-  popped_shared_msg = intra_process_buffer.consume_shared();
-  auto popped_message_pointer = reinterpret_cast<std::uintptr_t>(popped_shared_msg.get());
-
-  EXPECT_EQ(history_depth, intra_process_buffer.available_capacity());
-  EXPECT_EQ(original_value, *popped_shared_msg);
-  EXPECT_EQ(original_message_pointer, popped_message_pointer);
-
-  original_unique_msg = std::make_unique<char>('b');
-  original_message_pointer = reinterpret_cast<std::uintptr_t>(original_unique_msg.get());
-  original_value = *original_unique_msg;
-
-  intra_process_buffer.add_unique(std::move(original_unique_msg));
-
-  auto second_unique_msg = std::make_unique<char>('c');
-  auto second_message_pointer = reinterpret_cast<std::uintptr_t>(second_unique_msg.get());
-  auto second_value = *second_unique_msg;
-
-  intra_process_buffer.add_unique(std::move(second_unique_msg));
-
-  EXPECT_EQ(history_depth - 2u, intra_process_buffer.available_capacity());
-
-  UniqueMessageT popped_unique_msg;
-  popped_unique_msg = intra_process_buffer.consume_unique();
-  popped_message_pointer = reinterpret_cast<std::uintptr_t>(popped_unique_msg.get());
-
-  EXPECT_EQ(history_depth - 1u, intra_process_buffer.available_capacity());
-  EXPECT_EQ(original_value, *popped_unique_msg);
-  EXPECT_EQ(original_message_pointer, popped_message_pointer);
-
-  popped_unique_msg = intra_process_buffer.consume_unique();
-  popped_message_pointer = reinterpret_cast<std::uintptr_t>(popped_unique_msg.get());
-
-  EXPECT_EQ(history_depth, intra_process_buffer.available_capacity());
-  EXPECT_EQ(second_value, *popped_unique_msg);
-  EXPECT_EQ(second_message_pointer, popped_message_pointer);
-}

rclcpp/test/rclcpp/test_intra_process_manager.cpp

@@ -156,26 +156,18 @@ public:
   {
     message_ptr = reinterpret_cast<std::uintptr_t>(msg.get());
     shared_msg = msg;
-    ++num_msgs;
   }
 
   void add(MessageUniquePtr msg)
   {
     message_ptr = reinterpret_cast<std::uintptr_t>(msg.get());
     unique_msg = std::move(msg);
-    ++num_msgs;
   }
 
   void pop(std::uintptr_t & msg_ptr)
   {
     msg_ptr = message_ptr;
     message_ptr = 0;
-    --num_msgs;
-  }
-
-  size_t size() const
-  {
-    return num_msgs;
   }
 
   // need to store the messages somewhere otherwise the memory address will be reused
@@ -183,8 +175,6 @@ public:
   MessageUniquePtr unique_msg;
 
   std::uintptr_t message_ptr;
-  // count add and pop
-  size_t num_msgs = 0u;
 };
 
 }  // namespace mock
@@ -231,10 +221,6 @@ public:
     return topic_name.c_str();
   }
 
-  virtual
-  size_t
-  available_capacity() const = 0;
-
   rclcpp::QoS qos_profile;
   std::string topic_name;
 };
@@ -294,12 +280,6 @@ public:
     return take_shared_method;
   }
 
-  size_t
-  available_capacity() const override
-  {
-    return qos_profile.depth() - buffer->size();
-  }
-
   bool take_shared_method;
 
   typename rclcpp::experimental::buffers::mock::IntraProcessBuffer<MessageT>::UniquePtr buffer;
@@ -732,91 +712,3 @@ TEST(TestIntraProcessManager, multiple_subscriptions_different_type) {
   EXPECT_EQ(original_message_pointer, received_message_pointer_10);
   EXPECT_NE(original_message_pointer, received_message_pointer_11);
 }
-
-/*
-   This tests the method "lowest_available_capacity":
-   - Creates 1 publisher.
-   - The available buffer capacity should be at least history size.
-   - Add 2 subscribers.
-   - Add everything to the intra-process manager.
-   - All the entities are expected to have different ids.
-   - Check the subscriptions count for the publisher.
-   - The available buffer capacity should be the history size.
-   - Publish one message (without receiving it).
-   - The available buffer capacity should decrease by 1.
-   - Publish another message (without receiving it).
-   - The available buffer capacity should decrease by 1.
-   - One subscriber receives one message.
-   - The available buffer capacity should stay the same,
-     as the other subscriber still has not freed its buffer.
-   - The other subscriber receives one message.
-   - The available buffer capacity should increase by 1.
-   - One subscription goes out of scope.
-   - The available buffer capacity should not change.
- */
-TEST(TestIntraProcessManager, lowest_available_capacity) {
-  using IntraProcessManagerT = rclcpp::experimental::IntraProcessManager;
-  using MessageT = rcl_interfaces::msg::Log;
-  using PublisherT = rclcpp::mock::Publisher<MessageT>;
-  using SubscriptionIntraProcessT = rclcpp::experimental::mock::SubscriptionIntraProcess<MessageT>;
-
-  constexpr auto history_depth = 10u;
-
-  auto ipm = std::make_shared<IntraProcessManagerT>();
-
-  auto p1 = std::make_shared<PublisherT>(rclcpp::QoS(history_depth).best_effort());
-
-  auto s1 = std::make_shared<SubscriptionIntraProcessT>(rclcpp::QoS(history_depth).best_effort());
-  auto s2 = std::make_shared<SubscriptionIntraProcessT>(rclcpp::QoS(history_depth).best_effort());
-
-  auto p1_id = ipm->add_publisher(p1);
-  p1->set_intra_process_manager(p1_id, ipm);
-
-  auto c1 = ipm->lowest_available_capacity(p1_id);
-
-  ASSERT_LE(0u, c1);
-
-  auto s1_id = ipm->add_subscription(s1);
-  auto s2_id = ipm->add_subscription(s2);
-
-  bool unique_ids = s1_id != s2_id && p1_id != s1_id;
-  ASSERT_TRUE(unique_ids);
-
-  size_t p1_subs = ipm->get_subscription_count(p1_id);
-  size_t non_existing_pub_subs = ipm->get_subscription_count(42);
-  ASSERT_EQ(2u, p1_subs);
-  ASSERT_EQ(0u, non_existing_pub_subs);
-
-  c1 = ipm->lowest_available_capacity(p1_id);
-  auto non_existing_pub_c = ipm->lowest_available_capacity(42);
-
-  ASSERT_EQ(history_depth, c1);
-  ASSERT_EQ(0u, non_existing_pub_c);
-
-  auto unique_msg = std::make_unique<MessageT>();
-  p1->publish(std::move(unique_msg));
-
-  c1 = ipm->lowest_available_capacity(p1_id);
-  ASSERT_EQ(history_depth - 1u, c1);
-
-  unique_msg = std::make_unique<MessageT>();
-  p1->publish(std::move(unique_msg));
-
-  c1 = ipm->lowest_available_capacity(p1_id);
-  ASSERT_EQ(history_depth - 2u, c1);
-
-  s1->pop();
-
-  c1 = ipm->lowest_available_capacity(p1_id);
-  ASSERT_EQ(history_depth - 2u, c1);
-
-  s2->pop();
-
-  c1 = ipm->lowest_available_capacity(p1_id);
-  ASSERT_EQ(history_depth - 1u, c1);
-
-  ipm->get_subscription_intra_process(s1_id).reset();
-
-  c1 = ipm->lowest_available_capacity(p1_id);
-  ASSERT_EQ(history_depth - 1u, c1);
-}

rclcpp/test/rclcpp/test_node.cpp

@@ -78,6 +78,7 @@ TEST_F(TestNode, construction_and_destruction) {
     EXPECT_NE(nullptr, node->get_node_options().get_rcl_node_options());
     EXPECT_NE(nullptr, node->get_graph_event());
     EXPECT_NE(nullptr, node->get_clock());
+    EXPECT_NE(nullptr, node->get_node_type_descriptions_interface());
   }
 
   {

rclcpp/test/rclcpp/test_parameter_client.cpp

@@ -59,6 +59,8 @@ protected:
     node_with_option.reset();
   }
 
+  // "start_type_description_service" and "use_sim_time"
+  const uint64_t builtin_param_count = 2;
   rclcpp::Node::SharedPtr node;
   rclcpp::Node::SharedPtr node_with_option;
 };
@@ -925,6 +927,7 @@ TEST_F(TestParameterClient, sync_parameter_delete_parameters) {
   Coverage for async load_parameters
  */
 TEST_F(TestParameterClient, async_parameter_load_parameters) {
+  const uint64_t expected_param_count = 4 + builtin_param_count;
   auto load_node = std::make_shared<rclcpp::Node>(
     "load_node",
     "namespace",
@@ -944,12 +947,13 @@ TEST_F(TestParameterClient, async_parameter_load_parameters) {
   auto list_parameters = asynchronous_client->list_parameters({}, 3);
   rclcpp::spin_until_future_complete(
     load_node, list_parameters, std::chrono::milliseconds(100));
-  ASSERT_EQ(list_parameters.get().names.size(), static_cast<uint64_t>(5));
+  ASSERT_EQ(list_parameters.get().names.size(), expected_param_count);
 }
 /*
   Coverage for sync load_parameters
  */
 TEST_F(TestParameterClient, sync_parameter_load_parameters) {
+  const uint64_t expected_param_count = 4 + builtin_param_count;
   auto load_node = std::make_shared<rclcpp::Node>(
     "load_node",
     "namespace",
@@ -964,13 +968,14 @@ TEST_F(TestParameterClient, sync_parameter_load_parameters) {
   ASSERT_EQ(load_future[0].successful, true);
   // list parameters
   auto list_parameters = synchronous_client->list_parameters({}, 3);
-  ASSERT_EQ(list_parameters.names.size(), static_cast<uint64_t>(5));
+  ASSERT_EQ(list_parameters.names.size(), static_cast<uint64_t>(expected_param_count));
 }
 
 /*
   Coverage for async load_parameters with complicated regex expression
  */
 TEST_F(TestParameterClient, async_parameter_load_parameters_complicated_regex) {
+  const uint64_t expected_param_count = 5 + builtin_param_count;
   auto load_node = std::make_shared<rclcpp::Node>(
     "load_node",
     "namespace",
@@ -990,7 +995,7 @@ TEST_F(TestParameterClient, async_parameter_load_parameters_complicated_regex) {
   auto list_parameters = asynchronous_client->list_parameters({}, 3);
   rclcpp::spin_until_future_complete(
     load_node, list_parameters, std::chrono::milliseconds(100));
-  ASSERT_EQ(list_parameters.get().names.size(), static_cast<uint64_t>(6));
+  ASSERT_EQ(list_parameters.get().names.size(), expected_param_count);
   // to check the parameter "a_value"
   std::string param_name = "a_value";
   auto param = load_node->get_parameter(param_name);
@@ -1020,6 +1025,7 @@ TEST_F(TestParameterClient, async_parameter_load_no_valid_parameter) {
   Coverage for async load_parameters from maps with complicated regex expression
  */
 TEST_F(TestParameterClient, async_parameter_load_parameters_from_map) {
+  const uint64_t expected_param_count = 5 + builtin_param_count;
   auto load_node = std::make_shared<rclcpp::Node>(
     "load_node",
     "namespace",
@@ -1068,7 +1074,7 @@ TEST_F(TestParameterClient, async_parameter_load_parameters_from_map) {
   auto list_parameters = asynchronous_client->list_parameters({}, 3);
   rclcpp::spin_until_future_complete(
     load_node, list_parameters, std::chrono::milliseconds(100));
-  ASSERT_EQ(list_parameters.get().names.size(), static_cast<uint64_t>(6));
+  ASSERT_EQ(list_parameters.get().names.size(), expected_param_count);
   // to check the parameter "a_value"
   std::string param_name = "a_value";
   auto param = load_node->get_parameter(param_name);

rclcpp/test/rclcpp/test_publisher.cpp

@@ -629,41 +629,6 @@ TEST_P(TestPublisherWaitForAllAcked, check_wait_for_all_acked_with_QosPolicy) {
   EXPECT_TRUE(pub->wait_for_all_acked(std::chrono::milliseconds(6000)));
 }
 
-TEST_F(TestPublisher, lowest_available_ipm_capacity) {
-  constexpr auto history_depth = 10u;
-
-  initialize(rclcpp::NodeOptions().use_intra_process_comms(true));
-
-  rclcpp::PublisherOptionsWithAllocator<std::allocator<void>> options_ipm_disabled;
-  options_ipm_disabled.use_intra_process_comm = rclcpp::IntraProcessSetting::Disable;
-
-  rclcpp::PublisherOptionsWithAllocator<std::allocator<void>> options_ipm_enabled;
-  options_ipm_enabled.use_intra_process_comm = rclcpp::IntraProcessSetting::Enable;
-
-  auto do_nothing = [](std::shared_ptr<const test_msgs::msg::Strings>) {};
-  auto pub_ipm_disabled = node->create_publisher<test_msgs::msg::Strings>(
-    "topic", history_depth,
-    options_ipm_disabled);
-  auto pub_ipm_enabled = node->create_publisher<test_msgs::msg::Strings>(
-    "topic", history_depth,
-    options_ipm_enabled);
-  auto sub = node->create_subscription<test_msgs::msg::Strings>(
-    "topic",
-    history_depth,
-    do_nothing);
-
-  ASSERT_EQ(1, pub_ipm_enabled->get_intra_process_subscription_count());
-  ASSERT_EQ(0, pub_ipm_disabled->lowest_available_ipm_capacity());
-  ASSERT_EQ(history_depth, pub_ipm_enabled->lowest_available_ipm_capacity());
-
-  auto msg = std::make_shared<test_msgs::msg::Strings>();
-  ASSERT_NO_THROW(pub_ipm_disabled->publish(*msg));
-  ASSERT_NO_THROW(pub_ipm_enabled->publish(*msg));
-
-  ASSERT_EQ(0, pub_ipm_disabled->lowest_available_ipm_capacity());
-  ASSERT_EQ(history_depth - 1u, pub_ipm_enabled->lowest_available_ipm_capacity());
-}
-
 INSTANTIATE_TEST_SUITE_P(
   TestWaitForAllAckedWithParm,
   TestPublisherWaitForAllAcked,

rclcpp/test/rclcpp/test_publisher_with_type_adapter.cpp

@@ -152,33 +152,54 @@ TEST_F(TestPublisher, conversion_exception_is_passed_up) {
   }
 }
 
+using UseTakeSharedMethod = bool;
+class TestPublisherFixture
+  : public TestPublisher,
+  public ::testing::WithParamInterface<UseTakeSharedMethod>
+{
+};
+
 /*
  * Testing that publisher sends type adapted types and ROS message types with intra proccess communications.
  */
-TEST_F(
-  TestPublisher,
+TEST_P(
+  TestPublisherFixture,
   check_type_adapted_message_is_sent_and_received_intra_process) {
   using StringTypeAdapter = rclcpp::TypeAdapter<std::string, rclcpp::msg::String>;
   const std::string message_data = "Message Data";
   const std::string topic_name = "topic_name";
   bool is_received;
 
-  auto callback =
-    [message_data, &is_received](
-    const rclcpp::msg::String::ConstSharedPtr msg,
-    const rclcpp::MessageInfo & message_info
-    ) -> void
-    {
-      is_received = true;
-      ASSERT_STREQ(message_data.c_str(), msg->data.c_str());
-      ASSERT_TRUE(message_info.get_rmw_message_info().from_intra_process);
-    };
-
   auto node = rclcpp::Node::make_shared(
     "test_intra_process",
     rclcpp::NodeOptions().use_intra_process_comms(true));
   auto pub = node->create_publisher<StringTypeAdapter>(topic_name, 10);
-  auto sub = node->create_subscription<rclcpp::msg::String>(topic_name, 1, callback);
+  rclcpp::Subscription<rclcpp::msg::String>::SharedPtr sub;
+  if (GetParam()) {
+    auto callback =
+      [message_data, &is_received](
+      const rclcpp::msg::String::ConstSharedPtr msg,
+      const rclcpp::MessageInfo & message_info
+      ) -> void
+      {
+        is_received = true;
+        ASSERT_STREQ(message_data.c_str(), msg->data.c_str());
+        ASSERT_TRUE(message_info.get_rmw_message_info().from_intra_process);
+      };
+    sub = node->create_subscription<rclcpp::msg::String>(topic_name, 1, callback);
+  } else {
+    auto callback_unique =
+      [message_data, &is_received](
+      rclcpp::msg::String::UniquePtr msg,
+      const rclcpp::MessageInfo & message_info
+      ) -> void
+      {
+        is_received = true;
+        ASSERT_STREQ(message_data.c_str(), msg->data.c_str());
+        ASSERT_TRUE(message_info.get_rmw_message_info().from_intra_process);
+      };
+    sub = node->create_subscription<rclcpp::msg::String>(topic_name, 1, callback_unique);
+  }
 
   auto wait_for_message_to_be_received = [&is_received, &node]() {
       rclcpp::executors::SingleThreadedExecutor executor;
@@ -239,6 +260,14 @@ TEST_F(
   }
 }
 
+INSTANTIATE_TEST_SUITE_P(
+  TestPublisherFixtureWithParam,
+  TestPublisherFixture,
+  ::testing::Values(
+    true,   // use take shared method
+    false   // not use take shared method
+));
+
 /*
  * Testing that publisher sends type adapted types and ROS message types with inter proccess communications.
  */

rclcpp/test/rclcpp/test_rate.cpp

@@ -18,10 +18,24 @@
 
 #include "rclcpp/rate.hpp"
 
+class TestRate : public ::testing::Test
+{
+public:
+  void SetUp()
+  {
+    rclcpp::init(0, nullptr);
+  }
+
+  void TearDown()
+  {
+    rclcpp::shutdown();
+  }
+};
+
 /*
    Basic tests for the Rate and WallRate classes.
  */
-TEST(TestRate, rate_basics) {
+TEST_F(TestRate, rate_basics) {
   auto period = std::chrono::milliseconds(1000);
   auto offset = std::chrono::milliseconds(500);
   auto epsilon = std::chrono::milliseconds(100);
@@ -61,7 +75,7 @@ TEST(TestRate, rate_basics) {
   ASSERT_TRUE(epsilon > delta);
 }
 
-TEST(TestRate, wall_rate_basics) {
+TEST_F(TestRate, wall_rate_basics) {
   auto period = std::chrono::milliseconds(100);
   auto offset = std::chrono::milliseconds(50);
   auto epsilon = std::chrono::milliseconds(1);
@@ -101,7 +115,7 @@ TEST(TestRate, wall_rate_basics) {
   EXPECT_GT(epsilon, delta);
 }
 
-TEST(TestRate, from_double) {
+TEST_F(TestRate, from_double) {
   {
     rclcpp::WallRate rate(1.0);
     EXPECT_EQ(std::chrono::seconds(1), rate.period());

rclcpp/test/rclcpp/test_rosout_subscription.cpp

@@ -70,7 +70,7 @@ TEST_F(TestRosoutSubscription, test_rosoutsubscription_getchild) {
     // before calling get_child of Logger
     {
       RCLCPP_INFO(
-        rclcpp::get_logger(logger_name), "%s", this->rosout_msg_data.c_str());
+        rclcpp::get_logger(logger_name), this->rosout_msg_data.c_str());
       auto future = received_msg_promise.get_future();
       auto return_code = rclcpp::spin_until_future_complete(this->node, future, 3s);
       ASSERT_EQ(rclcpp::FutureReturnCode::TIMEOUT, return_code);
@@ -83,7 +83,7 @@ TEST_F(TestRosoutSubscription, test_rosoutsubscription_getchild) {
     // after calling get_child of Logger
     // 1. use child_logger directly
     {
-      RCLCPP_INFO(child_logger, "%s", this->rosout_msg_data.c_str());
+      RCLCPP_INFO(child_logger, this->rosout_msg_data.c_str());
       auto future = received_msg_promise.get_future();
       auto return_code = rclcpp::spin_until_future_complete(this->node, future, 3s);
       ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
@@ -93,7 +93,7 @@ TEST_F(TestRosoutSubscription, test_rosoutsubscription_getchild) {
 
     // 2. use rclcpp::get_logger
     {
-      RCLCPP_INFO(rclcpp::get_logger(logger_name), "%s", this->rosout_msg_data.c_str());
+      RCLCPP_INFO(rclcpp::get_logger(logger_name), this->rosout_msg_data.c_str());
       auto future = received_msg_promise.get_future();
       auto return_code = rclcpp::spin_until_future_complete(this->node, future, 3s);
       ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
@@ -104,7 +104,7 @@ TEST_F(TestRosoutSubscription, test_rosoutsubscription_getchild) {
 
   // `child_logger` is end of life, there is no sublogger
   {
-    RCLCPP_INFO(rclcpp::get_logger(logger_name), "%s", this->rosout_msg_data.c_str());
+    RCLCPP_INFO(rclcpp::get_logger(logger_name), this->rosout_msg_data.c_str());
     auto future = received_msg_promise.get_future();
     auto return_code = rclcpp::spin_until_future_complete(this->node, future, 3s);
     ASSERT_EQ(rclcpp::FutureReturnCode::TIMEOUT, return_code);
@@ -119,7 +119,7 @@ TEST_F(TestRosoutSubscription, test_rosoutsubscription_parent_log) {
 
   rclcpp::Logger logger = this->node->get_logger();
   ASSERT_EQ(logger.get_name(), logger_name);
-  RCLCPP_INFO(logger, "%s", this->rosout_msg_data.c_str());
+  RCLCPP_INFO(logger, this->rosout_msg_data.c_str());
   auto future = received_msg_promise.get_future();
   auto return_code = rclcpp::spin_until_future_complete(this->node, future, 3s);
   ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
@@ -133,14 +133,14 @@ TEST_F(TestRosoutSubscription, test_rosoutsubscription_child_log) {
   this->rosout_msg_name = logger_name;
 
   rclcpp::Logger logger = this->node->get_logger();
-  RCLCPP_INFO(logger, "%s", this->rosout_msg_data.c_str());
+  RCLCPP_INFO(logger, this->rosout_msg_data.c_str());
   auto future = received_msg_promise.get_future();
   auto return_code = rclcpp::spin_until_future_complete(this->node, future, 3s);
   ASSERT_EQ(rclcpp::FutureReturnCode::TIMEOUT, return_code);
   received_msg_promise = {};
 
   logger = this->node->get_logger().get_child("child1");
-  RCLCPP_INFO(logger, "%s", this->rosout_msg_data.c_str());
+  RCLCPP_INFO(logger, this->rosout_msg_data.c_str());
   future = received_msg_promise.get_future();
   return_code = rclcpp::spin_until_future_complete(this->node, future, 3s);
   ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
@@ -148,14 +148,14 @@ TEST_F(TestRosoutSubscription, test_rosoutsubscription_child_log) {
   received_msg_promise = {};
 
   logger = this->node->get_logger().get_child("child2");
-  RCLCPP_INFO(logger, "%s", this->rosout_msg_data.c_str());
+  RCLCPP_INFO(logger, this->rosout_msg_data.c_str());
   future = received_msg_promise.get_future();
   return_code = rclcpp::spin_until_future_complete(this->node, future, 3s);
   ASSERT_EQ(rclcpp::FutureReturnCode::TIMEOUT, return_code);
   received_msg_promise = {};
 
   this->rosout_msg_name = "ns.test_rosout_subscription.child2";
-  RCLCPP_INFO(logger, "%s", this->rosout_msg_data.c_str());
+  RCLCPP_INFO(logger, this->rosout_msg_data.c_str());
   future = received_msg_promise.get_future();
   return_code = rclcpp::spin_until_future_complete(this->node, future, 3s);
   ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);
@@ -171,7 +171,7 @@ TEST_F(TestRosoutSubscription, test_rosoutsubscription_getchild_hierarchy) {
   rclcpp::Logger grandchild_logger =
     this->node->get_logger().get_child("child").get_child("grandchild");
   ASSERT_EQ(grandchild_logger.get_name(), logger_name);
-  RCLCPP_INFO(grandchild_logger, "%s", this->rosout_msg_data.c_str());
+  RCLCPP_INFO(grandchild_logger, this->rosout_msg_data.c_str());
   auto future = received_msg_promise.get_future();
   auto return_code = rclcpp::spin_until_future_complete(this->node, future, 3s);
   ASSERT_EQ(rclcpp::FutureReturnCode::SUCCESS, return_code);

rclcpp/test/rclcpp/test_subscription_content_filter.cpp

@@ -310,3 +310,25 @@ TEST_F(CLASSNAME(TestContentFilterSubscription, RMW_IMPLEMENTATION), content_fil
     }
   }
 }
+
+TEST_F(
+  CLASSNAME(
+    TestContentFilterSubscription,
+    RMW_IMPLEMENTATION), create_two_content_filters_with_same_topic_name_and_destroy) {
+
+  // Create another content filter
+  auto options = rclcpp::SubscriptionOptions();
+
+  std::string filter_expression = "int32_value > %0";
+  std::vector<std::string> expression_parameters = {"4"};
+
+  options.content_filter_options.filter_expression = filter_expression;
+  options.content_filter_options.expression_parameters = expression_parameters;
+
+  auto callback = [](std::shared_ptr<const test_msgs::msg::BasicTypes>) {};
+  auto sub_2 = node->create_subscription<test_msgs::msg::BasicTypes>(
+    "content_filter_topic", qos, callback, options);
+
+  EXPECT_NE(nullptr, sub_2);
+  sub_2.reset();
+}

rclcpp/test/rclcpp/test_timers_manager.cpp

@@ -296,7 +296,7 @@ TEST_F(TestTimersManager, timers_thread)
 
   // Run timers thread for a while
   timers_manager->start();
-  std::this_thread::sleep_for(10ms);
+  std::this_thread::sleep_for(5ms);
   timers_manager->stop();
 
   EXPECT_LT(1u, t1_runs);

rclcpp_action/CHANGELOG.rst

@@ -3,13 +3,31 @@ Changelog for package rclcpp_action
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
-21.2.0 (2023-06-07)
+21.0.8 (2024-11-08)
 -------------------
 
-21.1.1 (2023-05-11)
+21.0.7 (2024-07-10)
+-------------------
+* fix: Fixed race condition in action server between is_ready and take"… (`#2531 <https://github.com/ros2/rclcpp/issues/2531>`_)
+* Do not generate the exception when action service response timeout. (`#2519 <https://github.com/ros2/rclcpp/issues/2519>`_)
+* Contributors: Janosch Machowinski, Tomoya Fujita, William Woodall
+
+21.0.6 (2024-04-19)
 -------------------
 
-21.1.0 (2023-04-27)
+21.0.5 (2024-02-07)
+-------------------
+
+21.0.4 (2023-11-17)
+-------------------
+
+21.0.3 (2023-09-08)
+-------------------
+
+21.0.2 (2023-07-14)
+-------------------
+
+21.0.1 (2023-05-11)
 -------------------
 
 21.0.0 (2023-04-18)

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>21.2.0</version>
+  <version>21.0.8</version>
   <description>Adds action APIs for C++.</description>
 
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>

rclcpp_action/src/client.cpp

@@ -19,6 +19,7 @@
 #include <string>
 #include <tuple>
 #include <utility>
+#include <variant>
 
 #include "rcl_action/action_client.h"
 #include "rcl_action/wait.h"
@@ -31,6 +32,67 @@
 namespace rclcpp_action
 {
 
+struct ClientBaseData
+{
+  struct FeedbackReadyData
+  {
+    FeedbackReadyData(rcl_ret_t retIn, std::shared_ptr<void> msg)
+    : ret(retIn), feedback_message(msg) {}
+    rcl_ret_t ret;
+    std::shared_ptr<void> feedback_message;
+  };
+  struct StatusReadyData
+  {
+    StatusReadyData(rcl_ret_t retIn, std::shared_ptr<void> msg)
+    : ret(retIn), status_message(msg) {}
+    rcl_ret_t ret;
+    std::shared_ptr<void> status_message;
+  };
+  struct GoalResponseData
+  {
+    GoalResponseData(rcl_ret_t retIn, rmw_request_id_t header, std::shared_ptr<void> response)
+    : ret(retIn), response_header(header), goal_response(response) {}
+    rcl_ret_t ret;
+    rmw_request_id_t response_header;
+    std::shared_ptr<void> goal_response;
+  };
+  struct CancelResponseData
+  {
+    CancelResponseData(rcl_ret_t retIn, rmw_request_id_t header, std::shared_ptr<void> response)
+    : ret(retIn), response_header(header), cancel_response(response) {}
+    rcl_ret_t ret;
+    rmw_request_id_t response_header;
+    std::shared_ptr<void> cancel_response;
+  };
+  struct ResultResponseData
+  {
+    ResultResponseData(rcl_ret_t retIn, rmw_request_id_t header, std::shared_ptr<void> response)
+    : ret(retIn), response_header(header), result_response(response) {}
+    rcl_ret_t ret;
+    rmw_request_id_t response_header;
+    std::shared_ptr<void> result_response;
+  };
+
+  std::variant<
+    FeedbackReadyData,
+    StatusReadyData,
+    GoalResponseData,
+    CancelResponseData,
+    ResultResponseData
+  > data;
+
+  explicit ClientBaseData(FeedbackReadyData && data_in)
+  : data(std::move(data_in)) {}
+  explicit ClientBaseData(StatusReadyData && data_in)
+  : data(std::move(data_in)) {}
+  explicit ClientBaseData(GoalResponseData && data_in)
+  : data(std::move(data_in)) {}
+  explicit ClientBaseData(CancelResponseData && data_in)
+  : data(std::move(data_in)) {}
+  explicit ClientBaseData(ResultResponseData && data_in)
+  : data(std::move(data_in)) {}
+};
+
 class ClientBaseImpl
 {
 public:
@@ -94,11 +156,13 @@ public:
   size_t num_clients{0u};
   size_t num_services{0u};
 
-  bool is_feedback_ready{false};
-  bool is_status_ready{false};
-  bool is_goal_response_ready{false};
-  bool is_cancel_response_ready{false};
-  bool is_result_response_ready{false};
+  // Lock for action_client_
+  std::recursive_mutex action_client_mutex_;
+
+  // next ready event for taking, will be set by is_ready and will be processed by take_data
+  std::atomic<size_t> next_ready_event;
+  // used to indicate that next_ready_event has no ready event for processing
+  static constexpr size_t NO_EVENT_READY = std::numeric_limits<size_t>::max();
 
   rclcpp::Context::SharedPtr context_;
   rclcpp::node_interfaces::NodeGraphInterface::WeakPtr node_graph_;
@@ -142,6 +206,7 @@ bool
 ClientBase::action_server_is_ready() const
 {
   bool is_ready;
+  std::lock_guard<std::recursive_mutex> lock(pimpl_->action_client_mutex_);
   rcl_ret_t ret = rcl_action_server_is_available(
     this->pimpl_->node_handle.get(),
     this->pimpl_->client_handle.get(),
@@ -255,6 +320,7 @@ ClientBase::get_number_of_ready_services()
 void
 ClientBase::add_to_wait_set(rcl_wait_set_t * wait_set)
 {
+  std::lock_guard<std::recursive_mutex> lock(pimpl_->action_client_mutex_);
   rcl_ret_t ret = rcl_action_wait_set_add_action_client(
     wait_set, pimpl_->client_handle.get(), nullptr, nullptr);
   if (RCL_RET_OK != ret) {
@@ -265,23 +331,56 @@ ClientBase::add_to_wait_set(rcl_wait_set_t * wait_set)
 bool
 ClientBase::is_ready(rcl_wait_set_t * wait_set)
 {
-  rcl_ret_t ret = rcl_action_client_wait_set_get_entities_ready(
-    wait_set, pimpl_->client_handle.get(),
-    &pimpl_->is_feedback_ready,
-    &pimpl_->is_status_ready,
-    &pimpl_->is_goal_response_ready,
-    &pimpl_->is_cancel_response_ready,
-    &pimpl_->is_result_response_ready);
-  if (RCL_RET_OK != ret) {
-    rclcpp::exceptions::throw_from_rcl_error(
-      ret, "failed to check for any ready entities");
+  bool is_feedback_ready{false};
+  bool is_status_ready{false};
+  bool is_goal_response_ready{false};
+  bool is_cancel_response_ready{false};
+  bool is_result_response_ready{false};
+
+  rcl_ret_t ret;
+  {
+    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_client_mutex_);
+    ret = rcl_action_client_wait_set_get_entities_ready(
+      wait_set, pimpl_->client_handle.get(),
+      &is_feedback_ready,
+      &is_status_ready,
+      &is_goal_response_ready,
+      &is_cancel_response_ready,
+      &is_result_response_ready);
+    if (RCL_RET_OK != ret) {
+      rclcpp::exceptions::throw_from_rcl_error(
+        ret, "failed to check for any ready entities");
+    }
   }
-  return
-    pimpl_->is_feedback_ready ||
-    pimpl_->is_status_ready ||
-    pimpl_->is_goal_response_ready ||
-    pimpl_->is_cancel_response_ready ||
-    pimpl_->is_result_response_ready;
+
+  pimpl_->next_ready_event = ClientBaseImpl::NO_EVENT_READY;
+
+  if (is_feedback_ready) {
+    pimpl_->next_ready_event = static_cast<size_t>(EntityType::FeedbackSubscription);
+    return true;
+  }
+
+  if (is_status_ready) {
+    pimpl_->next_ready_event = static_cast<size_t>(EntityType::StatusSubscription);
+    return true;
+  }
+
+  if (is_goal_response_ready) {
+    pimpl_->next_ready_event = static_cast<size_t>(EntityType::GoalClient);
+    return true;
+  }
+
+  if (is_result_response_ready) {
+    pimpl_->next_ready_event = static_cast<size_t>(EntityType::ResultClient);
+    return true;
+  }
+
+  if (is_cancel_response_ready) {
+    pimpl_->next_ready_event = static_cast<size_t>(EntityType::CancelClient);
+    return true;
+  }
+
+  return false;
 }
 
 void
@@ -432,7 +531,6 @@ ClientBase::set_callback_to_entity(
       }
     };
 
-
   // Set it temporarily to the new callback, while we replace the old one.
   // This two-step setting, prevents a gap where the old std::function has
   // been replaced but the middleware hasn't been told about the new one yet.
@@ -550,140 +648,159 @@ ClientBase::clear_on_ready_callback()
 std::shared_ptr<void>
 ClientBase::take_data()
 {
-  if (pimpl_->is_feedback_ready) {
-    std::shared_ptr<void> feedback_message = this->create_feedback_message();
-    rcl_ret_t ret = rcl_action_take_feedback(
-      pimpl_->client_handle.get(), feedback_message.get());
-    return std::static_pointer_cast<void>(
-      std::make_shared<std::tuple<rcl_ret_t, std::shared_ptr<void>>>(
-        ret, feedback_message));
-  } else if (pimpl_->is_status_ready) {
-    std::shared_ptr<void> status_message = this->create_status_message();
-    rcl_ret_t ret = rcl_action_take_status(
-      pimpl_->client_handle.get(), status_message.get());
-    return std::static_pointer_cast<void>(
-      std::make_shared<std::tuple<rcl_ret_t, std::shared_ptr<void>>>(
-        ret, status_message));
-  } else if (pimpl_->is_goal_response_ready) {
-    rmw_request_id_t response_header;
-    std::shared_ptr<void> goal_response = this->create_goal_response();
-    rcl_ret_t ret = rcl_action_take_goal_response(
-      pimpl_->client_handle.get(), &response_header, goal_response.get());
-    return std::static_pointer_cast<void>(
-      std::make_shared<std::tuple<rcl_ret_t, rmw_request_id_t, std::shared_ptr<void>>>(
-        ret, response_header, goal_response));
-  } else if (pimpl_->is_result_response_ready) {
-    rmw_request_id_t response_header;
-    std::shared_ptr<void> result_response = this->create_result_response();
-    rcl_ret_t ret = rcl_action_take_result_response(
-      pimpl_->client_handle.get(), &response_header, result_response.get());
-    return std::static_pointer_cast<void>(
-      std::make_shared<std::tuple<rcl_ret_t, rmw_request_id_t, std::shared_ptr<void>>>(
-        ret, response_header, result_response));
-  } else if (pimpl_->is_cancel_response_ready) {
-    rmw_request_id_t response_header;
-    std::shared_ptr<void> cancel_response = this->create_cancel_response();
-    rcl_ret_t ret = rcl_action_take_cancel_response(
-      pimpl_->client_handle.get(), &response_header, cancel_response.get());
-    return std::static_pointer_cast<void>(
-      std::make_shared<std::tuple<rcl_ret_t, rmw_request_id_t, std::shared_ptr<void>>>(
-        ret, response_header, cancel_response));
-  } else {
-    throw std::runtime_error("Taking data from action client but nothing is ready");
+  // next_ready_event is an atomic, caching localy
+  size_t next_ready_event = pimpl_->next_ready_event.exchange(ClientBaseImpl::NO_EVENT_READY);
+
+  if (next_ready_event == ClientBaseImpl::NO_EVENT_READY) {
+    // there is a known bug in iron, that take_data might be called multiple
+    // times. Therefore instead of throwing, we just return a nullptr as a workaround.
+    return nullptr;
   }
+
+  return take_data_by_entity_id(next_ready_event);
 }
 
 std::shared_ptr<void>
 ClientBase::take_data_by_entity_id(size_t id)
 {
+  std::shared_ptr<ClientBaseData> data_ptr;
+  rcl_ret_t ret;
+
   // Mark as ready the entity from which we want to take data
   switch (static_cast<EntityType>(id)) {
     case EntityType::GoalClient:
-      pimpl_->is_goal_response_ready = true;
+      {
+        rmw_request_id_t response_header;
+        std::shared_ptr<void> goal_response;
+        {
+          std::lock_guard<std::recursive_mutex> lock(pimpl_->action_client_mutex_);
+
+          goal_response = this->create_goal_response();
+          ret = rcl_action_take_goal_response(
+            pimpl_->client_handle.get(), &response_header, goal_response.get());
+        }
+        data_ptr = std::make_shared<ClientBaseData>(
+          ClientBaseData::GoalResponseData(
+            ret, response_header, goal_response));
+      }
       break;
     case EntityType::ResultClient:
-      pimpl_->is_result_response_ready = true;
+      {
+        rmw_request_id_t response_header;
+        std::shared_ptr<void> result_response;
+        {
+          std::lock_guard<std::recursive_mutex> lock(pimpl_->action_client_mutex_);
+          result_response = this->create_result_response();
+          ret = rcl_action_take_result_response(
+            pimpl_->client_handle.get(), &response_header, result_response.get());
+        }
+        data_ptr =
+          std::make_shared<ClientBaseData>(
+          ClientBaseData::ResultResponseData(
+            ret, response_header, result_response));
+      }
       break;
     case EntityType::CancelClient:
-      pimpl_->is_cancel_response_ready = true;
+      {
+        rmw_request_id_t response_header;
+        std::shared_ptr<void> cancel_response;
+        {
+          std::lock_guard<std::recursive_mutex> lock(pimpl_->action_client_mutex_);
+          cancel_response = this->create_cancel_response();
+          ret = rcl_action_take_cancel_response(
+            pimpl_->client_handle.get(), &response_header, cancel_response.get());
+        }
+        data_ptr =
+          std::make_shared<ClientBaseData>(
+          ClientBaseData::CancelResponseData(
+            ret, response_header, cancel_response));
+      }
       break;
     case EntityType::FeedbackSubscription:
-      pimpl_->is_feedback_ready = true;
+      {
+        std::shared_ptr<void> feedback_message;
+        {
+          std::lock_guard<std::recursive_mutex> lock(pimpl_->action_client_mutex_);
+          feedback_message = this->create_feedback_message();
+          ret = rcl_action_take_feedback(
+            pimpl_->client_handle.get(), feedback_message.get());
+        }
+        data_ptr =
+          std::make_shared<ClientBaseData>(
+          ClientBaseData::FeedbackReadyData(
+            ret, feedback_message));
+      }
       break;
     case EntityType::StatusSubscription:
-      pimpl_->is_status_ready = true;
+      {
+        std::shared_ptr<void> status_message;
+        {
+          std::lock_guard<std::recursive_mutex> lock(pimpl_->action_client_mutex_);
+          status_message = this->create_status_message();
+          ret = rcl_action_take_status(
+            pimpl_->client_handle.get(), status_message.get());
+        }
+        data_ptr =
+          std::make_shared<ClientBaseData>(
+          ClientBaseData::StatusReadyData(
+            ret, status_message));
+      }
       break;
   }
 
-  return take_data();
+  return std::static_pointer_cast<void>(data_ptr);
 }
 
 void
-ClientBase::execute(std::shared_ptr<void> & data)
+ClientBase::execute(std::shared_ptr<void> & data_in)
 {
-  if (!data) {
-    throw std::runtime_error("'data' is empty");
+  if (!data_in) {
+    // workaround, if take_data was called multiple timed, it returns a nullptr
+    // normally we should throw here, but as an API stable bug fix, we just ignore this...
+    return;
   }
 
-  if (pimpl_->is_feedback_ready) {
-    auto shared_ptr = std::static_pointer_cast<std::tuple<rcl_ret_t, std::shared_ptr<void>>>(data);
-    auto ret = std::get<0>(*shared_ptr);
-    pimpl_->is_feedback_ready = false;
-    if (RCL_RET_OK == ret) {
-      auto feedback_message = std::get<1>(*shared_ptr);
-      this->handle_feedback_message(feedback_message);
-    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking feedback");
-    }
-  } else if (pimpl_->is_status_ready) {
-    auto shared_ptr = std::static_pointer_cast<std::tuple<rcl_ret_t, std::shared_ptr<void>>>(data);
-    auto ret = std::get<0>(*shared_ptr);
-    pimpl_->is_status_ready = false;
-    if (RCL_RET_OK == ret) {
-      auto status_message = std::get<1>(*shared_ptr);
-      this->handle_status_message(status_message);
-    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking status");
-    }
-  } else if (pimpl_->is_goal_response_ready) {
-    auto shared_ptr = std::static_pointer_cast<
-      std::tuple<rcl_ret_t, rmw_request_id_t, std::shared_ptr<void>>>(data);
-    auto ret = std::get<0>(*shared_ptr);
-    pimpl_->is_goal_response_ready = false;
-    if (RCL_RET_OK == ret) {
-      auto response_header = std::get<1>(*shared_ptr);
-      auto goal_response = std::get<2>(*shared_ptr);
-      this->handle_goal_response(response_header, goal_response);
-    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking goal response");
-    }
-  } else if (pimpl_->is_result_response_ready) {
-    auto shared_ptr = std::static_pointer_cast<
-      std::tuple<rcl_ret_t, rmw_request_id_t, std::shared_ptr<void>>>(data);
-    auto ret = std::get<0>(*shared_ptr);
-    pimpl_->is_result_response_ready = false;
-    if (RCL_RET_OK == ret) {
-      auto response_header = std::get<1>(*shared_ptr);
-      auto result_response = std::get<2>(*shared_ptr);
-      this->handle_result_response(response_header, result_response);
-    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking result response");
-    }
-  } else if (pimpl_->is_cancel_response_ready) {
-    auto shared_ptr = std::static_pointer_cast<
-      std::tuple<rcl_ret_t, rmw_request_id_t, std::shared_ptr<void>>>(data);
-    auto ret = std::get<0>(*shared_ptr);
-    pimpl_->is_cancel_response_ready = false;
-    if (RCL_RET_OK == ret) {
-      auto response_header = std::get<1>(*shared_ptr);
-      auto cancel_response = std::get<2>(*shared_ptr);
-      this->handle_cancel_response(response_header, cancel_response);
-    } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != ret) {
-      rclcpp::exceptions::throw_from_rcl_error(ret, "error taking cancel response");
-    }
-  } else {
-    throw std::runtime_error("Executing action client but nothing is ready");
-  }
+  std::shared_ptr<ClientBaseData> data_ptr = std::static_pointer_cast<ClientBaseData>(data_in);
+
+  std::visit(
+    [&](auto && data) -> void {
+      using T = std::decay_t<decltype(data)>;
+      if constexpr (std::is_same_v<T, ClientBaseData::FeedbackReadyData>) {
+        if (RCL_RET_OK == data.ret) {
+          this->handle_feedback_message(data.feedback_message);
+        } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != data.ret) {
+          rclcpp::exceptions::throw_from_rcl_error(data.ret, "error taking feedback");
+        }
+      }
+      if constexpr (std::is_same_v<T, ClientBaseData::StatusReadyData>) {
+        if (RCL_RET_OK == data.ret) {
+          this->handle_status_message(data.status_message);
+        } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != data.ret) {
+          rclcpp::exceptions::throw_from_rcl_error(data.ret, "error taking status");
+        }
+      }
+      if constexpr (std::is_same_v<T, ClientBaseData::GoalResponseData>) {
+        if (RCL_RET_OK == data.ret) {
+          this->handle_goal_response(data.response_header, data.goal_response);
+        } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != data.ret) {
+          rclcpp::exceptions::throw_from_rcl_error(data.ret, "error taking goal response");
+        }
+      }
+      if constexpr (std::is_same_v<T, ClientBaseData::ResultResponseData>) {
+        if (RCL_RET_OK == data.ret) {
+          this->handle_result_response(data.response_header, data.result_response);
+        } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != data.ret) {
+          rclcpp::exceptions::throw_from_rcl_error(data.ret, "error taking result response");
+        }
+      }
+      if constexpr (std::is_same_v<T, ClientBaseData::CancelResponseData>) {
+        if (RCL_RET_OK == data.ret) {
+          this->handle_cancel_response(data.response_header, data.cancel_response);
+        } else if (RCL_RET_ACTION_CLIENT_TAKE_FAILED != data.ret) {
+          rclcpp::exceptions::throw_from_rcl_error(data.ret, "error taking cancel response");
+        }
+      }
+    }, data_ptr->data);
 }
 
 }  // namespace rclcpp_action

rclcpp_action/src/server.cpp

@@ -18,6 +18,7 @@
 #include <tuple>
 #include <unordered_map>
 #include <utility>
+#include <variant>
 #include <vector>
 
 #include "rcl_action/action_server.h"
@@ -33,8 +34,50 @@
 using rclcpp_action::ServerBase;
 using rclcpp_action::GoalUUID;
 
+struct ServerBaseData;
+
 namespace rclcpp_action
 {
+
+struct ServerBaseData
+{
+  using GoalRequestData = std::tuple<
+    rcl_ret_t,
+    const rcl_action_goal_info_t,
+    rmw_request_id_t,
+    std::shared_ptr<void>
+  >;
+
+  using CancelRequestData = std::tuple<
+    rcl_ret_t,
+    std::shared_ptr<action_msgs::srv::CancelGoal::Request>,
+    rmw_request_id_t
+  >;
+
+  using ResultRequestData = std::tuple<rcl_ret_t, std::shared_ptr<void>, rmw_request_id_t>;
+
+  using GoalExpiredData = struct Empty {};
+
+  std::variant<GoalRequestData, CancelRequestData, ResultRequestData, GoalExpiredData> data;
+
+  explicit ServerBaseData(GoalRequestData && data_in)
+  : data(std::move(data_in)) {}
+  explicit ServerBaseData(CancelRequestData && data_in)
+  : data(std::move(data_in)) {}
+  explicit ServerBaseData(ResultRequestData && data_in)
+  : data(std::move(data_in)) {}
+  explicit ServerBaseData(GoalExpiredData && data_in)
+  : data(std::move(data_in)) {}
+};
+
+enum class ActionEventType : std::size_t
+{
+  GoalService,
+  ResultService,
+  CancelService,
+  Expired,
+};
+
 class ServerBaseImpl
 {
 public:
@@ -60,11 +103,6 @@ public:
   size_t num_services_ = 0;
   size_t num_guard_conditions_ = 0;
 
-  std::atomic<bool> goal_request_ready_{false};
-  std::atomic<bool> cancel_request_ready_{false};
-  std::atomic<bool> result_request_ready_{false};
-  std::atomic<bool> goal_expired_{false};
-
   // Lock for unordered_maps
   std::recursive_mutex unordered_map_mutex_;
 
@@ -75,8 +113,15 @@ public:
   // rcl goal handles are kept so api to send result doesn't try to access freed memory
   std::unordered_map<GoalUUID, std::shared_ptr<rcl_action_goal_handle_t>> goal_handles_;
 
+
+  // next ready event for taking, will be set by is_ready and will be processed by take_data
+  std::atomic<size_t> next_ready_event;
+  // used to indicate that next_ready_event has no ready event for processing
+  static constexpr size_t NO_EVENT_READY = std::numeric_limits<size_t>::max();
+
   rclcpp::Logger logger_;
 };
+
 }  // namespace rclcpp_action
 
 ServerBase::ServerBase(
@@ -194,124 +239,170 @@ ServerBase::is_ready(rcl_wait_set_t * wait_set)
       &goal_expired);
   }
 
-  pimpl_->goal_request_ready_ = goal_request_ready;
-  pimpl_->cancel_request_ready_ = cancel_request_ready;
-  pimpl_->result_request_ready_ = result_request_ready;
-  pimpl_->goal_expired_ = goal_expired;
-
   if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret);
   }
 
-  return pimpl_->goal_request_ready_.load() ||
-         pimpl_->cancel_request_ready_.load() ||
-         pimpl_->result_request_ready_.load() ||
-         pimpl_->goal_expired_.load();
+  pimpl_->next_ready_event = ServerBaseImpl::NO_EVENT_READY;
+
+  if (goal_request_ready) {
+    pimpl_->next_ready_event = static_cast<uint32_t>(ActionEventType::GoalService);
+    return true;
+  }
+
+  if (cancel_request_ready) {
+    pimpl_->next_ready_event = static_cast<uint32_t>(ActionEventType::CancelService);
+    return true;
+  }
+
+  if (result_request_ready) {
+    pimpl_->next_ready_event = static_cast<uint32_t>(ActionEventType::ResultService);
+    return true;
+  }
+
+  if (goal_expired) {
+    pimpl_->next_ready_event = static_cast<uint32_t>(ActionEventType::Expired);
+    return true;
+  }
+
+  return false;
 }
 
 std::shared_ptr<void>
 ServerBase::take_data()
 {
-  if (pimpl_->goal_request_ready_.load()) {
-    rcl_ret_t ret;
-    rcl_action_goal_info_t goal_info = rcl_action_get_zero_initialized_goal_info();
-    rmw_request_id_t request_header;
+  size_t next_ready_event = pimpl_->next_ready_event.exchange(ServerBaseImpl::NO_EVENT_READY);
 
-    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
-
-    std::shared_ptr<void> message = create_goal_request();
-    ret = rcl_action_take_goal_request(
-      pimpl_->action_server_.get(),
-      &request_header,
-      message.get());
-
-    return std::static_pointer_cast<void>(
-      std::make_shared
-      <std::tuple<rcl_ret_t, rcl_action_goal_info_t, rmw_request_id_t, std::shared_ptr<void>>>(
-        ret,
-        goal_info,
-        request_header, message));
-  } else if (pimpl_->cancel_request_ready_.load()) {
-    rcl_ret_t ret;
-    rmw_request_id_t request_header;
-
-    // Initialize cancel request
-    auto request = std::make_shared<action_msgs::srv::CancelGoal::Request>();
-
-    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
-    ret = rcl_action_take_cancel_request(
-      pimpl_->action_server_.get(),
-      &request_header,
-      request.get());
-
-    return std::static_pointer_cast<void>(
-      std::make_shared
-      <std::tuple<rcl_ret_t, std::shared_ptr<action_msgs::srv::CancelGoal::Request>,
-      rmw_request_id_t>>(ret, request, request_header));
-  } else if (pimpl_->result_request_ready_.load()) {
-    rcl_ret_t ret;
-    // Get the result request message
-    rmw_request_id_t request_header;
-    std::shared_ptr<void> result_request = create_result_request();
-    std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
-    ret = rcl_action_take_result_request(
-      pimpl_->action_server_.get(), &request_header, result_request.get());
-
-    return std::static_pointer_cast<void>(
-      std::make_shared<std::tuple<rcl_ret_t, std::shared_ptr<void>, rmw_request_id_t>>(
-        ret, result_request, request_header));
-  } else if (pimpl_->goal_expired_.load()) {
+  if (next_ready_event == ServerBaseImpl::NO_EVENT_READY) {
+    // there is a known bug in iron, that take_data might be called multiple
+    // times. Therefore instead of throwing, we just return a nullptr as a workaround.
     return nullptr;
-  } else {
-    throw std::runtime_error("Taking data from action server but nothing is ready");
   }
+
+  return take_data_by_entity_id(next_ready_event);
 }
 
 std::shared_ptr<void>
 ServerBase::take_data_by_entity_id(size_t id)
 {
+  static_assert(
+    static_cast<size_t>(EntityType::GoalService) ==
+    static_cast<size_t>(ActionEventType::GoalService));
+  static_assert(
+    static_cast<size_t>(EntityType::ResultService) ==
+    static_cast<size_t>(ActionEventType::ResultService));
+  static_assert(
+    static_cast<size_t>(EntityType::CancelService) ==
+    static_cast<size_t>(ActionEventType::CancelService));
+
+  std::shared_ptr<ServerBaseData> data_ptr;
   // Mark as ready the entity from which we want to take data
-  switch (static_cast<EntityType>(id)) {
-    case EntityType::GoalService:
-      pimpl_->goal_request_ready_ = true;
+  switch (static_cast<ActionEventType>(id)) {
+    case ActionEventType::GoalService:
+      {
+        rcl_ret_t ret;
+        rcl_action_goal_info_t goal_info = rcl_action_get_zero_initialized_goal_info();
+        rmw_request_id_t request_header;
+
+        std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+
+        std::shared_ptr<void> message = create_goal_request();
+        ret = rcl_action_take_goal_request(
+          pimpl_->action_server_.get(),
+          &request_header,
+          message.get());
+
+        data_ptr = std::make_shared<ServerBaseData>(
+          ServerBaseData::GoalRequestData(ret, goal_info, request_header, message));
+      }
       break;
-    case EntityType::ResultService:
-      pimpl_->result_request_ready_ = true;
+    case ActionEventType::ResultService:
+      {
+        rcl_ret_t ret;
+        // Get the result request message
+        rmw_request_id_t request_header;
+        std::shared_ptr<void> result_request = create_result_request();
+        std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+        ret = rcl_action_take_result_request(
+          pimpl_->action_server_.get(), &request_header, result_request.get());
+
+        data_ptr =
+          std::make_shared<ServerBaseData>(
+          ServerBaseData::ResultRequestData(ret, result_request, request_header));
+      }
       break;
-    case EntityType::CancelService:
-      pimpl_->cancel_request_ready_ = true;
+    case ActionEventType::CancelService:
+      {
+        rcl_ret_t ret;
+        rmw_request_id_t request_header;
+
+        // Initialize cancel request
+        auto request = std::make_shared<action_msgs::srv::CancelGoal::Request>();
+
+        std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
+        ret = rcl_action_take_cancel_request(
+          pimpl_->action_server_.get(),
+          &request_header,
+          request.get());
+
+        data_ptr =
+          std::make_shared<ServerBaseData>(
+          ServerBaseData::CancelRequestData(ret, request, request_header));
+      }
+      break;
+    case ActionEventType::Expired:
+      {
+        data_ptr =
+          std::make_shared<ServerBaseData>(ServerBaseData::GoalExpiredData());
+      }
       break;
   }
 
-  return take_data();
+  return std::static_pointer_cast<void>(data_ptr);
 }
 
 void
-ServerBase::execute(std::shared_ptr<void> & data)
+ServerBase::execute(std::shared_ptr<void> & data_in)
 {
-  if (!data && !pimpl_->goal_expired_.load()) {
-    throw std::runtime_error("'data' is empty");
+  if (!data_in) {
+    // workaround, if take_data was called multiple timed, it returns a nullptr
+    // normally we should throw here, but as an API stable bug fix, we just ignore this...
+    return;
   }
 
-  if (pimpl_->goal_request_ready_.load()) {
-    execute_goal_request_received(data);
-  } else if (pimpl_->cancel_request_ready_.load()) {
-    execute_cancel_request_received(data);
-  } else if (pimpl_->result_request_ready_.load()) {
-    execute_result_request_received(data);
-  } else if (pimpl_->goal_expired_.load()) {
-    execute_check_expired_goals();
-  } else {
-    throw std::runtime_error("Executing action server but nothing is ready");
-  }
+  std::shared_ptr<ServerBaseData> data_ptr = std::static_pointer_cast<ServerBaseData>(data_in);
+
+  std::visit(
+    [&](auto && data) -> void {
+      using T = std::decay_t<decltype(data)>;
+      if constexpr (std::is_same_v<T, ServerBaseData::GoalRequestData>) {
+        execute_goal_request_received(data_in);
+      }
+      if constexpr (std::is_same_v<T, ServerBaseData::CancelRequestData>) {
+        execute_cancel_request_received(data_in);
+      }
+      if constexpr (std::is_same_v<T, ServerBaseData::ResultRequestData>) {
+        execute_result_request_received(data_in);
+      }
+      if constexpr (std::is_same_v<T, ServerBaseData::GoalExpiredData>) {
+        execute_check_expired_goals();
+      }
+    },
+    data_ptr->data);
 }
 
 void
 ServerBase::execute_goal_request_received(std::shared_ptr<void> & data)
 {
-  auto shared_ptr = std::static_pointer_cast
-    <std::tuple<rcl_ret_t, rcl_action_goal_info_t, rmw_request_id_t, std::shared_ptr<void>>>(data);
-  rcl_ret_t ret = std::get<0>(*shared_ptr);
+  std::shared_ptr<ServerBaseData> data_ptr = std::static_pointer_cast<ServerBaseData>(data);
+  const ServerBaseData::GoalRequestData & gData(
+    std::get<ServerBaseData::GoalRequestData>(data_ptr->data));
+
+  rcl_ret_t ret = std::get<0>(gData);
+  rcl_action_goal_info_t goal_info = std::get<1>(gData);
+  rmw_request_id_t request_header = std::get<2>(gData);
+  const std::shared_ptr<void> message = std::get<3>(gData);
+
   if (RCL_RET_ACTION_SERVER_TAKE_FAILED == ret) {
     // Ignore take failure because connext fails if it receives a sample without valid data.
     // This happens when a client shuts down and connext receives a sample saying the client is
@@ -320,14 +411,6 @@ ServerBase::execute_goal_request_received(std::shared_ptr<void> & data)
   } else if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret);
   }
-  rcl_action_goal_info_t goal_info = std::get<1>(*shared_ptr);
-  rmw_request_id_t request_header = std::get<2>(*shared_ptr);
-  std::shared_ptr<void> message = std::get<3>(*shared_ptr);
-
-  bool expected = true;
-  if (!pimpl_->goal_request_ready_.compare_exchange_strong(expected, false)) {
-    return;
-  }
 
   GoalUUID uuid = get_goal_id_from_goal_request(message.get());
   convert(uuid, &goal_info);
@@ -344,7 +427,16 @@ ServerBase::execute_goal_request_received(std::shared_ptr<void> & data)
   }
 
   if (RCL_RET_OK != ret) {
-    rclcpp::exceptions::throw_from_rcl_error(ret);
+    if (ret == RCL_RET_TIMEOUT) {
+      RCLCPP_WARN(
+        pimpl_->logger_,
+        "Failed to send goal response %s (timeout): %s",
+        to_string(uuid).c_str(), rcl_get_error_string().str);
+      rcl_reset_error();
+      return;
+    } else {
+      rclcpp::exceptions::throw_from_rcl_error(ret);
+    }
   }
 
   const auto status = response_pair.first;
@@ -402,10 +494,15 @@ ServerBase::execute_goal_request_received(std::shared_ptr<void> & data)
 void
 ServerBase::execute_cancel_request_received(std::shared_ptr<void> & data)
 {
-  auto shared_ptr = std::static_pointer_cast
-    <std::tuple<rcl_ret_t, std::shared_ptr<action_msgs::srv::CancelGoal::Request>,
-      rmw_request_id_t>>(data);
-  auto ret = std::get<0>(*shared_ptr);
+  std::shared_ptr<ServerBaseData> data_ptr = std::static_pointer_cast<ServerBaseData>(data);
+  const ServerBaseData::CancelRequestData & gData(
+    std::get<ServerBaseData::CancelRequestData>(data_ptr->data));
+
+  rcl_ret_t ret = std::get<0>(gData);
+  std::shared_ptr<action_msgs::srv::CancelGoal::Request> request = std::get<1>(gData);
+  rmw_request_id_t request_header = std::get<2>(gData);
+
+
   if (RCL_RET_ACTION_SERVER_TAKE_FAILED == ret) {
     // Ignore take failure because connext fails if it receives a sample without valid data.
     // This happens when a client shuts down and connext receives a sample saying the client is
@@ -414,9 +511,6 @@ ServerBase::execute_cancel_request_received(std::shared_ptr<void> & data)
   } else if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret);
   }
-  auto request = std::get<1>(*shared_ptr);
-  auto request_header = std::get<2>(*shared_ptr);
-  pimpl_->cancel_request_ready_ = false;
 
   // Convert c++ message to C message
   rcl_action_cancel_request_t cancel_request = rcl_action_get_zero_initialized_cancel_request();
@@ -483,6 +577,15 @@ ServerBase::execute_cancel_request_received(std::shared_ptr<void> & data)
       pimpl_->action_server_.get(), &request_header, response.get());
   }
 
+  if (ret == RCL_RET_TIMEOUT) {
+    GoalUUID uuid = request->goal_info.goal_id.uuid;
+    RCLCPP_WARN(
+      pimpl_->logger_,
+      "Failed to send cancel response %s (timeout): %s",
+      to_string(uuid).c_str(), rcl_get_error_string().str);
+    rcl_reset_error();
+    return;
+  }
   if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret);
   }
@@ -492,9 +595,14 @@ ServerBase::execute_cancel_request_received(std::shared_ptr<void> & data)
 void
 ServerBase::execute_result_request_received(std::shared_ptr<void> & data)
 {
-  auto shared_ptr = std::static_pointer_cast
-    <std::tuple<rcl_ret_t, std::shared_ptr<void>, rmw_request_id_t>>(data);
-  auto ret = std::get<0>(*shared_ptr);
+  std::shared_ptr<ServerBaseData> data_ptr = std::static_pointer_cast<ServerBaseData>(data);
+  const ServerBaseData::ResultRequestData & gData(
+    std::get<ServerBaseData::ResultRequestData>(data_ptr->data));
+
+  rcl_ret_t ret = std::get<0>(gData);
+  std::shared_ptr<void> result_request = std::get<1>(gData);
+  rmw_request_id_t request_header = std::get<2>(gData);
+
   if (RCL_RET_ACTION_SERVER_TAKE_FAILED == ret) {
     // Ignore take failure because connext fails if it receives a sample without valid data.
     // This happens when a client shuts down and connext receives a sample saying the client is
@@ -503,10 +611,7 @@ ServerBase::execute_result_request_received(std::shared_ptr<void> & data)
   } else if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret);
   }
-  auto result_request = std::get<1>(*shared_ptr);
-  auto request_header = std::get<2>(*shared_ptr);
 
-  pimpl_->result_request_ready_ = false;
   std::shared_ptr<void> result_response;
 
   // check if the goal exists
@@ -538,6 +643,14 @@ ServerBase::execute_result_request_received(std::shared_ptr<void> & data)
     std::lock_guard<std::recursive_mutex> lock(pimpl_->action_server_reentrant_mutex_);
     rcl_ret_t rcl_ret = rcl_action_send_result_response(
       pimpl_->action_server_.get(), &request_header, result_response.get());
+    if (rcl_ret == RCL_RET_TIMEOUT) {
+      RCLCPP_WARN(
+        pimpl_->logger_,
+        "Failed to send result response %s (timeout): %s",
+        to_string(uuid).c_str(), rcl_get_error_string().str);
+      rcl_reset_error();
+      return;
+    }
     if (RCL_RET_OK != rcl_ret) {
       rclcpp::exceptions::throw_from_rcl_error(rcl_ret);
     }
@@ -671,7 +784,13 @@ ServerBase::publish_result(const GoalUUID & uuid, std::shared_ptr<void> result_m
       for (auto & request_header : iter->second) {
         rcl_ret_t ret = rcl_action_send_result_response(
           pimpl_->action_server_.get(), &request_header, result_msg.get());
-        if (RCL_RET_OK != ret) {
+        if (ret == RCL_RET_TIMEOUT) {
+          RCLCPP_WARN(
+            pimpl_->logger_,
+            "Failed to send result response %s (timeout): %s",
+            to_string(uuid).c_str(), rcl_get_error_string().str);
+          rcl_reset_error();
+        } else if (RCL_RET_OK != ret) {
           rclcpp::exceptions::throw_from_rcl_error(ret);
         }
       }

rclcpp_components/CHANGELOG.rst

@@ -2,13 +2,32 @@
 Changelog for package rclcpp_components
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-21.2.0 (2023-06-07)
+21.0.8 (2024-11-08)
 -------------------
 
-21.1.1 (2023-05-11)
+21.0.7 (2024-07-10)
 -------------------
 
-21.1.0 (2023-04-27)
+21.0.6 (2024-04-19)
+-------------------
+
+21.0.5 (2024-02-07)
+-------------------
+* Increase the service queue sizes in component_container (`#2381 <https://github.com/ros2/rclcpp/issues/2381>`_)
+* Contributors: M. Fatih Cırıt
+
+21.0.4 (2023-11-17)
+-------------------
+* Add missing header required by the rclcpp::NodeOptions type (`#2325 <https://github.com/ros2/rclcpp/issues/2325>`_)
+* Contributors: Ignacio Vizzo
+
+21.0.3 (2023-09-08)
+-------------------
+
+21.0.2 (2023-07-14)
+-------------------
+
+21.0.1 (2023-05-11)
 -------------------
 
 21.0.0 (2023-04-18)

rclcpp_components/include/rclcpp_components/node_factory.hpp

@@ -15,6 +15,7 @@
 #ifndef RCLCPP_COMPONENTS__NODE_FACTORY_HPP__
 #define RCLCPP_COMPONENTS__NODE_FACTORY_HPP__
 
+#include "rclcpp/node_options.hpp"
 #include "rclcpp_components/node_instance_wrapper.hpp"
 
 namespace rclcpp_components

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>21.2.0</version>
+  <version>21.0.8</version>
   <description>Package containing tools for dynamically loadable components</description>
 
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>

rclcpp_components/src/component_manager.cpp

@@ -39,10 +39,12 @@ ComponentManager::ComponentManager(
 {
   loadNode_srv_ = create_service<LoadNode>(
     "~/_container/load_node",
-    std::bind(&ComponentManager::on_load_node, this, _1, _2, _3));
+    std::bind(&ComponentManager::on_load_node, this, _1, _2, _3),
+    rclcpp::ServicesQoS().keep_last(200));
   unloadNode_srv_ = create_service<UnloadNode>(
     "~/_container/unload_node",
-    std::bind(&ComponentManager::on_unload_node, this, _1, _2, _3));
+    std::bind(&ComponentManager::on_unload_node, this, _1, _2, _3),
+    rclcpp::ServicesQoS().keep_last(200));
   listNodes_srv_ = create_service<ListNodes>(
     "~/_container/list_nodes",
     std::bind(&ComponentManager::on_list_nodes, this, _1, _2, _3));

rclcpp_lifecycle/CHANGELOG.rst

@@ -3,13 +3,39 @@ Changelog for package rclcpp_lifecycle
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 
-21.2.0 (2023-06-07)
+21.0.8 (2024-11-08)
+-------------------
+* add logger level service to lifecycle node. (`#2288 <https://github.com/ros2/rclcpp/issues/2288>`_)
+* Contributors: Tomoya Fujita
+
+21.0.7 (2024-07-10)
+-------------------
+* revert call shutdown in LifecycleNode destructor (Iron) (`#2559 <https://github.com/ros2/rclcpp/issues/2559>`_)
+* lifecycle node dtor shutdown should be called only in primary state. (`#2543 <https://github.com/ros2/rclcpp/issues/2543>`_)
+* rclcpp::shutdown should not be called before LifecycleNode dtor. (`#2539 <https://github.com/ros2/rclcpp/issues/2539>`_)
+* Contributors: Tomoya Fujita
+
+21.0.6 (2024-04-19)
+-------------------
+* call shutdown in LifecycleNode dtor to avoid leaving the device in un… (`#2490 <https://github.com/ros2/rclcpp/issues/2490>`_)
+* Contributors: Tomoya Fujita
+
+21.0.5 (2024-02-07)
 -------------------
 
-21.1.1 (2023-05-11)
+21.0.4 (2023-11-17)
 -------------------
 
-21.1.0 (2023-04-27)
+21.0.3 (2023-09-08)
+-------------------
+* Switch lifecycle to use the RCLCPP macros. (`#2244 <https://github.com/ros2/rclcpp/issues/2244>`_)
+* Add new node interface TypeDescriptionsInterface to provide GetTypeDescription service (`#2236 <https://github.com/ros2/rclcpp/issues/2236>`_)
+* Contributors: Emerson Knapp
+
+21.0.2 (2023-07-14)
+-------------------
+
+21.0.1 (2023-05-11)
 -------------------
 
 21.0.0 (2023-04-18)

rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp

@@ -72,6 +72,7 @@
 #include "rclcpp/node_interfaces/node_time_source_interface.hpp"
 #include "rclcpp/node_interfaces/node_timers_interface.hpp"
 #include "rclcpp/node_interfaces/node_topics_interface.hpp"
+#include "rclcpp/node_interfaces/node_type_descriptions_interface.hpp"
 #include "rclcpp/node_interfaces/node_waitables_interface.hpp"
 #include "rclcpp/parameter.hpp"
 #include "rclcpp/publisher.hpp"
@@ -823,6 +824,14 @@ public:
   rclcpp::node_interfaces::NodeTimeSourceInterface::SharedPtr
   get_node_time_source_interface();
 
+  /// Return the Node's internal NodeTypeDescriptionsInterface implementation.
+  /**
+   * \sa rclcpp::Node::get_node_type_descriptions_interface
+   */
+  RCLCPP_LIFECYCLE_PUBLIC
+  rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
+  get_node_type_descriptions_interface();
+
   /// Return the Node's internal NodeWaitablesInterface implementation.
   /**
    * \sa rclcpp::Node::get_node_waitables_interface

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>21.2.0</version>
+  <version>21.0.8</version>
   <description>Package containing a prototype for lifecycle implementation</description>
 
   <maintainer email="ivanpauno@ekumenlabs.com">Ivan Paunovic</maintainer>

rclcpp_lifecycle/src/lifecycle_node.cpp

@@ -43,6 +43,7 @@
 #include "rclcpp/node_interfaces/node_time_source.hpp"
 #include "rclcpp/node_interfaces/node_timers.hpp"
 #include "rclcpp/node_interfaces/node_topics.hpp"
+#include "rclcpp/node_interfaces/node_type_descriptions.hpp"
 #include "rclcpp/node_interfaces/node_waitables.hpp"
 #include "rclcpp/parameter_service.hpp"
 #include "rclcpp/qos.hpp"
@@ -115,7 +116,11 @@ LifecycleNode::LifecycleNode(
     )),
   node_waitables_(new rclcpp::node_interfaces::NodeWaitables(node_base_.get())),
   node_options_(options),
-  impl_(new LifecycleNodeInterfaceImpl(node_base_, node_services_))
+  impl_(new LifecycleNodeInterfaceImpl(
+      node_base_,
+      node_logging_,
+      node_parameters_,
+      node_services_))
 {
   impl_->init(enable_communication_interface);
 
@@ -137,6 +142,10 @@ LifecycleNode::LifecycleNode(
       &LifecycleNodeInterface::on_deactivate, this,
       std::placeholders::_1));
   register_on_error(std::bind(&LifecycleNodeInterface::on_error, this, std::placeholders::_1));
+
+  if (options.enable_logger_service()) {
+    node_logging_->create_logger_services(node_services_);
+  }
 }
 
 LifecycleNode::~LifecycleNode()
@@ -474,6 +483,12 @@ LifecycleNode::get_node_services_interface()
   return node_services_;
 }
 
+rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
+LifecycleNode::get_node_type_descriptions_interface()
+{
+  return impl_->get_node_type_descriptions_interface();
+}
+
 rclcpp::node_interfaces::NodeParametersInterface::SharedPtr
 LifecycleNode::get_node_parameters_interface()
 {

rclcpp_lifecycle/src/lifecycle_node_interface_impl.cpp

@@ -29,7 +29,9 @@
 #include "lifecycle_msgs/srv/get_available_transitions.hpp"
 
 #include "rclcpp/node_interfaces/node_base_interface.hpp"
+#include "rclcpp/node_interfaces/node_logging_interface.hpp"
 #include "rclcpp/node_interfaces/node_services_interface.hpp"
+#include "rclcpp/node_interfaces/node_type_descriptions.hpp"
 
 #include "rclcpp_lifecycle/node_interfaces/lifecycle_node_interface.hpp"
 
@@ -50,9 +52,17 @@ namespace rclcpp_lifecycle
 
 LifecycleNode::LifecycleNodeInterfaceImpl::LifecycleNodeInterfaceImpl(
   std::shared_ptr<rclcpp::node_interfaces::NodeBaseInterface> node_base_interface,
+  std::shared_ptr<rclcpp::node_interfaces::NodeLoggingInterface> node_logging_interface,
+  std::shared_ptr<rclcpp::node_interfaces::NodeParametersInterface> node_parameters_interface,
   std::shared_ptr<rclcpp::node_interfaces::NodeServicesInterface> node_services_interface)
 : node_base_interface_(node_base_interface),
-  node_services_interface_(node_services_interface)
+  node_services_interface_(node_services_interface),
+  node_logging_interface_(node_logging_interface),
+  node_type_descriptions_(new rclcpp::node_interfaces::NodeTypeDescriptions(
+      node_base_interface,
+      node_logging_interface,
+      node_parameters_interface,
+      node_services_interface))
 {
 }
 
@@ -65,8 +75,8 @@ LifecycleNode::LifecycleNodeInterfaceImpl::~LifecycleNodeInterfaceImpl()
     ret = rcl_lifecycle_state_machine_fini(&state_machine_, node_handle);
   }
   if (ret != RCL_RET_OK) {
-    RCUTILS_LOG_FATAL_NAMED(
-      "rclcpp_lifecycle",
+    RCLCPP_FATAL(
+      node_logging_interface_->get_logger(),
       "failed to destroy rcl_state_machine");
   }
 }
@@ -398,7 +408,8 @@ LifecycleNode::LifecycleNodeInterfaceImpl::change_state(
   {
     std::lock_guard<std::recursive_mutex> lock(state_machine_mutex_);
     if (rcl_lifecycle_state_machine_is_initialized(&state_machine_) != RCL_RET_OK) {
-      RCUTILS_LOG_ERROR(
+      RCLCPP_ERROR(
+        node_logging_interface_->get_logger(),
         "Unable to change state for state machine for %s: %s",
         node_base_interface_->get_name(), rcl_get_error_string().str);
       return RCL_RET_ERROR;
@@ -411,7 +422,8 @@ LifecycleNode::LifecycleNodeInterfaceImpl::change_state(
       rcl_lifecycle_trigger_transition_by_id(
         &state_machine_, transition_id, publish_update) != RCL_RET_OK)
     {
-      RCUTILS_LOG_ERROR(
+      RCLCPP_ERROR(
+        node_logging_interface_->get_logger(),
         "Unable to start transition %u from current state %s: %s",
         transition_id, state_machine_.current_state->label, rcl_get_error_string().str);
       rcutils_reset_error();
@@ -443,7 +455,8 @@ LifecycleNode::LifecycleNodeInterfaceImpl::change_state(
       rcl_lifecycle_trigger_transition_by_label(
         &state_machine_, transition_label, publish_update) != RCL_RET_OK)
     {
-      RCUTILS_LOG_ERROR(
+      RCLCPP_ERROR(
+        node_logging_interface_->get_logger(),
         "Failed to finish transition %u. Current state is now: %s (%s)",
         transition_id, state_machine_.current_state->label, rcl_get_error_string().str);
       rcutils_reset_error();
@@ -458,7 +471,9 @@ LifecycleNode::LifecycleNodeInterfaceImpl::change_state(
   // error handling ?!
   // TODO(karsten1987): iterate over possible ret value
   if (cb_return_code == node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR) {
-    RCUTILS_LOG_WARN("Error occurred while doing error handling.");
+    RCLCPP_WARN(
+      node_logging_interface_->get_logger(),
+      "Error occurred while doing error handling.");
 
     auto error_cb_code = execute_callback(current_state_id, initial_state);
     auto error_cb_label = get_label_for_return_code(error_cb_code);
@@ -467,7 +482,9 @@ LifecycleNode::LifecycleNodeInterfaceImpl::change_state(
       rcl_lifecycle_trigger_transition_by_label(
         &state_machine_, error_cb_label, publish_update) != RCL_RET_OK)
     {
-      RCUTILS_LOG_ERROR("Failed to call cleanup on error state: %s", rcl_get_error_string().str);
+      RCLCPP_ERROR(
+        node_logging_interface_->get_logger(),
+        "Failed to call cleanup on error state: %s", rcl_get_error_string().str);
       rcutils_reset_error();
       return RCL_RET_ERROR;
     }
@@ -495,8 +512,12 @@ LifecycleNode::LifecycleNodeInterfaceImpl::execute_callback(
     try {
       cb_success = callback(State(previous_state));
     } catch (const std::exception & e) {
-      RCUTILS_LOG_ERROR("Caught exception in callback for transition %d", it->first);
-      RCUTILS_LOG_ERROR("Original error: %s", e.what());
+      RCLCPP_ERROR(
+        node_logging_interface_->get_logger(),
+        "Caught exception in callback for transition %d", it->first);
+      RCLCPP_ERROR(
+        node_logging_interface_->get_logger(),
+        "Original error: %s", e.what());
       cb_success = node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;
     }
   }
@@ -581,4 +602,10 @@ LifecycleNode::LifecycleNodeInterfaceImpl::on_deactivate() const
   }
 }
 
+rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
+LifecycleNode::LifecycleNodeInterfaceImpl::get_node_type_descriptions_interface()
+{
+  return node_type_descriptions_;
+}
+
 }  // namespace rclcpp_lifecycle

rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp

@@ -32,6 +32,7 @@
 
 #include "rclcpp/macros.hpp"
 #include "rclcpp/node_interfaces/node_base_interface.hpp"
+#include "rclcpp/node_interfaces/node_logging_interface.hpp"
 #include "rclcpp/node_interfaces/node_services_interface.hpp"
 
 #include "rclcpp_lifecycle/node_interfaces/lifecycle_node_interface.hpp"
@@ -52,6 +53,8 @@ class LifecycleNode::LifecycleNodeInterfaceImpl final
 public:
   LifecycleNodeInterfaceImpl(
     std::shared_ptr<rclcpp::node_interfaces::NodeBaseInterface> node_base_interface,
+    std::shared_ptr<rclcpp::node_interfaces::NodeLoggingInterface> node_logging_interface,
+    std::shared_ptr<rclcpp::node_interfaces::NodeParametersInterface> node_parameters_interface,
     std::shared_ptr<rclcpp::node_interfaces::NodeServicesInterface> node_services_interface);
 
   ~LifecycleNodeInterfaceImpl();
@@ -102,6 +105,9 @@ public:
   void
   add_timer_handle(std::shared_ptr<rclcpp::TimerBase> timer);
 
+  rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr
+  get_node_type_descriptions_interface();
+
 private:
   RCLCPP_DISABLE_COPY(LifecycleNodeInterfaceImpl)
 
@@ -152,6 +158,7 @@ private:
 
   using NodeBasePtr = std::shared_ptr<rclcpp::node_interfaces::NodeBaseInterface>;
   using NodeServicesPtr = std::shared_ptr<rclcpp::node_interfaces::NodeServicesInterface>;
+  using NodeLoggingPtr = std::shared_ptr<rclcpp::node_interfaces::NodeLoggingInterface>;
   using ChangeStateSrvPtr = std::shared_ptr<rclcpp::Service<ChangeStateSrv>>;
   using GetStateSrvPtr = std::shared_ptr<rclcpp::Service<GetStateSrv>>;
   using GetAvailableStatesSrvPtr =
@@ -163,6 +170,7 @@ private:
 
   NodeBasePtr node_base_interface_;
   NodeServicesPtr node_services_interface_;
+  NodeLoggingPtr node_logging_interface_;
   ChangeStateSrvPtr srv_change_state_;
   GetStateSrvPtr srv_get_state_;
   GetAvailableStatesSrvPtr srv_get_available_states_;
@@ -172,6 +180,9 @@ private:
   // to controllable things
   std::vector<std::weak_ptr<rclcpp_lifecycle::ManagedEntityInterface>> weak_managed_entities_;
   std::vector<std::weak_ptr<rclcpp::TimerBase>> weak_timers_;
+
+  // Backported members hidden in impl
+  rclcpp::node_interfaces::NodeTypeDescriptionsInterface::SharedPtr node_type_descriptions_;
 };
 
 }  // namespace rclcpp_lifecycle

rclcpp_lifecycle/test/test_lifecycle_node.cpp

@@ -25,6 +25,8 @@
 #include "lifecycle_msgs/msg/transition.hpp"
 
 #include "rcl_lifecycle/rcl_lifecycle.h"
+#include "rcl_interfaces/srv/get_logger_levels.hpp"
+#include "rcl_interfaces/srv/set_logger_levels.hpp"
 
 #include "rclcpp/rclcpp.hpp"
 #include "rclcpp_lifecycle/lifecycle_node.hpp"
@@ -34,6 +36,8 @@
 using lifecycle_msgs::msg::State;
 using lifecycle_msgs::msg::Transition;
 
+using namespace std::chrono_literals;
+
 static const std::chrono::nanoseconds DEFAULT_EVENT_TIMEOUT = std::chrono::seconds(3);
 static const std::chrono::nanoseconds DEFAULT_EVENT_SLEEP_PERIOD = std::chrono::milliseconds(100);
 
@@ -249,6 +253,35 @@ TEST_F(TestDefaultStateMachine, empty_initializer_rcl_errors) {
   }
 }
 
+TEST_F(TestDefaultStateMachine, check_logger_services_exist) {
+  // Logger level services are disabled
+  {
+    rclcpp::NodeOptions options = rclcpp::NodeOptions();
+    options.enable_logger_service(false);
+    auto node = std::make_shared<rclcpp_lifecycle::LifecycleNode>(
+      "test_logger_service", "/test", options);
+    auto get_client = node->create_client<rcl_interfaces::srv::GetLoggerLevels>(
+      "/test/test_logger_service/get_logger_levels");
+    ASSERT_FALSE(get_client->wait_for_service(2s));
+    auto set_client = node->create_client<rcl_interfaces::srv::SetLoggerLevels>(
+      "/test/test_logger_service/set_logger_levels");
+    ASSERT_FALSE(set_client->wait_for_service(2s));
+  }
+  // Logger level services are enabled
+  {
+    rclcpp::NodeOptions options = rclcpp::NodeOptions();
+    options.enable_logger_service(true);
+    auto node = std::make_shared<rclcpp_lifecycle::LifecycleNode>(
+      "test_logger_service", "/test", options);
+    auto get_client = node->create_client<rcl_interfaces::srv::GetLoggerLevels>(
+      "/test/test_logger_service/get_logger_levels");
+    ASSERT_TRUE(get_client->wait_for_service(2s));
+    auto set_client = node->create_client<rcl_interfaces::srv::SetLoggerLevels>(
+      "/test/test_logger_service/set_logger_levels");
+    ASSERT_TRUE(set_client->wait_for_service(2s));
+  }
+}
+
 TEST_F(TestDefaultStateMachine, trigger_transition) {
   auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
 
@@ -427,11 +460,15 @@ TEST_F(TestDefaultStateMachine, lifecycle_subscriber) {
 // Parameters are tested more thoroughly in rclcpp's test_node.cpp
 // These are provided for coverage of lifecycle node's API
 TEST_F(TestDefaultStateMachine, declare_parameters) {
+  // "start_type_description_service" and "use_sim_time"
+  const uint64_t builtin_param_count = 2;
+  const uint64_t expected_param_count = 6 + builtin_param_count;
   auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
 
   auto list_result = test_node->list_parameters({}, 0u);
-  EXPECT_EQ(list_result.names.size(), 1u);
-  EXPECT_STREQ(list_result.names[0].c_str(), "use_sim_time");
+  EXPECT_EQ(list_result.names.size(), builtin_param_count);
+  EXPECT_STREQ(list_result.names[0].c_str(), "start_type_description_service");
+  EXPECT_STREQ(list_result.names[1].c_str(), "use_sim_time");
 
   const std::string bool_name = "test_boolean";
   const std::string int_name = "test_int";
@@ -469,10 +506,11 @@ TEST_F(TestDefaultStateMachine, declare_parameters) {
   test_node->declare_parameters("test_double", double_parameters);
 
   list_result = test_node->list_parameters({}, 0u);
-  EXPECT_EQ(list_result.names.size(), 7u);
+  EXPECT_EQ(list_result.names.size(), expected_param_count);
 
   // The order of these names is not controlled by lifecycle_node, doing set equality
   std::set<std::string> expected_names = {
+    "start_type_description_service",
     "test_boolean",
     "test_double.double_one",
     "test_double.double_two",
@@ -487,11 +525,13 @@ TEST_F(TestDefaultStateMachine, declare_parameters) {
 }
 
 TEST_F(TestDefaultStateMachine, check_parameters) {
+  const uint64_t builtin_param_count = 2;
   auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
 
   auto list_result = test_node->list_parameters({}, 0u);
-  EXPECT_EQ(list_result.names.size(), 1u);
-  EXPECT_STREQ(list_result.names[0].c_str(), "use_sim_time");
+  EXPECT_EQ(list_result.names.size(), builtin_param_count);
+  EXPECT_STREQ(list_result.names[0].c_str(), "start_type_description_service");
+  EXPECT_STREQ(list_result.names[1].c_str(), "use_sim_time");
 
   const std::string bool_name = "test_boolean";
   const std::string int_name = "test_int";
@@ -549,8 +589,7 @@ TEST_F(TestDefaultStateMachine, check_parameters) {
   std::map<std::string, rclcpp::ParameterValue> parameter_map;
   EXPECT_TRUE(test_node->get_parameters({}, parameter_map));
 
-  // int param, bool param, and use_sim_time
-  EXPECT_EQ(parameter_map.size(), 3u);
+  EXPECT_EQ(parameter_map.size(), parameter_names.size() + builtin_param_count);
 
   // Check parameter types
   auto parameter_types = test_node->get_parameter_types(parameter_names);
@@ -585,10 +624,12 @@ TEST_F(TestDefaultStateMachine, check_parameters) {
 
   // List parameters
   list_result = test_node->list_parameters({}, 0u);
-  EXPECT_EQ(list_result.names.size(), 3u);
-  EXPECT_STREQ(list_result.names[0].c_str(), parameter_names[0].c_str());
-  EXPECT_STREQ(list_result.names[1].c_str(), parameter_names[1].c_str());
-  EXPECT_STREQ(list_result.names[2].c_str(), "use_sim_time");
+  EXPECT_EQ(list_result.names.size(), parameter_names.size() + builtin_param_count);
+  size_t index = 0;
+  EXPECT_STREQ(list_result.names[index++].c_str(), "start_type_description_service");
+  EXPECT_STREQ(list_result.names[index++].c_str(), parameter_names[0].c_str());
+  EXPECT_STREQ(list_result.names[index++].c_str(), parameter_names[1].c_str());
+  EXPECT_STREQ(list_result.names[index++].c_str(), "use_sim_time");
 
   // Undeclare parameter
   test_node->undeclare_parameter(bool_name);
@@ -633,6 +674,7 @@ TEST_F(TestDefaultStateMachine, test_getters) {
   EXPECT_LT(0u, test_node->now().nanoseconds());
   EXPECT_STREQ("testnode", test_node->get_logger().get_name());
   EXPECT_NE(nullptr, const_cast<const EmptyLifecycleNode *>(test_node.get())->get_clock());
+  EXPECT_NE(nullptr, test_node->get_node_type_descriptions_interface());
 }
 
 TEST_F(TestDefaultStateMachine, test_graph_topics) {

rclcpp_lifecycle/test/test_lifecycle_publisher.cpp

@@ -55,12 +55,6 @@ public:
   explicit EmptyLifecycleNode(const std::string & node_name, const TimerType & timer_type)
   : rclcpp_lifecycle::LifecycleNode(node_name)
   {
-    rclcpp::PublisherOptionsWithAllocator<std::allocator<void>> options;
-    publisher_ =
-      std::make_shared<rclcpp_lifecycle::LifecyclePublisher<test_msgs::msg::Empty>>(
-      get_node_base_interface().get(), std::string("topic"), rclcpp::QoS(10), options);
-    add_managed_entity(publisher_);
-
     // For coverage this is being added here
     switch (timer_type) {
       case TimerType::WALL_TIMER:
@@ -77,14 +71,6 @@ public:
         }
     }
   }
-
-  std::shared_ptr<rclcpp_lifecycle::LifecyclePublisher<test_msgs::msg::Empty>> publisher()
-  {
-    return publisher_;
-  }
-
-private:
-  std::shared_ptr<rclcpp_lifecycle::LifecyclePublisher<test_msgs::msg::Empty>> publisher_;
 };
 
 class TestLifecyclePublisher : public ::testing::TestWithParam<TimerType>
@@ -93,95 +79,103 @@ public:
   void SetUp()
   {
     rclcpp::init(0, nullptr);
-    node_ = std::make_shared<EmptyLifecycleNode>("node", GetParam());
   }
 
   void TearDown()
   {
     rclcpp::shutdown();
   }
-
-protected:
-  std::shared_ptr<EmptyLifecycleNode> node_;
 };
 
 TEST_P(TestLifecyclePublisher, publish_managed_by_node) {
+  auto node = std::make_shared<EmptyLifecycleNode>("node", GetParam());
+
+  rclcpp::PublisherOptionsWithAllocator<std::allocator<void>> options;
+  std::shared_ptr<rclcpp_lifecycle::LifecyclePublisher<test_msgs::msg::Empty>> publisher =
+    node->create_publisher<test_msgs::msg::Empty>(std::string("topic"), rclcpp::QoS(10), options);
+
   // transition via LifecycleNode
   auto success = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
   auto reset_key = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::ERROR;
   auto ret = reset_key;
 
-  EXPECT_EQ(State::PRIMARY_STATE_UNCONFIGURED, node_->get_current_state().id());
-  node_->trigger_transition(
+  EXPECT_EQ(State::PRIMARY_STATE_UNCONFIGURED, node->get_current_state().id());
+  node->trigger_transition(
     rclcpp_lifecycle::Transition(Transition::TRANSITION_CONFIGURE), ret);
   ASSERT_EQ(success, ret);
   ret = reset_key;
-  node_->trigger_transition(
+  node->trigger_transition(
     rclcpp_lifecycle::Transition(Transition::TRANSITION_ACTIVATE), ret);
   ASSERT_EQ(success, ret);
   ret = reset_key;
-  EXPECT_TRUE(node_->publisher()->is_activated());
+  EXPECT_TRUE(publisher->is_activated());
   {
     auto msg_ptr = std::make_unique<test_msgs::msg::Empty>();
-    EXPECT_NO_THROW(node_->publisher()->publish(*msg_ptr));
+    EXPECT_NO_THROW(publisher->publish(*msg_ptr));
   }
   {
     auto msg_ptr = std::make_unique<test_msgs::msg::Empty>();
-    EXPECT_NO_THROW(node_->publisher()->publish(std::move(msg_ptr)));
+    EXPECT_NO_THROW(publisher->publish(std::move(msg_ptr)));
   }
   {
-    auto loaned_msg = node_->publisher()->borrow_loaned_message();
-    EXPECT_NO_THROW(node_->publisher()->publish(std::move(loaned_msg)));
+    auto loaned_msg = publisher->borrow_loaned_message();
+    EXPECT_NO_THROW(publisher->publish(std::move(loaned_msg)));
   }
-  node_->trigger_transition(
+  node->trigger_transition(
     rclcpp_lifecycle::Transition(Transition::TRANSITION_DEACTIVATE), ret);
   ASSERT_EQ(success, ret);
   ret = reset_key;
   (void)ret;  // Just to make clang happy
-  EXPECT_FALSE(node_->publisher()->is_activated());
+  EXPECT_FALSE(publisher->is_activated());
   {
     auto msg_ptr = std::make_unique<test_msgs::msg::Empty>();
-    EXPECT_NO_THROW(node_->publisher()->publish(*msg_ptr));
+    EXPECT_NO_THROW(publisher->publish(*msg_ptr));
   }
   {
     auto msg_ptr = std::make_unique<test_msgs::msg::Empty>();
-    EXPECT_NO_THROW(node_->publisher()->publish(std::move(msg_ptr)));
+    EXPECT_NO_THROW(publisher->publish(std::move(msg_ptr)));
   }
   {
-    auto loaned_msg = node_->publisher()->borrow_loaned_message();
-    EXPECT_NO_THROW(node_->publisher()->publish(std::move(loaned_msg)));
+    auto loaned_msg = publisher->borrow_loaned_message();
+    EXPECT_NO_THROW(publisher->publish(std::move(loaned_msg)));
   }
 }
 
 TEST_P(TestLifecyclePublisher, publish) {
+  auto node = std::make_shared<EmptyLifecycleNode>("node", GetParam());
+
+  rclcpp::PublisherOptionsWithAllocator<std::allocator<void>> options;
+  std::shared_ptr<rclcpp_lifecycle::LifecyclePublisher<test_msgs::msg::Empty>> publisher =
+    node->create_publisher<test_msgs::msg::Empty>(std::string("topic"), rclcpp::QoS(10), options);
+
   // transition via LifecyclePublisher
-  node_->publisher()->on_deactivate();
-  EXPECT_FALSE(node_->publisher()->is_activated());
+  publisher->on_deactivate();
+  EXPECT_FALSE(publisher->is_activated());
   {
     auto msg_ptr = std::make_unique<test_msgs::msg::Empty>();
-    EXPECT_NO_THROW(node_->publisher()->publish(*msg_ptr));
+    EXPECT_NO_THROW(publisher->publish(*msg_ptr));
   }
   {
     auto msg_ptr = std::make_unique<test_msgs::msg::Empty>();
-    EXPECT_NO_THROW(node_->publisher()->publish(std::move(msg_ptr)));
+    EXPECT_NO_THROW(publisher->publish(std::move(msg_ptr)));
   }
   {
-    auto loaned_msg = node_->publisher()->borrow_loaned_message();
-    EXPECT_NO_THROW(node_->publisher()->publish(std::move(loaned_msg)));
+    auto loaned_msg = publisher->borrow_loaned_message();
+    EXPECT_NO_THROW(publisher->publish(std::move(loaned_msg)));
   }
-  node_->publisher()->on_activate();
-  EXPECT_TRUE(node_->publisher()->is_activated());
+  publisher->on_activate();
+  EXPECT_TRUE(publisher->is_activated());
   {
     auto msg_ptr = std::make_unique<test_msgs::msg::Empty>();
-    EXPECT_NO_THROW(node_->publisher()->publish(*msg_ptr));
+    EXPECT_NO_THROW(publisher->publish(*msg_ptr));
   }
   {
     auto msg_ptr = std::make_unique<test_msgs::msg::Empty>();
-    EXPECT_NO_THROW(node_->publisher()->publish(std::move(msg_ptr)));
+    EXPECT_NO_THROW(publisher->publish(std::move(msg_ptr)));
   }
   {
-    auto loaned_msg = node_->publisher()->borrow_loaned_message();
-    EXPECT_NO_THROW(node_->publisher()->publish(std::move(loaned_msg)));
+    auto loaned_msg = publisher->borrow_loaned_message();
+    EXPECT_NO_THROW(publisher->publish(std::move(loaned_msg)));
   }
 }