Skip to content

Fix loaned message ownership transfer by returning loans via shared_ptr custom deleter - #3280

Open
Aaravanand00 wants to merge 1 commit into
ros2:rollingfrom
Aaravanand00:fix-loaned-message-ownership
Open

Aaravanand00 wants to merge 1 commit into
ros2:rollingfrom
Aaravanand00:fix-loaned-message-ownership

Conversation

@Aaravanand00

Copy link
Copy Markdown

Description

This PR implements the proper transfer of ownership between the RMW and the rcl layer for loaned messages, resolving a safety gap where loaned memory could be overwritten by the middleware if a user stored the shared_ptr for asynchronous processing.

Previously, rclcpp::Executor::execute_subscription unconditionally returned the loaned message immediately after the callback execution. With this change, the rcl_return_loaned_message_from_subscription is now tied to the destructor of the shared_ptr provided to the user.

  • Created a custom deleter inside Subscription::handle_loaned_message which captures the rcl_subscription_t and safely returns the loaned message when the reference count drops to zero.
  • Added a std::mutex (loaned_message_mutex_) in SubscriptionBase to ensure thread-safe execution of rcl_return and rcl_take.
  • Removed the immediate rcl_return from executor.cpp.
  • Removed the obsolete RCLCPP_WARN_ONCE about shared_ptr callbacks being unsafe for loaned messages.

Fixes #3119

Is this user-facing behavior change?

Yes, advanced users relying on zero-copy transport can now safely retain a std::shared_ptr to the loaned message. The underlying RMW memory is not returned to the pool until all references to the pointer are released.

Did you use Generative AI?

Yes, I used Antigravity (Google DeepMind agentic coding assistant) to help implement the shared_ptr custom deleter pattern and the thread-safe mutex logic across subscription_base.hpp/cpp, subscription.hpp, and executor.cpp.

Additional Information

This fix is based on the feedback from @jmachowinski to properly transfer ownership back and forth instead of introducing a new is_data_valid API. It also addresses the TODO(clalancette) in subscription_base.cpp about handling this with a custom deleter and proper locking.

Signed-off-by: Aaravanand <aaravanand@gmail.com>
@Aaravanand00
Aaravanand00 force-pushed the fix-loaned-message-ownership branch from 5a81776 to fa15b1a Compare September 20, 2026 08:13
@Aaravanand00

Copy link
Copy Markdown
Author

@fujitatomoya ptal...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proposal: Expose is_data_valid API for Loaned/Zero-Copy Messages

1 participant