Skip to content

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

Merged
1st1 merged 2 commits intopython:masterfrom
1st1:fixgather
May 29, 2018
Merged

bpo-32684: Fix gather to propagate cancel of itself with return_exceptions#7209
1st1 merged 2 commits intopython:masterfrom
1st1:fixgather

Conversation

@1st1
Copy link
Copy Markdown
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
for child in self._children:
if child.cancel():
ret = True
if ret:
Copy link
Copy Markdown
Contributor

@asvetlov asvetlov May 29, 2018

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
Copy Markdown
Member Author

@1st1 1st1 May 29, 2018

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
Copy Markdown
Contributor

@asvetlov asvetlov May 29, 2018

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
Copy Markdown
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
Copy Markdown
Contributor

miss-islington commented May 29, 2018

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
Copy Markdown

bedevere-bot commented May 29, 2018

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
Copy Markdown
Contributor

miss-islington commented May 29, 2018

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
Copy Markdown
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