Open jmblixt3 opened 1 month ago
I suspect it's because the same executor (default executor) is being executed simultaneously on different threads.
I tried to use 2 executors (executor1 and default executor) and this issue doesn't occur.
fibonacci_action_hybrid.py.txt
Looking more closely looks like it isn't caused exclusively by calling a cpp action server and I've adjusted issue description and steps to reproduce
Looks like by doing something like this, can mitigate the issue, but feels really awkward. Here's a patch implementing this example.txt. Maybe this is just a fundamental limitation of nodes, but using different callback groups should be sufficient in my eyes anyway.
I suspect it's because the same executor (default executor) is being executed simultaneously on different threads.
This guess is incorrect.
The issue is likely related to using the default SingleThreadedExecutor.
I can't explain the root cause yet (It will take some time to pinpoint the cause.). I just tried switching to MultiThreadedExecutor and found that the problem didn't occur.
fibonacci_action_hybrid.py.txt
@jmblixt3
The root cause is related to rclpy.spin_until_future_complete().
rclpy.spin(node)
is to add node
to default executor.
The implementation of rclpy.spin_until_future_complete() is https://github.com/ros2/rclpy/blob/43198cbfe11c84480b15478f76d5fa3e998e5039/rclpy/rclpy/__init__.py#L324-L329
The last step is to remove node. After this step, no node is in executor. So no processing will be called at all (rclpy.spin()).
Change code as below
...
#rclpy.spin_until_future_complete(self,request_future)
rclpy.get_global_executor().spin_until_future_complete(request_future)
...
#rclpy.spin_until_future_complete(self,result_future)
rclpy.get_global_executor().spin_until_future_complete(result_future)
Would it be reasonable to check before adding the node if it is already in executor. If already present not remove it after completion?
Would it be reasonable to check before adding the node if it is already in executor. If already present not remove it after completion?
There won't be any duplicate Nodes added. Refer to line 271 https://github.com/ros2/rclpy/blob/43198cbfe11c84480b15478f76d5fa3e998e5039/rclpy/rclpy/executors.py#L263-L277
I understand it's not adding a duplicate. I was wondering if it would be appropriate to have the node not be removed from executor at the end if it was already spinning.
I was wondering if it would be appropriate to have the node not be removed from executor at the end if it was already spinning.
Spinning is the executor's responsibility. While spinning, the Nodes added to the executor can be changed.
The executor will recollect the entities from added nodes at an appropriate point.
Recollecting entities will occur after handling all the ready tasks returned by wait_set.wait().
I saw your PR #1316 and I understand your idea. I misunderstood earlier.
Bug report
Required Info:
Steps to reproduce issue
Utilizing demos repo with action tutorials_py and action_tutorials_cpp code Apply the following patch (its in a txt file because github issues don't like patches for some reason) patch
Then build and run the following nodes in separate terminals
or
and
Finally, run
Then run it again
Expected behavior
On the first execution of ros2 action send_goal we get the expected behavior of
And any subsequent calls to send_goals to some_action work exactly the same only varying Goal ID
Actual behavior
Any Subsequent calls actually result in
So the goal request is never processed by the action hybrid, after the first call.
Additional information
I initially found this when working on #1123, thinking there was a race condition within rclpy, but since the goal request is never getting received by the hybrid action. The default goal request processing is a simple return accepted leaving no place for potential race condition issues in that instance.