Skip to content

gh-113538: Don't crash in stream reader protocol callback when task is cancelled#113690

Merged
gvanrossum merged 3 commits into
python:mainfrom
gvanrossum:asyncio-streams-fix
Jan 4, 2024
Merged

gh-113538: Don't crash in stream reader protocol callback when task is cancelled#113690
gvanrossum merged 3 commits into
python:mainfrom
gvanrossum:asyncio-streams-fix

Conversation

@gvanrossum

@gvanrossum gvanrossum commented Jan 3, 2024

Copy link
Copy Markdown
Member

That callback exists purely to log an error when the coroutine running in the task raises an exception. But if the task was cancelled instead, the callback itself crashes, which just logs noise. In that case we should just close the transport without logging anything.

CC @jsundahl

@itamaro

itamaro commented Jan 4, 2024

Copy link
Copy Markdown
Contributor

is there a test case we can add that would cover the issue and fix?

@jsundahl jsundahl left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

looks good, I do think closing the transport if the task is cancelled is the correct thing to do.

@itamaro itamaro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks for adding a test case! LG!

@gvanrossum
gvanrossum merged commit 4681a52 into python:main Jan 4, 2024
@miss-islington-app

Copy link
Copy Markdown

Thanks @gvanrossum for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

@gvanrossum
gvanrossum deleted the asyncio-streams-fix branch January 4, 2024 20:20
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 4, 2024
…task is cancelled (pythonGH-113690)

(cherry picked from commit 4681a52)

Co-authored-by: Guido van Rossum <guido@python.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 4, 2024
…task is cancelled (pythonGH-113690)

(cherry picked from commit 4681a52)

Co-authored-by: Guido van Rossum <guido@python.org>
@bedevere-app

bedevere-app Bot commented Jan 4, 2024

Copy link
Copy Markdown

GH-113713 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Jan 4, 2024
@bedevere-app

bedevere-app Bot commented Jan 4, 2024

Copy link
Copy Markdown

GH-113714 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.11 only security fixes label Jan 4, 2024
gvanrossum added a commit that referenced this pull request Jan 4, 2024
… task is cancelled (GH-113690) (#113714)

(cherry picked from commit 4681a52)

Co-authored-by: Guido van Rossum <guido@python.org>
gvanrossum added a commit that referenced this pull request Jan 4, 2024
… task is cancelled (GH-113690) (#113713)

(cherry picked from commit 4681a52)

Co-authored-by: Guido van Rossum <guido@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants