Revert "Declare rclcpp callbacks before the rcl entities (ros2#2024)"

This reverts commit 018eb9b.

rclcpp/include/rclcpp/client.hpp

@@ -363,16 +363,12 @@ class ClientBase
   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>

rclcpp/include/rclcpp/service.hpp

@@ -265,19 +265,15 @@ class ServiceBase
 
   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>

rclcpp/include/rclcpp/subscription_base.hpp

@@ -645,14 +645,6 @@ class SubscriptionBase : public std::enable_shared_from_this<SubscriptionBase>
   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 @@ class SubscriptionBase : public std::enable_shared_from_this<SubscriptionBase>
   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