Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix object destruction order #497

Merged
merged 11 commits into from
Mar 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 79 additions & 52 deletions rclpy/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
cmake_minimum_required(VERSION 3.5)

project(rclpy C)
project(rclpy)
ivanpauno marked this conversation as resolved.
Show resolved Hide resolved

# 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)
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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"
Expand All @@ -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}"
Expand All @@ -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)
Expand All @@ -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}")
Expand Down
1 change: 1 addition & 0 deletions rclpy/package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
22 changes: 11 additions & 11 deletions rclpy/rclpy/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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:
Expand All @@ -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):
Expand Down
3 changes: 2 additions & 1 deletion rclpy/rclpy/executors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion rclpy/rclpy/guard_condition.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading