ServerGoalHandle may throw from a destructor

Generated by Generative AI

No response

Operating System:

ubuntu 22

ROS version or commit hash:

humble

RMW implementation (if applicable):

No response

RMW Configuration (if applicable):

No response

Client library (if applicable):

rclcpp_action

'ros2 doctor --report' output

Steps to reproduce issue

Destructor of ServerGoalHandle may throw an exception from on_terminate_state callback https://github.com/ros2/rclcpp/blob/rolling/rclcpp_action/include/rclcpp_action/server_goal_handle.hpp#L250, callback is defined here https://github.com/ros2/rclcpp/blob/rolling/rclcpp_action/include/rclcpp_action/server.hpp#L511. Since destructors are nothrow in C++11 this results in termination of a program.

Expected behavior

destructors should not throw

Actual behavior

.

Additional information

PS: the goal should be aborted on destruction rather than canceled to match design spec

ABORTED - The goal was terminated by the action server without an external request.
CANCELED - The goal was canceled after an external request from an action client.

https://design.ros2.org/articles/actions.html

@asherikov

@asherikov thanks for creating follow-up issue and description.

i do agree with you that it should abort the goal instead of cancel. this is kind of surprise for action client, since it never requests the cancel.

Since destructors are nothrow in C++11 this results in termination of a program.

I am not quite following this line, can you rephrase this?
my understanding is that, destructors are not always nothrow in C++11.
They are only noexcept(true) if all subobjects’ destructors are also non-throwing.

Based on the current implementation, the lifecycle of the ServerGoalHandle is controlled by the user.

In Action Server, only a weak pointer to ServerGoalHandle is held.

rclcpp/rclcpp_action/include/rclcpp_action/server.hpp

Lines 602 to 605 in 82696f6

    
           using GoalHandleWeakPtr = std::weak_ptr<ServerGoalHandle<ActionT>>; 
        
           /// A map of goal id to goal handle weak pointers. 
        
           /// This is used to provide a goal handle to handle_cancel. 
        
           std::unordered_map<GoalUUID, GoalHandleWeakPtr> goal_handles_;

When the user's handle_goal callback function returns "accepted," the server will create a ServerGoalHandle and pass it as a parameter to the handle_accepted callback function registered by the user. That is, the user is responsible for managing the ServerGoalHandle.

rclcpp/rclcpp_action/include/rclcpp_action/server.hpp

Lines 545 to 552 in 82696f6

    
           goal_handle.reset( 
        
             new ServerGoalHandle<ActionT>( 
        
               rcl_goal_handle, uuid, goal, on_terminal_state, on_executing, publish_feedback)); 
        
           { 
        
             std::lock_guard<std::mutex> lock(goal_handles_mutex_); 
        
             goal_handles_[uuid] = goal_handle; 
        
           } 
        
           handle_accepted_(goal_handle);

It's easy to reproduce this issue by modifying fibonacci_action_server.cpp as follows.
rclcpp::Rate loop_rate(1); => rclcpp::Rate loop_rate(0.01);
https://github.com/ros2/demos/blob/472a4796bb838906a286c8c00260169873255e59/action_tutorials/action_tutorials_cpp/src/fibonacci_action_server.cpp#L148

I haven't come up with a good solution to change action server.
To ensure that the ServerGoalHandle is properly abort, my current idea is for the user to register a cleanup function through context::add_pre_shutdown_callback(), so that the ServerGoalHandle is correctly abort and the user process is aware that it is about to shut down before shutdown. Since ServerGoalHandle is abort, the destructor of ServerGoalHandle will do nothing.

my understanding is that, destructors are not always nothrow in C++11.

Yes, but destructors must be marked as noexcept(false) explicitly. Anyway my test get terminated, https://www.en.cppreference.com/w/cpp/language/destructor.html:

a destructor may terminate by throwing an exception (this usually requires it to be explicitly declared noexcept(false))(since C++11), however if this destructor happens to be called during stack unwinding, std::terminate is called instead.

In any case, the destructor should try-catch to be safe https://clang.llvm.org/extra/clang-tidy/checks/bugprone/exception-escape.html

PS: it seems that explicit destruction of action server before destruction of the goals prevents attempts to cancel the goals and can be used as a workaround.

@asherikov

@asherikov if you have time to take a look at ros2/rcl#1257 and #2953, that would be really appreciated.

@fujitatomoya

@fujitatomoya thank you, looks good

PR #2953 fixes a crash we are reproducing reliably on Jazzy.

A duplicate UUID is a specific trigger for the destructor-throw bug.

Can this please be backported to jazzy.

Navigation Menu

Repository navigation

ServerGoalHandle may throw from a destructor #2948

Generated by Generative AI

Operating System:

ROS version or commit hash:

RMW implementation (if applicable):

RMW Configuration (if applicable):

Client library (if applicable):

'ros2 doctor --report' output

Steps to reproduce issue

Expected behavior

Actual behavior

Additional information

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	using GoalHandleWeakPtr = std::weak_ptr<ServerGoalHandle<ActionT>>;
	/// A map of goal id to goal handle weak pointers.
	/// This is used to provide a goal handle to handle_cancel.
	std::unordered_map<GoalUUID, GoalHandleWeakPtr> goal_handles_;

	goal_handle.reset(
	new ServerGoalHandle<ActionT>(
	rcl_goal_handle, uuid, goal, on_terminal_state, on_executing, publish_feedback));
	{
	std::lock_guard<std::mutex> lock(goal_handles_mutex_);
	goal_handles_[uuid] = goal_handle;
	}
	handle_accepted_(goal_handle);

ServerGoalHandle may throw from a destructor #2948

Description

Generated by Generative AI

Operating System:

ROS version or commit hash:

RMW implementation (if applicable):

RMW Configuration (if applicable):

Client library (if applicable):

'ros2 doctor --report' output

Steps to reproduce issue

Expected behavior

Actual behavior

Additional information

Activity

fujitatomoya commented on Sep 11, 2025

Barry-Xu-2018 commented on Sep 11, 2025

asherikov commented on Sep 11, 2025

fujitatomoya commented on Sep 13, 2025

asherikov commented on Sep 15, 2025

Smit-tay commented on Apr 1, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions