-
Notifications
You must be signed in to change notification settings - Fork 263
Description
Bug report
Required Info:
- Operating System:
- N/A
- Installation type:
- N/A
- Version or commit hash:
- master and earlier
- DDS implementation:
- N/A
- Client library (if applicable):
- rclpy
Steps to reproduce issue
Create an rclpy.task.Future, and add a done callback to it. Set a result on the future so it becomes done. The done callback will never be called
import rclpy.task
fut = rclpy.task.Future()
fut.add_done_callback(lambda f: print('Future done!'))
fut.set_result(42)Expected behavior
Future done! should be printed to the console.
Actual behavior
Nothing is printed to the console.
Additional information
The future isn't associated with an executor, so Future._schedule_done_callbacks() just discards them 🙃
Lines 130 to 136 in 772115d
| def _schedule_done_callbacks(self): | |
| """Schedule done callbacks on the executor if possible.""" | |
| executor = self._executor() | |
| if executor is not None: | |
| for callback in self._callbacks: | |
| executor.create_task(callback, self) | |
| self._callbacks = [] |
The only future's who's done callbacks are called are ones that are known to the executor. This includes futures used for service clients and futures used for waitables. I forgot when using futures in tf2_ros, and as a result done callbacks on those futures are never called.
asyncio solves this problem by having a single global event loop that asyncio.future.Future can always get a reference to. This won't work for rclpy since there may be multiple executors, and there is no global reference to them. Even if there was a global reference, the future can't know which executor it will be associated with.
concurrent.futures solves this problem by calling the done callbacks at the point the future is completed. This means a done callback potentially block code completing the future, but the callbacks are always guaranteed to run.