Bugfix: executor doesn't propagate exception from task that awaited a future (backport #1643) - #1652
Bugfix: executor doesn't propagate exception from task that awaited a future (backport #1643)#1652mergify[bot] wants to merge 3 commits into
Conversation
… future (#1643) * Schedule the original task when task awaits a future Signed-off-by: Nadav Elkabets <elnadav12@gmail.com> * Add MultiThreadedExecutor to test Signed-off-by: Nadav Elkabets <elnadav12@gmail.com> * Add tests for awaiting a done future and task cancellation during await Signed-off-by: Nadav Elkabets <elnadav12@gmail.com> * Removed unused variable Signed-off-by: Nadav Elkabets <elnadav12@gmail.com> --------- Signed-off-by: Nadav Elkabets <elnadav12@gmail.com> (cherry picked from commit aac0ebb) # Conflicts: # rclpy/rclpy/task.py # rclpy/test/test_executor.py
|
Cherry-pick of aac0ebb has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
|
Pulls: #1652
|
|
Tick the box to add this pull request to the merge queue (same as
|
Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
|
|
Pulls: #1652
|
| else: | ||
| self._callbacks.append(task) | ||
|
|
||
| def remove_done_callback(self, callback: Callable[['Future[T]'], None]) -> bool: |
There was a problem hiding this comment.
The T TypeVar is not defined in jazzy, and caused flake8 to fail CI (it was added in this backport)
| def remove_done_callback(self, callback: Callable[['Future[T]'], None]) -> bool: | |
| def remove_done_callback(self, callback: Callable[['Future'], None]) -> bool: |
There was a problem hiding this comment.
With this fix the test suite is passing for me locally
|
Tick the box to add this pull request to the merge queue (same as
|
4 participants
Fixes #1642
Changes
A task now registers itself on the awaited future via new
Future._add_waiting_task.The executor then dispatches the same
Taskobject and sees its exception throughhandler.exception()as intended.Future._callbacksnow holdsUnion[Callable, Task]_schedule_or_invoke_done_callbackshandlesTaskentries throughexecutor._call_task_in_next_spin._wait_for_ready_callbacksskips tasks that were cancelled or completed between being queued and being popped.warnings.warnfires if a waitingTaskis dropped because the executor weakref could not be resolved.This is an automatic backport of pull request Bugfix: executor doesn't propagate exception from task that awaited a future #1643 done by Mergify.