Close cancelled coroutine #1394
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nadavelkabets
requested review from
sloretz and
adityapande-1995
as code owners
nadavelkabets
force-pushed
the
nadavelkabets/close-cancelled-coroutine
branch
from
057ec92 to
abc12b1
Compare
Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]>
Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]>
Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]>
nadavelkabets
force-pushed
the
nadavelkabets/close-cancelled-coroutine
branch
from
abc12b1 to
c52e58d
Compare
fujitatomoya
approved these changes
Jan 5, 2025
There was a problem hiding this comment.
@nadavelkabets really appreciate your effort with detailed description, lgtm.
fujitatomoya
merged commit 38e5ebb
into
ros2:fujitatomoya/check-task-canceled
2 checks passed
fujitatomoya
pushed a commit
that referenced
this pull request
Jan 5, 2025
* Add FutureState Signed-off-by: Nadav Elkabets <[email protected]> * Close canceled coroutine Signed-off-by: Nadav Elkabets <[email protected]> * Fixed behavior in test Signed-off-by: Nadav Elkabets <[email protected]> --------- Signed-off-by: Nadav Elkabets <[email protected]> Signed-off-by: Tomoya Fujita <[email protected]>
fujitatomoya
added a commit
that referenced
this pull request
Jan 15, 2025
* Check if Task(Future) is canceled. Signed-off-by: Tomoya Fujita <[email protected]> * Close cancelled coroutine (#1394) * Add FutureState Signed-off-by: Nadav Elkabets <[email protected]> * Close canceled coroutine Signed-off-by: Nadav Elkabets <[email protected]> * Fixed behavior in test Signed-off-by: Nadav Elkabets <[email protected]> --------- Signed-off-by: Nadav Elkabets <[email protected]> Signed-off-by: Tomoya Fujita <[email protected]> * address flake8 and pep257 failures. Signed-off-by: Tomoya Fujita <[email protected]> * Cancelled future is not done (#1397) * Remove redundant coro.close Signed-off-by: nadav <[email protected]> * Only finished future is done Signed-off-by: nadav <[email protected]> * Add function _pending and fix checks Signed-off-by: = <[email protected]> * Replace check in done from pending to finished Signed-off-by: = <[email protected]> * Adapt test to new behavior Signed-off-by: = <[email protected]> * Add tests Signed-off-by: = <[email protected]> * Make changes within active task mutex Signed-off-by: = <[email protected]> --------- Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]> * keep the consistent behavior to avoid exception, and adjusted some tests accordingly. Signed-off-by: Tomoya Fujita <[email protected]> * revert doc section to raise the exception. Signed-off-by: Tomoya Fujita <[email protected]> * remove StrEnum and put logical operator in the beginning of line. Signed-off-by: Tomoya Fujita <[email protected]> * add more test to check Task state. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]> Signed-off-by: Nadav Elkabets <[email protected]> Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]> Co-authored-by: Nadav Elkabets <[email protected]> Co-authored-by: Nadav Elkabets <[email protected]>
mergify bot
pushed a commit
that referenced
this pull request
Jan 17, 2025
* Check if Task(Future) is canceled. Signed-off-by: Tomoya Fujita <[email protected]> * Close cancelled coroutine (#1394) * Add FutureState Signed-off-by: Nadav Elkabets <[email protected]> * Close canceled coroutine Signed-off-by: Nadav Elkabets <[email protected]> * Fixed behavior in test Signed-off-by: Nadav Elkabets <[email protected]> --------- Signed-off-by: Nadav Elkabets <[email protected]> Signed-off-by: Tomoya Fujita <[email protected]> * address flake8 and pep257 failures. Signed-off-by: Tomoya Fujita <[email protected]> * Cancelled future is not done (#1397) * Remove redundant coro.close Signed-off-by: nadav <[email protected]> * Only finished future is done Signed-off-by: nadav <[email protected]> * Add function _pending and fix checks Signed-off-by: = <[email protected]> * Replace check in done from pending to finished Signed-off-by: = <[email protected]> * Adapt test to new behavior Signed-off-by: = <[email protected]> * Add tests Signed-off-by: = <[email protected]> * Make changes within active task mutex Signed-off-by: = <[email protected]> --------- Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]> * keep the consistent behavior to avoid exception, and adjusted some tests accordingly. Signed-off-by: Tomoya Fujita <[email protected]> * revert doc section to raise the exception. Signed-off-by: Tomoya Fujita <[email protected]> * remove StrEnum and put logical operator in the beginning of line. Signed-off-by: Tomoya Fujita <[email protected]> * add more test to check Task state. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]> Signed-off-by: Nadav Elkabets <[email protected]> Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]> Co-authored-by: Nadav Elkabets <[email protected]> Co-authored-by: Nadav Elkabets <[email protected]> (cherry picked from commit 9a144bf) # Conflicts: # rclpy/rclpy/executors.py # rclpy/rclpy/task.py # rclpy/test/test_executor.py
mergify bot
pushed a commit
that referenced
this pull request
Jan 17, 2025
* Check if Task(Future) is canceled. Signed-off-by: Tomoya Fujita <[email protected]> * Close cancelled coroutine (#1394) * Add FutureState Signed-off-by: Nadav Elkabets <[email protected]> * Close canceled coroutine Signed-off-by: Nadav Elkabets <[email protected]> * Fixed behavior in test Signed-off-by: Nadav Elkabets <[email protected]> --------- Signed-off-by: Nadav Elkabets <[email protected]> Signed-off-by: Tomoya Fujita <[email protected]> * address flake8 and pep257 failures. Signed-off-by: Tomoya Fujita <[email protected]> * Cancelled future is not done (#1397) * Remove redundant coro.close Signed-off-by: nadav <[email protected]> * Only finished future is done Signed-off-by: nadav <[email protected]> * Add function _pending and fix checks Signed-off-by: = <[email protected]> * Replace check in done from pending to finished Signed-off-by: = <[email protected]> * Adapt test to new behavior Signed-off-by: = <[email protected]> * Add tests Signed-off-by: = <[email protected]> * Make changes within active task mutex Signed-off-by: = <[email protected]> --------- Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]> * keep the consistent behavior to avoid exception, and adjusted some tests accordingly. Signed-off-by: Tomoya Fujita <[email protected]> * revert doc section to raise the exception. Signed-off-by: Tomoya Fujita <[email protected]> * remove StrEnum and put logical operator in the beginning of line. Signed-off-by: Tomoya Fujita <[email protected]> * add more test to check Task state. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]> Signed-off-by: Nadav Elkabets <[email protected]> Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]> Co-authored-by: Nadav Elkabets <[email protected]> Co-authored-by: Nadav Elkabets <[email protected]> (cherry picked from commit 9a144bf) # Conflicts: # rclpy/rclpy/executors.py # rclpy/rclpy/task.py # rclpy/test/test_executor.py
fujitatomoya
added a commit
that referenced
this pull request
Jan 22, 2025
* Check if Task(Future) is canceled. (#1377) * Check if Task(Future) is canceled. Signed-off-by: Tomoya Fujita <[email protected]> * Close cancelled coroutine (#1394) * Add FutureState Signed-off-by: Nadav Elkabets <[email protected]> * Close canceled coroutine Signed-off-by: Nadav Elkabets <[email protected]> * Fixed behavior in test Signed-off-by: Nadav Elkabets <[email protected]> --------- Signed-off-by: Nadav Elkabets <[email protected]> Signed-off-by: Tomoya Fujita <[email protected]> * address flake8 and pep257 failures. Signed-off-by: Tomoya Fujita <[email protected]> * Cancelled future is not done (#1397) * Remove redundant coro.close Signed-off-by: nadav <[email protected]> * Only finished future is done Signed-off-by: nadav <[email protected]> * Add function _pending and fix checks Signed-off-by: = <[email protected]> * Replace check in done from pending to finished Signed-off-by: = <[email protected]> * Adapt test to new behavior Signed-off-by: = <[email protected]> * Add tests Signed-off-by: = <[email protected]> * Make changes within active task mutex Signed-off-by: = <[email protected]> --------- Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]> * keep the consistent behavior to avoid exception, and adjusted some tests accordingly. Signed-off-by: Tomoya Fujita <[email protected]> * revert doc section to raise the exception. Signed-off-by: Tomoya Fujita <[email protected]> * remove StrEnum and put logical operator in the beginning of line. Signed-off-by: Tomoya Fujita <[email protected]> * add more test to check Task state. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]> Signed-off-by: Nadav Elkabets <[email protected]> Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]> Co-authored-by: Nadav Elkabets <[email protected]> Co-authored-by: Nadav Elkabets <[email protected]> (cherry picked from commit 9a144bf) # Conflicts: # rclpy/rclpy/executors.py # rclpy/rclpy/task.py # rclpy/test/test_executor.py * resolve conflicts. Signed-off-by: Tomoya Fujita <[email protected]> * _spin_once_until_future_complete does not exist. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]> Co-authored-by: Tomoya Fujita <[email protected]>
fujitatomoya
added a commit
that referenced
this pull request
Jan 22, 2025
* Check if Task(Future) is canceled. (#1377) * Check if Task(Future) is canceled. Signed-off-by: Tomoya Fujita <[email protected]> * Close cancelled coroutine (#1394) * Add FutureState Signed-off-by: Nadav Elkabets <[email protected]> * Close canceled coroutine Signed-off-by: Nadav Elkabets <[email protected]> * Fixed behavior in test Signed-off-by: Nadav Elkabets <[email protected]> --------- Signed-off-by: Nadav Elkabets <[email protected]> Signed-off-by: Tomoya Fujita <[email protected]> * address flake8 and pep257 failures. Signed-off-by: Tomoya Fujita <[email protected]> * Cancelled future is not done (#1397) * Remove redundant coro.close Signed-off-by: nadav <[email protected]> * Only finished future is done Signed-off-by: nadav <[email protected]> * Add function _pending and fix checks Signed-off-by: = <[email protected]> * Replace check in done from pending to finished Signed-off-by: = <[email protected]> * Adapt test to new behavior Signed-off-by: = <[email protected]> * Add tests Signed-off-by: = <[email protected]> * Make changes within active task mutex Signed-off-by: = <[email protected]> --------- Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]> * keep the consistent behavior to avoid exception, and adjusted some tests accordingly. Signed-off-by: Tomoya Fujita <[email protected]> * revert doc section to raise the exception. Signed-off-by: Tomoya Fujita <[email protected]> * remove StrEnum and put logical operator in the beginning of line. Signed-off-by: Tomoya Fujita <[email protected]> * add more test to check Task state. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]> Signed-off-by: Nadav Elkabets <[email protected]> Signed-off-by: nadav <[email protected]> Signed-off-by: = <[email protected]> Co-authored-by: Nadav Elkabets <[email protected]> Co-authored-by: Nadav Elkabets <[email protected]> (cherry picked from commit 9a144bf) # Conflicts: # rclpy/rclpy/executors.py # rclpy/rclpy/task.py # rclpy/test/test_executor.py * resolve conflicts. Signed-off-by: Tomoya Fujita <[email protected]> * _spin_once_until_future_complete does not exist. Signed-off-by: Tomoya Fujita <[email protected]> --------- Signed-off-by: Tomoya Fujita <[email protected]> Co-authored-by: Tomoya Fujita <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixed the issue in PR #1377 where cancelled coroutines are not closed and implemented a FutureState similar to asyncio.
The failed test is now passing.