Fix data race in QOSEventHandlerBase

Both QOSEventHandler and its associated pubs/subs share
the same underlying rmw event listener.
When a pub/sub is destroyed, the listener is destroyed.
There is a data race when the ~QOSEventHandlerBase wants
to access the listener after it has been destroyed.

The QOSEventHandler stores a shared_ptr of its associated pub/sub.
But since we were clearing the listener event callbacks on the
base class destructor ~QOSEventHandlerBase, the pub/sub was
already destroyed, which means the rmw event listener was also
destroyed, thus causing a segfault when trying to obtain it
to clear the callbacks.

Clearing the callbacks on ~QOSEventHandler instead of
~QOSEventHandlerBase fixes the race, since the pub/sub are still valid.

Signed-off-by: Mauro Passerino <[email protected]>

Author: Mauro Passerino

rclcpp/include/rclcpp/qos_event.hpp

@@ -246,6 +246,16 @@ class QOSEventHandler : public QOSEventHandlerBase
     }
   }
 
+  ~QOSEventHandler()
+  {
+    // Since the rmw event listener holds a reference to the
+    // "on ready" callback, we need to clear it on destruction of this class.
+    // This clearing is not needed for other rclcpp entities like pub/subs, since
+    // they do own the underlying rmw entities, which are destroyed
+    // on their rclcpp destructors, thus no risk of dangling pointers.
+    clear_on_ready_callback();
+  }
+
   /// Take data so that the callback cannot be scheduled again
   std::shared_ptr<void>
   take_data() override

rclcpp/src/rclcpp/qos_event.cpp

@@ -35,13 +35,6 @@ UnsupportedEventTypeException::UnsupportedEventTypeException(
 
 QOSEventHandlerBase::~QOSEventHandlerBase()
 {
-  // Since the rmw event listener holds a reference to
-  // this callback, we need to clear it on destruction of this class.
-  // This clearing is not needed for other rclcpp entities like pub/subs, since
-  // they do own the underlying rmw entities, which are destroyed
-  // on their rclcpp destructors, thus no risk of dangling pointers.
-  clear_on_ready_callback();
-
   if (rcl_event_fini(&event_handle_) != RCL_RET_OK) {
     RCUTILS_LOG_ERROR_NAMED(
       "rclcpp",