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-128479: fix asyncio staggered race leaking tasks, and logging unhandled exception.append exception by graingert · Pull Request #128475 · python/cpython

Conversation

@graingert

@graingert graingert commented Jan 4, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Issue currently pending, but this fixes the following exception when running under anyio's pytest plugin, or logging unhandled asyncio exceptions:

https://github.com/Chia-Network/chia-blockchain/actions/runs/12586550910/job/35084132471#step:16:1813


______ test_long_reorg_nodes[ConsensusMode.HARD_FORK_2_0-2-500-100-True] _______
[gw1] darwin -- Python 3.12.8 /Users/runner/work/chia-blockchain/chia-blockchain/.venv/bin/python
/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/staggered.py:104: in run_one_coro
    result = await coro_fn()
.venv/lib/python3.12/site-packages/aiohappyeyeballs/impl.py:166: in _connect_sock
    await loop.sock_connect(sock, address)
/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/selector_events.py:651: in sock_connect
    return await fut
E   asyncio.exceptions.CancelledError

During handling of the above exception, another exception occurred:
.venv/lib/python3.12/site-packages/anyio/pytest_plugin.py:160: in pytest_pyfunc_call
    runner.run_test(pyfuncitem.obj, testargs)
.venv/lib/python3.12/site-packages/anyio/_backends/_asyncio.py:2316: in run_test
    self._raise_async_exceptions()
.venv/lib/python3.12/site-packages/anyio/_backends/_asyncio.py:2220: in _raise_async_exceptions
    raise exceptions[0]
/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/staggered.py:108: in run_one_coro
    exceptions[this_index] = e
E   NameError: cannot access free variable 'exceptions' where it is not associated with a value in enclosing scope
  • Issue: asyncio.staggered.staggered_race can log NameError: cannot access free variable 'exceptions' where it is not associated with a value in enclosing scope #128479

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@graingert
fix asyncio staggered leaking tasks, and logging unhandled exceptions…
2518c76 
….append exception
graingert
graingert commented Jan 4, 2025
View reviewed changes
Comment thread Lib/asyncio/staggered.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
graingert added 3 commits January 4, 2025 09:08
@graingert
add test
9501879
@graingert
Update Lib/asyncio/staggered.py
dfe8384
@graingert
Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger
dc7c3e3
graingert
graingert commented Jan 4, 2025
View reviewed changes
Comment thread Lib/asyncio/staggered.py
for task in running_tasks:
task.cancel(*ex.args)
on_completed_fut = None
if __debug__ and unhandled_exceptions:

@graingert graingert Jan 4, 2025

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

probably this should just always raise it, not sure why this was suppressed in the original code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@graingert graingert changed the title fix asyncio staggered leaking tasks, and logging unhandled exception.append exception gh-128479: fix asyncio staggered leaking tasks, and logging unhandled exception.append exception Jan 4, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Jan 4, 2025
asyncio.staggered.staggered_race can log NameError: cannot access free variable 'exceptions' where it is not associated with a value in enclosing scope #128479
Closed
@blurb-it
📜🤖 Added by blurb_it.
1c34edc
graingert
graingert commented Jan 4, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2025-01-04-11-10-04.gh-issue-128479.jvOrF-.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@graingert
Update Misc/NEWS.d/next/Library/2025-01-04-11-10-04.gh-issue-128479.j…
62c83a2 
…vOrF-.rst
@graingert graingert added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Jan 4, 2025
@graingert
Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger
f75d77a
@graingert
graingert marked this pull request as ready for review January 4, 2025 11:13
@graingert
graingert requested review from 1st1, asvetlov, kumaraditya303 and willingc as code owners January 4, 2025 11:13
@bedevere-app bedevere-app Bot added the awaiting review label Jan 4, 2025
@graingert graingert changed the title gh-128479: fix asyncio staggered leaking tasks, and logging unhandled exception.append exception gh-128479: fix asyncio staggered race leaking tasks, and logging unhandled exception.append exception Jan 4, 2025
ZeroIntensity
ZeroIntensity reviewed Jan 4, 2025
View reviewed changes

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I haven't stayed up-to-date with the staggered_race problems. Did we adopt some of aiohttp's tests?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Misc/NEWS.d/next/Library/2025-01-04-11-10-04.gh-issue-128479.jvOrF-.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@graingert @ZeroIntensity
Update Misc/NEWS.d/next/Library/2025-01-04-11-10-04.gh-issue-128479.j…
4a4c027 
…vOrF-.rst

Co-authored-by: Peter Bierma <[email protected]>
@graingert

graingert commented Jan 4, 2025

Copy link
Copy Markdown
Contributor Author

I haven't stayed up-to-date with the staggered_race problems. Did we adopt some of aiohttp's tests?

Did you mean aiohappyeyeballs? No those have yet to be added
All reactions

Sorry, something went wrong.

Uh oh!

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

ZeroIntensity commented Jan 4, 2025

Copy link
Copy Markdown
Member

Ah yeah, aiohappyeyeballs. Can we run their (old) test suite against this PR? I don't want another release blocker fiasco.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@graingert
Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger
dd55eae
@graingert

graingert commented Jan 4, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

@ZeroIntensity I got it running against the newest suite: graingert/aiohappyeyeballs@8587b00#diff-d38ea0c7de1f2c4e6030c2e14c786b426be1f20310d14997973c0c1f7621d0e4L33

but it failed on a test that multiple winners can complete. This is not acceptable for the cpython implementation, because it would result in connection objects being left unclosed and result in ResourceWarnings.

I'm not sure which test suite is the old test suite you refer to, do you have a commit hash for that?
All reactions

Sorry, something went wrong.

Uh oh!

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

ZeroIntensity commented Jan 4, 2025

Copy link
Copy Markdown
Member

I'm not sure which test suite is the old test suite you refer to, do you have a commit hash for that?

I don't have a commit hash, but I mean "old" as in when aiohappyeyeballs was still using our staggered_race instead of their own implementation. But, if you just replaced their staggered_race with ours then I'm comfortable.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ZeroIntensity
ZeroIntensity reviewed Jan 4, 2025
View reviewed changes
Comment thread Lib/asyncio/staggered.py Resolved

Uh oh!

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

Uh oh!

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

Uh oh!

There was an error while loading. Please reload this page.
graingert
graingert commented Jan 4, 2025
View reviewed changes
Comment thread Lib/asyncio/staggered.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@graingert
Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger
04532ab
ZeroIntensity
ZeroIntensity reviewed Jan 6, 2025
View reviewed changes

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry for dropping the ball here. Looks mostly good, with a few edge cases/nitpicks to be dealt with.

Sorry, something went wrong.

Uh oh!

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

Uh oh!

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

Uh oh!

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

Uh oh!

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

Uh oh!

There was an error while loading. Please reload this page.
@graingert
Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger
620a344
graingert
graingert commented Jan 6, 2025
View reviewed changes
Comment thread Lib/asyncio/staggered.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
graingert
graingert commented Jan 6, 2025
View reviewed changes
Comment thread Lib/asyncio/staggered.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
graingert
graingert commented Jan 6, 2025
View reviewed changes
Comment thread Lib/asyncio/staggered.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@graingert
Apply suggestions from code review
58f39d9
@graingert
graingert requested a review from ZeroIntensity January 6, 2025 08:42
ZeroIntensity
ZeroIntensity approved these changes Jan 6, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting core review and removed awaiting review labels Jan 6, 2025
graingert added 2 commits January 6, 2025 20:52
@graingert
Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger
fb41c8d
@graingert
Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger
f387540
@ambv
ambv merged commit ec91e1c into python:main Jan 23, 2025
@bedevere-app bedevere-app Bot removed the awaiting core review label Jan 23, 2025
@miss-islington-app

miss-islington-app Bot commented Jan 23, 2025

Copy link
Copy Markdown

Thanks @graingert for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖
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 Jan 23, 2025
@graingert @ZeroIntensity @miss-islington
pythongh-128479: fix asyncio staggered race leaking tasks, and loggin…
2234a53 
…g unhandled exception.append exception (pythonGH-128475)

(cherry picked from commit ec91e1c)

Co-authored-by: Thomas Grainger <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
@bedevere-app

bedevere-app Bot commented Jan 23, 2025

Copy link
Copy Markdown

GH-129227 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Jan 23, 2025
@graingert
graingert deleted the fix-exeptions-append-after-cancel-async-stagger branch January 23, 2025 15:54
@miss-islington-app

miss-islington-app Bot commented Jan 23, 2025

Copy link
Copy Markdown

Sorry @graingert and @ambv, I had trouble completing the backport.
Please retry by removing and re-adding the "needs backport to 3.13" label.
Please backport backport using cherry_picker on the command line.

cherry_picker ec91e1c2762412f1408b0dfb5d281873b852affe 3.13
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app miss-islington-app Bot assigned ambv Jan 23, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 23, 2025
@graingert @ZeroIntensity @miss-islington
pythongh-128479: fix asyncio staggered race leaking tasks, and loggin…
d5053a3 
…g unhandled exception.append exception (pythonGH-128475)

(cherry picked from commit ec91e1c)

Co-authored-by: Thomas Grainger <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
@bedevere-app

bedevere-app Bot commented Jan 23, 2025

Copy link
Copy Markdown

GH-129228 is a backport of this pull request to the 3.12 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Jan 23, 2025
@ambv

ambv commented Jan 23, 2025

Copy link
Copy Markdown
Contributor

The misleading comment by miss-islington is actually an improvement over the previous behavior. There were HTTP timeouts in PR creation so Stamina retried them a few times, not getting a response in time. The final retry returned HTTP 422 since one of the previous retries actually managed to create a PR. And this HTTP 422 is the basis of miss-islington's comment about not being able to complete the backport.

This is all due to an ongoing GitHub incident with Actions:
Screenshot 2025-01-23 at 17 01 52
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ambv pushed a commit that referenced this pull request Jan 23, 2025
@miss-islington @graingert @ZeroIntensity
[3.13] gh-128479: fix asyncio staggered race leaking tasks, and loggi…
75ec6ca 
…ng unhandled exception.append exception (GH-128475) (#129227)

gh-128479: fix asyncio staggered race leaking tasks, and logging unhandled exception.append exception (GH-128475)
(cherry picked from commit ec91e1c)

Co-authored-by: Thomas Grainger <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
ambv pushed a commit that referenced this pull request Jan 23, 2025
@miss-islington @graingert @ZeroIntensity
[3.12] gh-128479: fix asyncio staggered race leaking tasks, and loggi…
e94939c 
…ng unhandled exception.append exception (GH-128475) (#129228)

gh-128479: fix asyncio staggered race leaking tasks, and logging unhandled exception.append exception (GH-128475)
(cherry picked from commit ec91e1c)

Co-authored-by: Thomas Grainger <[email protected]>
Co-authored-by: Peter Bierma <[email protected]>
bdraco added a commit to aio-libs/aiohappyeyeballs that referenced this pull request Mar 3, 2025
@bdraco
fix: update staggered race implementation to use latest cpython version
d248314 
related aiohttp issue aio-libs/aiohttp#10506

In #101 we replaced the staggered race implementation since the cpython version had races
that were not fixed at the time. cpython has since updated the implementation to fix
additional races. Our current implementation still has problems with cancellation
and cpython has fixed that in python/cpython#128475
and python/cpython#124847

This PR ports the latest cpython implementation
@bdraco bdraco mentioned this pull request Mar 3, 2025
fix: update staggered race implementation to use latest cpython version aio-libs/aiohappyeyeballs#142
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ZeroIntensity ZeroIntensity ZeroIntensity 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
@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner
@willingc willingc Awaiting requested review from willingc willingc is a code owner

Uh oh!

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

Assignees

@ambv ambv

Labels

None yet

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.

3 participants

@graingert @ZeroIntensity @ambv
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.