Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the executor to not throw ExternalShutdownException #1279

Open
clalancette opened this issue May 7, 2024 · 2 comments
Open

Update the executor to not throw ExternalShutdownException #1279

clalancette opened this issue May 7, 2024 · 2 comments
Labels

Comments

@clalancette
Copy link
Contributor

This is an issue split out of ros2/examples#379

Currently the rclpy executor can throw an ExternalShutdownException:

raise ExternalShutdownException()

However, there is really no need to throw an exception at this point that callers of rclpy.spin would need to handle; we could instead just return a value indicating why the spin stopped. This would make it so users need less error-handling code, make this act more like rclcpp (which doesn't throw an exception here), and still provide information in case somebody wants to know why the spin stopped.

Further, we can easily stop throwing an exception here with no impact to downstream consumers. If they aren't handling the exception today, this will just make it a bit more robust. If they are handling ExternalShutdownException today, then that effectively becomes dead code but nothing really breaks.

@sbarber-dsi
Copy link

No.
This exception was useful because the multi-threaded executor does NOT shutdown cleanly so we can catch this exception and ignore it.
Now it throws a generic RuntimeError so ignoring it is unadvised.
Now we're checking strings to decide if we report an exception or not.

File ".../ros/node.py", line 433, in spin self._executor.spin() File "/opt/ros/humble/local/lib/python3.10/dist-packages/rclpy/executors.py", line 294, in spin self.spin_once() File "/opt/ros/humble/local/lib/python3.10/dist-packages/rclpy/executors.py", line 794, in spin_once self._spin_once_impl(timeout_sec) File "/opt/ros/humble/local/lib/python3.10/dist-packages/rclpy/executors.py", line 786, in _spin_once_impl self._executor.submit(handler) File "/usr/lib/python3.10/concurrent/futures/thread.py", line 167, in submit raise RuntimeError('cannot schedule new futures after shutdown') RuntimeError: cannot schedule new futures after shutdown

@clalancette
Copy link
Contributor Author

To be clear; this was an idea we had, never implemented. Whatever you are running into is a different issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants