Fix object destruction order (#497)

Signed-off-by: Ivan Santiago Paunovic <[email protected]>

rclpy/CMakeLists.txt

@@ -1,7 +1,11 @@
 cmake_minimum_required(VERSION 3.5)
 
-project(rclpy C)
+project(rclpy)
 
+# Default to C++14
+if(NOT CMAKE_CXX_STANDARD)
+  set(CMAKE_CXX_STANDARD 14)
+endif()
 # Default to C11
 if(NOT CMAKE_C_STANDARD)
   set(CMAKE_C_STANDARD 11)
@@ -71,8 +75,9 @@ function(configure_python_c_extension_library _library_name)
   )
 endfunction()
 
-add_library(rclpy_common
-  SHARED src/rclpy_common/src/common.c
+add_library(rclpy_common SHARED
+  src/rclpy_common/src/common.c
+  src/rclpy_common/src/handle.c
 )
 target_link_libraries(rclpy_common
   ${PythonExtra_LIBRARIES}
@@ -142,6 +147,9 @@ add_library(
   rclpy_signal_handler
   SHARED src/rclpy/_rclpy_signal_handler.c
 )
+target_link_libraries(rclpy_signal_handler
+  rclpy_common
+)
 configure_python_c_extension_library(rclpy_signal_handler)
 ament_target_dependencies(rclpy_signal_handler
   "rcl"
@@ -155,6 +163,19 @@ add_library(
 )
 configure_python_c_extension_library(rclpy_pycapsule)
 
+# Handle library, used to keep dependencies between C objects.
+add_library(
+  rclpy_handle
+  SHARED src/rclpy/_rclpy_handle.c
+)
+target_link_libraries(rclpy_handle
+  rclpy_common
+)
+configure_python_c_extension_library(rclpy_handle)
+ament_target_dependencies(rclpy_handle
+  "rcutils"
+)
+
 if(NOT WIN32)
   ament_environment_hooks(
     "${ament_cmake_package_templates_ENVIRONMENT_HOOK_LIBRARY_PATH}"
@@ -167,6 +188,7 @@ if(BUILD_TESTING)
 
   find_package(ament_cmake_pytest REQUIRED)
   find_package(rosidl_generator_py REQUIRED)
+  find_package(ament_cmake_gtest REQUIRED)
 
   rosidl_generator_py_get_typesupports(_typesupport_impls)
   ament_index_get_prefix_path(ament_index_build_path SKIP_AMENT_PREFIX_PATH)
@@ -176,58 +198,63 @@ if(BUILD_TESTING)
     # On Windows prevent CMake errors and prevent it being evaluated as a list.
     string(REPLACE "\\" "/" ament_index_build_path "${ament_index_build_path}")
   endif()
-  if(NOT _typesupport_impls STREQUAL "")
 
-  # Run each test in its own pytest invocation to isolate any global state in rclpy
-  set(_rclpy_pytest_tests
-    test/test_action_client.py
-    test/test_action_graph.py
-    test/test_action_server.py
-    test/test_callback_group.py
-    test/test_client.py
-    test/test_clock.py
-    test/test_create_node.py
-    test/test_create_while_spinning.py
-    test/test_destruction.py
-    test/test_executor.py
-    test/test_expand_topic_name.py
-    test/test_guard_condition.py
-    test/test_handle.py
-    test/test_init_shutdown.py
-    test/test_logging.py
-    test/test_logging_rosout.py
-    test/test_messages.py
-    test/test_node.py
-    test/test_parameter.py
-    test/test_parameters_callback.py
-    test/test_qos.py
-    test/test_qos_event.py
-    test/test_rate.py
-    test/test_serialization.py
-    test/test_task.py
-    test/test_time_source.py
-    test/test_time.py
-    test/test_timer.py
-    test/test_topic_or_service_is_hidden.py
-    test/test_topic_endpoint_info.py
-    test/test_utilities.py
-    test/test_validate_full_topic_name.py
-    test/test_validate_namespace.py
-    test/test_validate_node_name.py
-    test/test_validate_topic_name.py
-    test/test_waitable.py
-  )
+  ament_add_gtest(test_c_handle
+    src/test/rclpy_common/test_handle.cpp)
+  target_link_libraries(test_c_handle
+    rclpy_common)
 
-  foreach(_test_path ${_rclpy_pytest_tests})
-    get_filename_component(_test_name ${_test_path} NAME_WE)
-    ament_add_pytest_test(${_test_name} ${_test_path}
-      PYTHON_EXECUTABLE "${PYTHON_EXECUTABLE}"
-      APPEND_ENV AMENT_PREFIX_PATH=${ament_index_build_path}
-        PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR}
-      TIMEOUT 120
-      WERROR ON
+  if(NOT _typesupport_impls STREQUAL "")
+    # Run each test in its own pytest invocation to isolate any global state in rclpy
+    set(_rclpy_pytest_tests
+      test/test_action_client.py
+      test/test_action_graph.py
+      test/test_action_server.py
+      test/test_callback_group.py
+      test/test_client.py
+      test/test_clock.py
+      test/test_create_node.py
+      test/test_create_while_spinning.py
+      test/test_destruction.py
+      test/test_executor.py
+      test/test_expand_topic_name.py
+      test/test_guard_condition.py
+      test/test_handle.py
+      test/test_init_shutdown.py
+      test/test_logging.py
+      test/test_logging_rosout.py
+      test/test_messages.py
+      test/test_node.py
+      test/test_parameter.py
+      test/test_parameters_callback.py
+      test/test_qos.py
+      test/test_qos_event.py
+      test/test_rate.py
+      test/test_serialization.py
+      test/test_task.py
+      test/test_time_source.py
+      test/test_time.py
+      test/test_timer.py
+      test/test_topic_or_service_is_hidden.py
+      test/test_topic_endpoint_info.py
+      test/test_utilities.py
+      test/test_validate_full_topic_name.py
+      test/test_validate_namespace.py
+      test/test_validate_node_name.py
+      test/test_validate_topic_name.py
+      test/test_waitable.py
     )
-  endforeach()
+
+    foreach(_test_path ${_rclpy_pytest_tests})
+      get_filename_component(_test_name ${_test_path} NAME_WE)
+      ament_add_pytest_test(${_test_name} ${_test_path}
+        PYTHON_EXECUTABLE "${PYTHON_EXECUTABLE}"
+        APPEND_ENV AMENT_PREFIX_PATH=${ament_index_build_path}
+          PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR}
+        TIMEOUT 120
+        WERROR ON
+      )
+    endforeach()
   endif()
 endif()
 set(PYTHON_EXECUTABLE "${_PYTHON_EXECUTABLE}")

rclpy/package.xml

@@ -28,6 +28,7 @@
   <exec_depend>rcl_interfaces</exec_depend>
   <exec_depend>rosgraph_msgs</exec_depend>
 
+  <test_depend>ament_cmake_gtest</test_depend>
   <test_depend>ament_cmake_pytest</test_depend>
   <test_depend>ament_lint_auto</test_depend>
   <test_depend>ament_lint_common</test_depend>

rclpy/rclpy/context.py

@@ -31,7 +31,8 @@ class Context:
 
     def __init__(self):
         from rclpy.impl.implementation_singleton import rclpy_implementation
-        self._handle = rclpy_implementation.rclpy_create_context()
+        from .handle import Handle
+        self._handle = Handle(rclpy_implementation.rclpy_create_context())
         self._lock = threading.Lock()
         self._callbacks = []
         self._callbacks_lock = threading.Lock()
@@ -48,16 +49,15 @@ def init(self, args: Optional[List[str]] = None):
         """
         # imported locally to avoid loading extensions on module import
         from rclpy.impl.implementation_singleton import rclpy_implementation
-        with self._lock:
-            rclpy_implementation.rclpy_init(
-                args if args is not None else sys.argv, self._handle)
+        with self._handle as capsule, self._lock:
+            rclpy_implementation.rclpy_init(args if args is not None else sys.argv, capsule)
 
     def ok(self):
         """Check if context hasn't been shut down."""
         # imported locally to avoid loading extensions on module import
         from rclpy.impl.implementation_singleton import rclpy_implementation
-        with self._lock:
-            return rclpy_implementation.rclpy_ok(self._handle)
+        with self._handle as capsule, self._lock:
+            return rclpy_implementation.rclpy_ok(capsule)
 
     def _call_on_shutdown_callbacks(self):
         with self._callbacks_lock:
@@ -70,17 +70,17 @@ def shutdown(self):
         """Shutdown this context."""
         # imported locally to avoid loading extensions on module import
         from rclpy.impl.implementation_singleton import rclpy_implementation
-        with self._lock:
-            rclpy_implementation.rclpy_shutdown(self._handle)
+        with self._handle as capsule, self._lock:
+            rclpy_implementation.rclpy_shutdown(capsule)
         self._call_on_shutdown_callbacks()
 
     def try_shutdown(self):
         """Shutdown this context, if not already shutdown."""
         # imported locally to avoid loading extensions on module import
         from rclpy.impl.implementation_singleton import rclpy_implementation
-        with self._lock:
-            if rclpy_implementation.rclpy_ok(self._handle):
-                rclpy_implementation.rclpy_shutdown(self._handle)
+        with self._handle as capsule, self._lock:
+            if rclpy_implementation.rclpy_ok(capsule):
+                rclpy_implementation.rclpy_shutdown(capsule)
                 self._call_on_shutdown_callbacks()
 
     def _remove_callback(self, weak_method):

rclpy/rclpy/executors.py

@@ -532,6 +532,7 @@ def _wait_for_ready_callbacks(
                     except InvalidHandle:
                         entity_count.num_guard_conditions -= 1
 
+                context_capsule = context_stack.enter_context(self._context.handle)
                 _rclpy.rclpy_wait_set_init(
                     wait_set,
                     entity_count.num_subscriptions,
@@ -540,7 +541,7 @@ def _wait_for_ready_callbacks(
                     entity_count.num_clients,
                     entity_count.num_services,
                     entity_count.num_events,
-                    self._context.handle)
+                    context_capsule)
 
                 _rclpy.rclpy_wait_set_clear_entities(wait_set)
                 for sub_capsule in sub_capsules:

rclpy/rclpy/guard_condition.py

@@ -21,7 +21,8 @@ class GuardCondition:
 
     def __init__(self, callback, callback_group, context=None):
         self._context = get_default_context() if context is None else context
-        self.__handle = Handle(_rclpy.rclpy_create_guard_condition(self._context.handle))
+        with self._context.handle as capsule:
+            self.__handle = Handle(_rclpy.rclpy_create_guard_condition(capsule))
         self.callback = callback
         self.callback_group = callback_group
         # True when the callback is ready to fire but has not been "taken" by an executor