ClientGoalHandle can throw in the destructor

Bug report

Required Info:

• Operating System:
  • Ubuntu 20.04
• Installation type:
  • Binary
• Version or commit hash:
  • Foxy ( 2.4.1-1focal.20220209.143946 )
• DDS implementation:
  • FastRTPS
• Client library (if applicable):
  • rclcpp

Steps to reproduce issue

1. Create an action server within a node
2. Spin the node using rclcpp::spin(node)
3. When receiving a new goal, store the goal handle inside the node
4. Shutdown the node while the goal is being executed

class MyActionServer : public rclcpp::Node {
public:
// Initialize action server here

void handle_accepted(const std::shared_ptr<MyGoalHandle> goal_handle)
{
    goal_handle_ = goal_handle;
    // Spin up a worker thread here probably
}

private:
    std::shared_ptr<MyGoalHandle> goal_handle_;
    std::shared_ptr<MyAction> server_;
}

int main()
{
    auto node = std::make_shared<MyActionServer>();
    rclcpp::spin(node):
    rclcpp::shutdown();
}

(Note : I can write a complete example code, but it's a lot of boilerplate to set up an action server...)

Expected behavior

The node shutdowns cleanly

Actual behavior

An exception is thrown in the destructor:

• The destructor will first call on_terminal_state
• on_terminal_state calls notify_goal_terminal_state
• Since rclcpp has already been shutdown, notify_goal_terminal_state will throw here

Additional information

Since I think it's undesirable to throw in the destructor, I would recommend to catch exceptions and log since not being able to notify the terminal state is a non-issue when destroying goal handles.

If storing GoalHandle is an undesired behavior, I wonder if there is any way to prevent the user from doing so?

Finally, the bug only appears if we destroy the goal handle before the action server. So this issue can be mitigated by managing manually the destruction ordering.

Start with rclcpp_action/include/rclcpp_action/server_goal_handle.hpp and follow on_terminal_state into rclcpp_action/include/rclcpp_action/server.hpp and rclcpp_action/src/server.cpp. Reproduce shutdown while a stored goal handle is active, then verify that destroying the handle after rclcpp shutdown no longer throws and the node shuts down cleanly.