Skip to content

gh-127529: Correct asyncio.selector_events.BaseSelectorEventLoop._accept_connection's behaviour for handling ConnectionAbortedError #127532

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
kumaraditya303 merged 7 commits into from
Jan 3, 2025
Merged

gh-127529: Correct asyncio.selector_events.BaseSelectorEventLoop._accept_connection's behaviour for handling ConnectionAbortedError #127532

kumaraditya303 merged 7 commits into from
Jan 3, 2025

Conversation

@jb2170
Copy link
Contributor

@jb2170 jb2170 commented Dec 2, 2024 •
edited by bedevere-app bot
Loading

Closes #127529

All good to go. ConnectionAbortedError now continues instead of returning. Improves OpenBSD performance. Full writeup in the issue.

I've left InterruptedError grouped with BlockingIOError to both return early, instead of InterruptedError continueing and BlockingIOError returning. PEP-475 should make InterruptedError never appear right, but hey if it's not broken don't fix it. Best to play it safe with the urgency of signals, especially as asyncio uses them for a wakeup fd.

@bedevere-app bedevere-app bot mentioned this pull request Dec 2, 2024
Closed
@jb2170 jb2170 changed the title gh-127529: Correct asyncio.BaseSelectorEventLoop._accept_connection's behaviour for handling ConnectionAbortedError gh-127529: Correct asyncio.selector_events.BaseSelectorEventLoop._accept_connection's behaviour for handling ConnectionAbortedError Dec 2, 2024
@jb2170
Fix pythongh-127529: Correct asyncio.selector_events.BaseSelectorEven…
7ab3149 
…tLoop._accept_connection's behaviour for handling ConnectionAbortedError
@jb2170
Copy link
Contributor Author

jb2170 commented Dec 2, 2024 •
edited
Loading

Force pushes to correct typo / my suckiness at reStructuredText

@jb2170
Copy link
Contributor Author

jb2170 commented Dec 28, 2024

@1st1 @asvetlov @kumaraditya303 @willingc Trivial PR to review + merge before the new year / before it fades out of memory? 🚀

@kumaraditya303
Copy link
Contributor

kumaraditya303 commented Dec 29, 2024

You need to add tests for this.

@jb2170
Add BaseSelectorEventLoopTests.test_accept_connection_skip_connection…
317923d 
…abortederror to test graceful handling of ConnectionAbortedError
@jb2170
Copy link
Contributor Author

jb2170 commented Dec 29, 2024

Hi @kumaraditya303 , thanks for the reply!

I've added a test to test_selector_events.py that checks for the _accept_connection for-loop continuing through aborted connections instead of returning early. The test passes with this PR's changes 😄

kumaraditya303
kumaraditya303 reviewed Jan 3, 2025
View reviewed changes
@kumaraditya303 kumaraditya303 enabled auto-merge (squash) January 3, 2025 10:11
@kumaraditya303 kumaraditya303 merged commit 830e106 into python:main Jan 3, 2025
44 checks passed
@jb2170 jb2170 deleted the gh-127529 branch January 3, 2025 20:17
WolframAlph pushed a commit to WolframAlph/cpython that referenced this pull request Jan 4, 2025
@jb2170 @kumaraditya303 @WolframAlph
pythongh-127529: Correct asyncio's accept_connection behaviour for …
d123195 
…handling `ConnectionAbortedError` (python#127532)

Co-authored-by: Kumar Aditya <[email protected]>
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
@jb2170 @kumaraditya303 @srinivasreddy
pythongh-127529: Correct asyncio's accept_connection behaviour for …
55a0f2f 
…handling `ConnectionAbortedError` (python#127532)

Co-authored-by: Kumar Aditya <[email protected]>
@ambv
Copy link
Contributor

ambv commented May 21, 2025 •
edited
Loading

@kumaraditya303 don't we want to backport this to 3.13? WDYT?

@ambv ambv mentioned this pull request May 21, 2025
Merged
@emmatyping
Copy link
Member

emmatyping commented Dec 14, 2025

@kumaraditya303 I think we still need to backport this, right? Looks like it is blocking the 3.13 backport of #134392

@kumaraditya303 kumaraditya303 added the needs backport to 3.13 bugs and security fixes label Dec 15, 2025
@miss-islington-app
Copy link

miss-islington-app bot commented Dec 15, 2025

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 15, 2025
@jb2170 @kumaraditya303 @miss-islington
pythongh-127529: Correct asyncio's accept_connection behaviour for …
11f69a7 
…handling `ConnectionAbortedError` (pythonGH-127532)

(cherry picked from commit 830e106)

Co-authored-by: jb2170 <[email protected]>
Co-authored-by: Kumar Aditya <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Dec 15, 2025

GH-142738 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Dec 15, 2025
@kumaraditya303
Copy link
Contributor

kumaraditya303 commented Dec 15, 2025

I think we still need to backport this, right? Looks like it is blocking the 3.13 backport of #134392

Thanks for the reminder, I'll get this and the other one backported.

kumaraditya303 added a commit that referenced this pull request Dec 15, 2025
@miss-islington @jb2170 @kumaraditya303
[3.13] gh-127529: Correct asyncio's accept_connection behaviour for…
18bd88f 
… handling `ConnectionAbortedError` (GH-127532) (#142738)

gh-127529: Correct asyncio's `accept_connection` behaviour for handling `ConnectionAbortedError` (GH-127532)
(cherry picked from commit 830e106)

Co-authored-by: jb2170 <[email protected]>
Co-authored-by: Kumar Aditya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes

@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner

@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner

@willingc willingc Awaiting requested review from willingc willingc is a code owner

Assignees

No one assigned

Labels

topic-asyncio

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

asyncio's BaseSelectorEventLoop._accept_connection returns when it should continue on ConnectionAbortedError

4 participants

@jb2170 @kumaraditya303 @ambv @emmatyping