rcl_shutdown is not thread-safe when used in both signal_handler and Context.__exit__.

[YuanYuYuan]

Bug report

We found this sporadic failure with rclpy (rolling) due to the race condition while calling rcl_shutdown.
In this issue, the conflict happens if rmw_shutdown is slow so that rcutils_atomic_store on Thread 1 is set after the check rcl_context_is_valid on Thread 2. Therefore rcl_shutdown would be called twice and cause an error.

Analysis

• Thread 1

  • rclpy::shutdown_contexts
  • rcl_shutdown
  • check rcl_context_is_valid
  • rmw_shutdown // Slow
  • rcutils_atomic_store

• Thread 2

  • InitContextManger.__exit__
  • _try_shutdown
  • g_default_context.try_shutdown()
  • if self.__context.ok()
  • Context::ok() -> rcl_context_is_valid // If this passes
  • Context::shutdown() -> rcl_shutdown // , then this will run

Required Info:

• Operating System:
  • Ubuntu 24.04
• Installation type:
  • Docker
• Version or commit hash:
  • ros-rolling
• DDS implementation:
  • This bug is RMW independent. Any RMW implementation with a slow rmw_shutdown could lead to this issue.
• Client library (if applicable):
  • Both rcl and rclpy are rolling.
• Relevant issue: Crash on shutdown: rcl_shutdown already called on the given context #1081

[Barry-Xu-2018]

After checking code, there is a issue.

Thread 1
Before calling rcl_shutdown(), g_contexts_mutex is locked.

rclpy/rclpy/src/rclpy/context.cpp

Lines 42 to 53 in 464a357

	namespace rclpy
	{
	void shutdown_contexts()
	{
	// graceful shutdown all contexts
	std::lock_guard<std::mutex> guard{g_contexts_mutex};
	for (auto * c : g_contexts) {
	rcl_ret_t ret = rcl_shutdown(c);
	(void)ret;
	}
	g_contexts.clear();
	}

Thread 2

Before calling rcl_shutdown(), g_contexts_mutex isn't used.

rclpy/rclpy/src/rclpy/context.cpp

Lines 150 to 165 in 464a357

	void
	Context::shutdown()
	{
	{
	std::lock_guard<std::mutex> guard{g_contexts_mutex};
	auto iter = std::find(g_contexts.begin(), g_contexts.end(), rcl_context_.get());
	if (iter != g_contexts.end()) {
	g_contexts.erase(iter);
	}
	}
	rcl_ret_t ret = rcl_shutdown(rcl_context_.get());
	if (RCL_RET_OK != ret) {
	throw RCLError("failed to shutdown");
	}
	}

If thread 2 executes rcl_shutdown first, thread 1 can still call rcl_shutdown again.
So rcl_shutdown should be safeguarded by g_contexts_mutex in Context::shutdown().

I create a fix #1353.
Could you test in your environment to see if the issue no longer occurs with this PR ?

[YuanYuYuan]

Hi @Barry-Xu-2018! Thanks for your attempt. But it seemed not to work...

[Barry-Xu-2018]

My fixing only prevented simultaneous calls to rcl_shutdown().
Another problem is that repeated calls to shutdown() cause an exception.

I have updated fixing. Please try again.

Currently, rclpy is designed to throw an exception if rcl_shutdown() is called multiple times on the same context (there's a specific test case for this). So, the error is expected behavior. However, in your situation, it shouldn't throw an error, but rather ignore the second shutdown call.
I will discuss with other members on how to fix this problem.

[YuanYuYuan]

Hi @Barry-Xu-2018, I have confirmed your latest fix resolves the issue (no more duplicated rcl_shutdown). Thanks!