ros-planning
`execute()` and `stop()` both call `spin_some()`, thus crash
Description
Despite #503 and #506, you cannot call MoveGroupInterface::stop() in a separate thread while MoveGroupInterface::execute() is running. This is related to thread-safety but I couldn't find information about thread-safety of MoveGroupInterface.
Your environment
- ROS Distro: Galactic
- OS Version: Ubuntu 20.04
- Source build, branch
main(419db3b3b0).
Steps to reproduce
Call move_group->execute(), then, before execute returns, call move_group->stop() in another thread.
Expected behaviour
The executable should not crash and the trajectory execution should stop.
Actual behaviour
The executable crashes with
terminate called after throwing an instance of 'std::runtime_error'
what(): Node has already been added to an executor.
Backtrace or Console output
Please note that I'm willing to fix this myself (in the range of my knowledge of MoveIt) but I don't know exactly which strategy to use. I even think that this should be solved in rclcpp to allow calling spin_some() concurrently.
@galou I think the MoveGroupInterface was never designed to be thread-safe, and I don't think there is a good way to make the full API work reliably across multiple threads. What you can do is to use MoveGroupInterface::asyncExecute() for your use case. However, I agree that spinning should not be implemented explicitly inside MGI, but removing all spin calls is a somewhat bigger effort. Would you be interested in working on that?
I can work on it but I have a question first. Why is there a call to spin_some() in MoveGroupInterface::stop()? Are there some callbacks involved? stop() just contains a call to publish().
My suggestion would be to add a std::atomic_bool is_spinning_ member variable and use it to conditionally call spin_some(). Would it be OK?
@galou #1305 should fix this bug. Could you verify and close this issue?