gh-128479: fix asyncio staggered race leaking tasks, and logging unhandled exception.append exception by graingert · Pull Request #128475 · python/cpython · GitHub

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

….append exception

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

…vOrF-.rst

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

…vOrF-.rst Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

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

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

@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?

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.

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

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

…g unhandled exception.append exception (pythonGH-128475) (cherry picked from commit ec91e1c) Co-authored-by: Thomas Grainger <tagrain@gmail.com> Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

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

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

…g unhandled exception.append exception (pythonGH-128475) (cherry picked from commit ec91e1c) Co-authored-by: Thomas Grainger <tagrain@gmail.com> Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

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

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:

…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 <tagrain@gmail.com> Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

…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 <tagrain@gmail.com> Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

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

fix asyncio staggered leaking tasks, and logging unhandled exceptions…

2518c76

….append exception

graingert commented Jan 4, 2025

View reviewed changes

Comment thread Lib/asyncio/staggered.py

graingert added 3 commits January 4, 2025 09:08

add test

9501879

Update Lib/asyncio/staggered.py

dfe8384

Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger

dc7c3e3

graingert commented Jan 4, 2025

View reviewed changes

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

📜🤖 Added by blurb_it.

1c34edc

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 Outdated

Update Misc/NEWS.d/next/Library/2025-01-04-11-10-04.gh-issue-128479.j…

62c83a2

…vOrF-.rst

graingert added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Jan 4, 2025

Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger

f75d77a

graingert marked this pull request as ready for review January 4, 2025 11:13

graingert requested review from 1st1, asvetlov, kumaraditya303 and willingc as code owners January 4, 2025 11:13

bedevere-app Bot added the awaiting review label Jan 4, 2025

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 reviewed Jan 4, 2025

View reviewed changes

Comment thread Misc/NEWS.d/next/Library/2025-01-04-11-10-04.gh-issue-128479.jvOrF-.rst Outdated

Update Misc/NEWS.d/next/Library/2025-01-04-11-10-04.gh-issue-128479.j…

4a4c027

…vOrF-.rst Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger

dd55eae

ZeroIntensity reviewed Jan 4, 2025

View reviewed changes

Comment thread Lib/asyncio/staggered.py Outdated

Comment thread Lib/asyncio/staggered.py Outdated

Comment thread Lib/asyncio/staggered.py Outdated

graingert commented Jan 4, 2025

View reviewed changes

Comment thread Lib/asyncio/staggered.py Outdated

Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger

04532ab

ZeroIntensity reviewed Jan 6, 2025

View reviewed changes

Comment thread Lib/asyncio/staggered.py

Comment thread Lib/asyncio/staggered.py Outdated

Comment thread Lib/asyncio/staggered.py

Comment thread Lib/asyncio/staggered.py Outdated

Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger

620a344

graingert commented Jan 6, 2025

View reviewed changes

Comment thread Lib/asyncio/staggered.py Outdated

graingert commented Jan 6, 2025

View reviewed changes

Comment thread Lib/asyncio/staggered.py Outdated

graingert commented Jan 6, 2025

View reviewed changes

Comment thread Lib/asyncio/staggered.py

Apply suggestions from code review

58f39d9

graingert requested a review from ZeroIntensity January 6, 2025 08:42

ZeroIntensity approved these changes Jan 6, 2025

View reviewed changes

bedevere-app Bot added awaiting core review and removed awaiting review labels Jan 6, 2025

graingert added 2 commits January 6, 2025 20:52

Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger

fb41c8d

Merge branch 'main' into fix-exeptions-append-after-cancel-async-stagger

f387540

ambv merged commit ec91e1c into python:main Jan 23, 2025

bedevere-app Bot removed the awaiting core review label Jan 23, 2025

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Jan 23, 2025

graingert deleted the fix-exeptions-append-after-cancel-async-stagger branch January 23, 2025 15:54

miss-islington-app Bot assigned ambv Jan 23, 2025

bedevere-app Bot removed the needs backport to 3.12 only security fixes label Jan 23, 2025

bdraco mentioned this pull request Mar 3, 2025

fix: update staggered race implementation to use latest cpython version aio-libs/aiohappyeyeballs#142

Closed

+                              for task in running_tasks:
+                                  task.cancel(*ex.args)
+                          on_completed_fut = None
+                      if __debug__ and unhandled_exceptions:

gh-128479: fix asyncio staggered race leaking tasks, and logging unhandled exception.append exception by graingert · Pull Request #128475 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-128479: fix asyncio staggered race leaking tasks, and logging unhandled exception.append exception - #128475

Conversation

graingert commented Jan 4, 2025 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

graingert Jan 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

graingert commented Jan 4, 2025

Uh oh!

ZeroIntensity commented Jan 4, 2025

Uh oh!

graingert commented Jan 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZeroIntensity commented Jan 4, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miss-islington-app Bot commented Jan 23, 2025

Uh oh!

bedevere-app Bot commented Jan 23, 2025

Uh oh!

miss-islington-app Bot commented Jan 23, 2025

Uh oh!

bedevere-app Bot commented Jan 23, 2025

Uh oh!

ambv commented Jan 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

graingert commented Jan 4, 2025 •
edited
Loading