Stop storing the context in the guard condition.

rclcpp/include/rclcpp/callback_group.hpp

@@ -129,16 +129,15 @@ class CallbackGroup
    * added to the executor in either case.
    *
    * \param[in] group_type The type of the callback group.
-   * \param[in] get_node_context Lambda to retrieve the node context when
-   *   checking that the creating node is valid and using the guard condition.
+   * \param[in] context A weak pointer to the context associated with this callback group.
    * \param[in] automatically_add_to_executor_with_node A boolean that
    *   determines whether a callback group is automatically added to an executor
    *   with the node with which it is associated.
    */
   RCLCPP_PUBLIC
   explicit CallbackGroup(
     CallbackGroupType group_type,
-    std::function<rclcpp::Context::SharedPtr(void)> get_node_context,
+    rclcpp::Context::WeakPtr context,
     bool automatically_add_to_executor_with_node = true);
 
   /// Default destructor.
@@ -228,16 +227,6 @@ class CallbackGroup
   bool
   automatically_add_to_executor_with_node() const;
 
-  /// Retrieve the guard condition used to signal changes to this callback group.
-  /**
-   * \param[in] context_ptr context to use when creating the guard condition
-   * \return guard condition if it is valid, otherwise nullptr.
-   */
-  [[deprecated("Use get_notify_guard_condition() without arguments")]]
-  RCLCPP_PUBLIC
-  rclcpp::GuardCondition::SharedPtr
-  get_notify_guard_condition(const rclcpp::Context::SharedPtr context_ptr);
-
   /// Retrieve the guard condition used to signal changes to this callback group.
   /**
    * \return guard condition if it is valid, otherwise nullptr.
@@ -297,7 +286,7 @@ class CallbackGroup
   std::shared_ptr<rclcpp::GuardCondition> notify_guard_condition_ = nullptr;
   std::recursive_mutex notify_guard_condition_mutex_;
 
-  std::function<rclcpp::Context::SharedPtr(void)> get_context_;
+  rclcpp::Context::WeakPtr context_;
 
 private:
   template<typename TypeT, typename Function>

rclcpp/include/rclcpp/guard_condition.hpp

@@ -48,7 +48,7 @@ class GuardCondition
    */
   RCLCPP_PUBLIC
   explicit GuardCondition(
-    rclcpp::Context::SharedPtr context =
+    const rclcpp::Context::SharedPtr & context =
     rclcpp::contexts::get_global_default_context(),
     rcl_guard_condition_options_t guard_condition_options =
     rcl_guard_condition_get_default_options());
@@ -57,11 +57,6 @@ class GuardCondition
   virtual
   ~GuardCondition();
 
-  /// Return the context used when creating this guard condition.
-  RCLCPP_PUBLIC
-  rclcpp::Context::SharedPtr
-  get_context() const;
-
   /// Return the underlying rcl guard condition structure.
   RCLCPP_PUBLIC
   rcl_guard_condition_t &
@@ -128,7 +123,6 @@ class GuardCondition
   set_on_trigger_callback(std::function<void(size_t)> callback);
 
 protected:
-  rclcpp::Context::SharedPtr context_;
   rcl_guard_condition_t rcl_guard_condition_;
   std::atomic<bool> in_use_by_wait_set_{false};
   std::recursive_mutex reentrant_mutex_;

rclcpp/src/rclcpp/callback_group.cpp

@@ -31,12 +31,12 @@ using rclcpp::CallbackGroupType;
 
 CallbackGroup::CallbackGroup(
   CallbackGroupType group_type,
-  std::function<rclcpp::Context::SharedPtr(void)> get_context,
+  rclcpp::Context::WeakPtr context,
   bool automatically_add_to_executor_with_node)
 : type_(group_type), associated_with_executor_(false),
   can_be_taken_from_(true),
   automatically_add_to_executor_with_node_(automatically_add_to_executor_with_node),
-  get_context_(get_context)
+  context_(context)
 {}
 
 CallbackGroup::~CallbackGroup()
@@ -123,40 +123,12 @@ CallbackGroup::automatically_add_to_executor_with_node() const
   return automatically_add_to_executor_with_node_;
 }
 
-// \TODO(mjcarroll) Deprecated, remove on tock
-rclcpp::GuardCondition::SharedPtr
-CallbackGroup::get_notify_guard_condition(const rclcpp::Context::SharedPtr context_ptr)
-{
-  std::lock_guard<std::recursive_mutex> lock(notify_guard_condition_mutex_);
-  if (notify_guard_condition_ && context_ptr != notify_guard_condition_->get_context()) {
-    if (associated_with_executor_) {
-      trigger_notify_guard_condition();
-    }
-    notify_guard_condition_ = nullptr;
-  }
-
-  if (!notify_guard_condition_) {
-    notify_guard_condition_ = std::make_shared<rclcpp::GuardCondition>(context_ptr);
-  }
-
-  return notify_guard_condition_;
-}
-
 rclcpp::GuardCondition::SharedPtr
 CallbackGroup::get_notify_guard_condition()
 {
   std::lock_guard<std::recursive_mutex> lock(notify_guard_condition_mutex_);
-  if (!this->get_context_) {
-    throw std::runtime_error("Callback group was created without context and not passed context");
-  }
-  auto context_ptr = this->get_context_();
+  rclcpp::Context::SharedPtr context_ptr = context_.lock();
   if (context_ptr && context_ptr->is_valid()) {
-    if (notify_guard_condition_ && context_ptr != notify_guard_condition_->get_context()) {
-      if (associated_with_executor_) {
-        trigger_notify_guard_condition();
-      }
-      notify_guard_condition_ = nullptr;
-    }
     if (!notify_guard_condition_) {
       notify_guard_condition_ = std::make_shared<rclcpp::GuardCondition>(context_ptr);
     }

rclcpp/src/rclcpp/guard_condition.cpp

@@ -23,16 +23,17 @@ namespace rclcpp
 {
 
 GuardCondition::GuardCondition(
-  rclcpp::Context::SharedPtr context,
+  const rclcpp::Context::SharedPtr & context,
   rcl_guard_condition_options_t guard_condition_options)
-: context_(context), rcl_guard_condition_{rcl_get_zero_initialized_guard_condition()}
+: rcl_guard_condition_{rcl_get_zero_initialized_guard_condition()}
 {
-  if (!context_) {
+  if (!context) {
     throw std::invalid_argument("context argument unexpectedly nullptr");
   }
+
   rcl_ret_t ret = rcl_guard_condition_init(
     &this->rcl_guard_condition_,
-    context_->get_rcl_context().get(),
+    context->get_rcl_context().get(),
     guard_condition_options);
   if (RCL_RET_OK != ret) {
     rclcpp::exceptions::throw_from_rcl_error(ret, "failed to create guard condition");
@@ -53,12 +54,6 @@ GuardCondition::~GuardCondition()
   }
 }
 
-rclcpp::Context::SharedPtr
-GuardCondition::get_context() const
-{
-  return context_;
-}
-
 rcl_guard_condition_t &
 GuardCondition::get_rcl_guard_condition()
 {

rclcpp/src/rclcpp/node_interfaces/node_base.cpp

@@ -218,14 +218,9 @@ NodeBase::create_callback_group(
   rclcpp::CallbackGroupType group_type,
   bool automatically_add_to_executor_with_node)
 {
-  auto weak_context = this->get_context()->weak_from_this();
-  auto get_node_context = [weak_context]() -> rclcpp::Context::SharedPtr {
-      return weak_context.lock();
-    };
-
   auto group = std::make_shared<rclcpp::CallbackGroup>(
     group_type,
-    get_node_context,
+    context_->weak_from_this(),
     automatically_add_to_executor_with_node);
   std::lock_guard<std::mutex> lock(callback_groups_mutex_);
   callback_groups_.push_back(group);

rclcpp/test/rclcpp/test_context.cpp

@@ -214,3 +214,25 @@ TEST(TestContext, check_on_shutdown_callback_order_after_del) {
 
   EXPECT_TRUE(result[0] == 1 && result[1] == 3 && result[2] == 4 && result[3] == 0);
 }
+
+// This test checks that contexts will be properly destroyed when leaving a scope, after a
+// guard condition has been created.
+TEST(TestContext, check_context_destroyed) {
+  rclcpp::Context::SharedPtr ctx;
+  {
+    ctx = std::make_shared<rclcpp::Context>();
+    ctx->init(0, nullptr);
+
+    auto group = std::make_shared<rclcpp::CallbackGroup>(
+      rclcpp::CallbackGroupType::MutuallyExclusive,
+      ctx->weak_from_this(),
+      false);
+
+    rclcpp::GuardCondition::SharedPtr gc = group->get_notify_guard_condition();
+    ASSERT_NE(gc, nullptr);
+
+    ASSERT_EQ(ctx.use_count(), 1u);
+  }
+
+  ASSERT_EQ(ctx.use_count(), 1u);
+}

rclcpp/test/rclcpp/test_guard_condition.cpp

@@ -66,16 +66,6 @@ TEST_F(TestGuardCondition, construction_and_destruction) {
   }
 }
 
-/*
- * Testing context accessor.
- */
-TEST_F(TestGuardCondition, get_context) {
-  {
-    auto gc = std::make_shared<rclcpp::GuardCondition>();
-    gc->get_context();
-  }
-}
-
 /*
  * Testing rcl guard condition accessor.
  */