GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-95289: Always call uncancel() when parent cancellation is requested by kumaraditya303 · Pull Request #95602 · python/cpython

Conversation

@kumaraditya303

@kumaraditya303 kumaraditya303 commented Aug 3, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • Issue: asyncio.TaskGroup corrupts uncancel() stack when the parent task replaces the cancelled error #95289

Co-Authored-By: Guido van Rossum [email protected]

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
gvanrossum and others added 2 commits August 3, 2022 16:37
@gvanrossum
pythonGH-95289: Always call uncancel() when required
14a5674
@kumaraditya303
add test
10d17bc
@kumaraditya303
kumaraditya303 requested review from 1st1 and asvetlov as code owners August 3, 2022 16:48
@bedevere-bot bedevere-bot added the awaiting review label Aug 3, 2022
@blurb-it
📜🤖 Added by blurb_it.
8cc90a5
@kumaraditya303
kumaraditya303 force-pushed the fix-taskgroup-uncancel branch from 8498dda to 8cc90a5 Compare August 3, 2022 16:53
@kumaraditya303 kumaraditya303 added type-bug An unexpected behavior, bug, or error release-blocker needs backport to 3.11 only security fixes labels Aug 3, 2022
@kumaraditya303 kumaraditya303 mentioned this pull request Aug 3, 2022
GH-95289: Always call uncancel() when required #95513
Closed
@kumaraditya303 kumaraditya303 assigned gvanrossum Aug 3, 2022
@kumaraditya303
kumaraditya303 requested a review from gvanrossum August 3, 2022 16:57
graingert
graingert requested changes Aug 3, 2022
View reviewed changes
Comment thread Lib/asyncio/taskgroups.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Aug 3, 2022
gvanrossum
gvanrossum reviewed Aug 3, 2022
View reviewed changes
Comment thread Lib/test/test_asyncio/test_taskgroups.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/asyncio/taskgroups.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@gvanrossum
Check that exactly 2 CustomExceptions were raised
f8004d2
@gvanrossum

gvanrossum commented Aug 4, 2022

Copy link
Copy Markdown
Member

We're getting very close to the RC1 release date (this Friday!). I'd like to get this in. @graingert can you bless this as strictly better than what we had before?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@kumaraditya303

kumaraditya303 commented Aug 4, 2022

Copy link
Copy Markdown
Contributor Author

We're getting very close to the RC1 release date (this Friday!). I'd like to get this in

I agree, I would like to get this in ASAP.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@graingert

graingert commented Aug 4, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

We're getting very close to the RC1 release date (this Friday!). I'd like to get this in. @graingert can you bless this as strictly better than what we had before?

This now follows the five (six? Cc @ambv) rules of uncancellation so I think therefore it's strictly better. However I can't say for sure. It looks to me like TaskGroup was relying on task.cancelling() >= 1 traveling up the stack to communicate with parent scopes without anticipating that it also travels down the rest of the function body.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@graingert

graingert commented Aug 4, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

In addition I think there's other more fundamental problems with uncancel, for example when a cancellation is delivered from a chained concurrent.fututes.Future or when a TaskGroup task waits on another TaskGroup task and cancellation is chained between them
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gvanrossum
gvanrossum merged commit 2fef275 into python:main Aug 4, 2022
@bedevere-bot bedevere-bot removed the awaiting core review label Aug 4, 2022
@miss-islington

miss-islington commented Aug 4, 2022

Copy link
Copy Markdown
Contributor

Thanks @kumaraditya303 for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 4, 2022
@kumaraditya303 @miss-islington
pythonGH-95289: Always call uncancel() when parent cancellation is re…
fb5fe45 
…quested (pythonGH-95602)

Co-authored-by: Guido van Rossum <[email protected]>
(cherry picked from commit 2fef275)

Co-authored-by: Kumar Aditya <[email protected]>
@bedevere-bot

bedevere-bot commented Aug 4, 2022

Copy link
Copy Markdown

GH-95654 is a backport of this pull request to the 3.11 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Aug 4, 2022
miss-islington added a commit that referenced this pull request Aug 4, 2022
@miss-islington @kumaraditya303
GH-95289: Always call uncancel() when parent cancellation is requested (
2d84fe5 
GH-95602)

Co-authored-by: Guido van Rossum <[email protected]>
(cherry picked from commit 2fef275)

Co-authored-by: Kumar Aditya <[email protected]>
@kumaraditya303
kumaraditya303 deleted the fix-taskgroup-uncancel branch August 4, 2022 14:51
@kumaraditya303 kumaraditya303 mentioned this pull request Aug 4, 2022
asyncio.TaskGroup corrupts uncancel() stack when the parent task replaces the cancelled error #95289
Closed
@gvanrossum gvanrossum mentioned this pull request Aug 10, 2022
GH-95704: Don't suppress errors from tasks when TG is cancelled #95761
Merged
@kumaraditya303 kumaraditya303 added the topic-asyncio label Jul 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@graingert graingert graingert requested changes
@gvanrossum gvanrossum gvanrossum left review comments
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@gvanrossum gvanrossum

Labels

release-blocker topic-asyncio type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@kumaraditya303 @gvanrossum @graingert @miss-islington @bedevere-bot
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.