From e52bb9dd1f8f6d33e3beef960b92ad1ecdedfa57 Mon Sep 17 00:00:00 2001 From: Hans-Joachim Krauch Date: Mon, 23 Oct 2023 20:48:40 -0300 Subject: [PATCH] Fix callback accessing invalid reference to promise (#268) ### Public-Facing Changes Fix callback accessing invalid reference to promise ### Description For ROS1, the service request and response types can only be retrieved by opening a connection to the service server. For this, we use `ros::ServiceManager::instance()->createServiceServerLink` which under the hood keeps a copy of the returned pointer, effectively keeping the connection alive. This patch makes sure that we drop the connection when we either have received the header or when the timeout has been exceeded. This avoids that the header-received callback is being called with an invalid reference to the promise. Fixes #267 Resolves FG-5366 --- ros1_foxglove_bridge/src/service_utils.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ros1_foxglove_bridge/src/service_utils.cpp b/ros1_foxglove_bridge/src/service_utils.cpp index 0403ae0..0cc47ce 100644 --- a/ros1_foxglove_bridge/src/service_utils.cpp +++ b/ros1_foxglove_bridge/src/service_utils.cpp @@ -22,7 +22,7 @@ std::string retrieveServiceType(const std::string& serviceName, std::chrono::mil auto future = promise.get_future(); link->getConnection()->setHeaderReceivedCallback( - [&promise](const ros::ConnectionPtr&, const ros::Header& header) { + [&promise](const ros::ConnectionPtr& conn, const ros::Header& header) { std::string serviceType; if (header.getValue("type", serviceType)) { promise.set_value(serviceType); @@ -30,10 +30,15 @@ std::string retrieveServiceType(const std::string& serviceName, std::chrono::mil promise.set_exception(std::make_exception_ptr( std::runtime_error("Key 'type' not found in service connection header"))); } + // Close connection since we don't need it any more. + conn->drop(ros::Connection::DropReason::Destructing); return true; }); if (future.wait_for(timeout) != std::future_status::ready) { + // Drop connection here, removes the link from the service manager instance and avoids + // that the header-received callback is called after the promise has already been destroyed. + link->getConnection()->drop(ros::Connection::DropReason::Destructing); throw std::runtime_error("Timed out when retrieving service type"); }