Skip to content

Commit 24f059c

Browse files
authored
Disable the loaned messages inside the executor. (backport #2335) (#2364)
* Disable the loaned messages inside the executor. (#2335) * Disable the loaned messages inside the executor. They are currently unsafe to use; see the comment in the commit for more information. Signed-off-by: Chris Lalancette <[email protected]> (cherry picked from commit f294488)
1 parent c1bf0d3 commit 24f059c

File tree

2 files changed

+19
-1
lines changed

2 files changed

+19
-1
lines changed

rclcpp/src/rclcpp/executor.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,11 @@ Executor::execute_subscription(rclcpp::SubscriptionBase::SharedPtr subscription)
646646
subscription->get_topic_name(),
647647
[&]() {return subscription->take_type_erased(message.get(), message_info);},
648648
[&]() {subscription->handle_message(message, message_info);});
649+
// TODO(clalancette): In the case that the user is using the MessageMemoryPool,
650+
// and they take a shared_ptr reference to the message in the callback, this can
651+
// inadvertently return the message to the pool when the user is still using it.
652+
// This is a bug that needs to be fixed in the pool, and we should probably have
653+
// a custom deleter for the message that actually does the return_message().
649654
subscription->return_message(message);
650655
}
651656
}

rclcpp/src/rclcpp/subscription_base.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,20 @@ SubscriptionBase::setup_intra_process(
229229
bool
230230
SubscriptionBase::can_loan_messages() const
231231
{
232-
return rcl_subscription_can_loan_messages(subscription_handle_.get());
232+
bool retval = rcl_subscription_can_loan_messages(subscription_handle_.get());
233+
if (retval) {
234+
// TODO(clalancette): The loaned message interface is currently not safe to use with
235+
// shared_ptr callbacks. If a user takes a copy of the shared_ptr, it can get freed from
236+
// underneath them via rcl_return_loaned_message_from_subscription(). The correct solution is
237+
// to return the loaned message in a custom deleter, but that needs to be carefully handled
238+
// with locking. Warn the user about this until we fix it.
239+
RCLCPP_WARN_ONCE(
240+
this->node_logger_,
241+
"Loaned messages are only safe with const ref subscription callbacks. "
242+
"If you are using any other kind of subscriptions, "
243+
"set the ROS_DISABLE_LOANED_MESSAGES environment variable to 1 (the default).");
244+
}
245+
return retval;
233246
}
234247

235248
rclcpp::Waitable::SharedPtr

0 commit comments

Comments
 (0)