Skip to content

bpo-32684: Fix gather to propagate cancel of itself with return_exceptions #7209

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 29, 2018
Merged

bpo-32684: Fix gather to propagate cancel of itself with return_exceptions #7209

merged 2 commits into from
May 29, 2018

Conversation

1st1
Copy link
Member

@1st1 1st1 commented May 29, 2018 •
edited by bedevere-bot
Loading

@1st1 1st1 requested a review from asvetlov as a code owner May 29, 2018 16:52
asvetlov
asvetlov reviewed May 29, 2018
View reviewed changes
Lib/asyncio/tasks.py
@@ -599,6 +600,11 @@ def cancel(self):
for child in self._children:
if child.cancel():
ret = True
if ret:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow why future cancellation should be propagated but if children are tasks nothing should be done.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://bugs.python.org/issue26923 and, specifically, Task.cancel(): if the task is done/cancelled, we don't raise CancelledError. Similarly for gather(), if all of its tasks are already done, we don't raise it.

This seems to be a super rare edge case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it.

asvetlov
asvetlov approved these changes May 29, 2018
View reviewed changes
Copy link
Contributor

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@1st1 1st1 merged commit 863b674 into python:master May 29, 2018
@miss-islington
Copy link
Contributor

Thanks @1st1 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

@1st1 1st1 deleted the fixgather branch May 29, 2018 21:20
@bedevere-bot
Copy link

GH-7222 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 29, 2018
@1st1 @miss-islington
bpo-32684: Fix gather to propagate cancel of itself with return_excep…
f26c580 
…tions (pythonGH-7209)

(cherry picked from commit 863b674)

Co-authored-by: Yury Selivanov <yury@magic.io>
@miss-islington
Copy link
Contributor

Sorry, @1st1, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 863b6749093a86810c4077112a857363410cc221 3.6

@1st1
Copy link
Member Author

1st1 commented May 29, 2018

3.6 PR: #7224

1st1 added a commit that referenced this pull request May 29, 2018
@miss-islington @1st1
bpo-32684: Fix gather to propagate cancel of itself with return_excep…
0364342 
…tions (GH-7209) (#7222)

(cherry picked from commit 863b674)

Co-authored-by: Yury Selivanov <yury@magic.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asvetlov asvetlov asvetlov approved these changes

Assignees

@1st1 1st1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@1st1 @miss-islington @bedevere-bot @asvetlov @the-knights-who-say-ni