Additional guard needed on Python single goal action server example?

[mrjogo]

I'm converting a ROS1 SimpleActionServer to ROS2, and I believe as written goal_handle.succeed() could be called on a cancelled goal (if a new goal is accepted just previously):

https://github.com/ros2/examples/blob/1d97c4fc7445554f6f85f63305d424fc017212a0/rclpy/actions/minimal_action_server/examples_rclpy_minimal_action_server/server_single_goal.py#L101-L109

Should this instead be:

with self._goal_lock:
  if not goal_handle.is_active:
    self.get_logger().info('Goal aborted')
    return Fibonacci.Result()

  goal_handle.succeed()
  
  # Populate result message
  result = Fibonacci.Result()
  result.sequence = feedback_msg.sequence
  
  self.get_logger().info('Returning result: {0}'.format(result.sequence))
  
  return result

If so, I can make a PR

[fujitatomoya]

i think so, there is a racy condition, and goal_handle could be invalid.

[audrow]

Sounds good, it'd be great if you make a PR for this!

[fujitatomoya]

@mrjogo really appreciate the PR, i posted a comment on that!