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-121621: Move asyncio_running_loop to private struct by colesbury · Pull Request #121939 · python/cpython

Conversation

@colesbury

@colesbury colesbury commented Jul 17, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

This avoids changing the ABI and places the field in the private struct.

  • Issue: asyncio module state should be locked with mutexes in free-threaded builds #121621

Sorry, something went wrong.

Uh oh!

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

All reactions
@colesbury
pythongh-121621: Move asyncio_running_loop to private struct
d623558
This avoids changing the ABI and keeps the field in the private struct.
@colesbury colesbury added skip news needs backport to 3.13 only security fixes labels Jul 17, 2024
@colesbury
colesbury requested review from Fidget-Spinner, JelleZijlstra and Yhg1s July 17, 2024 21:46
@colesbury
colesbury requested review from 1st1, asvetlov, ericsnowcurrently, gvanrossum, kumaraditya303 and willingc as code owners July 17, 2024 21:46
@bedevere-app bedevere-app Bot mentioned this pull request Jul 17, 2024
asyncio module state should be locked with mutexes in free-threaded builds #121621
Closed
@bedevere-app bedevere-app Bot added the awaiting core review label Jul 17, 2024
@colesbury

colesbury commented Jul 17, 2024

Copy link
Copy Markdown
Contributor Author

I don't know if we care enough about avoiding ABI changes in 3.13. If not, then feel free to close this PR.

All reactions

Sorry, something went wrong.

Uh oh!

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

@colesbury
Fixes
d3a2809
@JelleZijlstra

JelleZijlstra commented Jul 17, 2024

Copy link
Copy Markdown
Member

I don't know much about this area, but it seems better in any case to put this element on a private struct if we don't need it to be public.

All reactions

Sorry, something went wrong.

Uh oh!

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

JelleZijlstra
JelleZijlstra approved these changes Jul 17, 2024
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Jul 17, 2024
@Yhg1s

Yhg1s commented Jul 17, 2024

Copy link
Copy Markdown
Member

Considering ABI stability is of great benefit for testing, and this doesn't seem like an otherwise onerous change, I'd prefer to get this in. We need to delay 3.13b4 until tomorrow anyway because of unrelated issues, so I'll respin with this in.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Yhg1s
Yhg1s merged commit 81fd625 into python:main Jul 17, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Jul 17, 2024
@miss-islington-app

miss-islington-app Bot commented Jul 17, 2024

Copy link
Copy Markdown

Thanks @colesbury for the PR, and @Yhg1s for merging it 🌮🎉.. I'm working now to backport this PR to: 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 Jul 17, 2024
@colesbury @miss-islington
pythongh-121621: Move asyncio_running_loop to private struct (pythonG…
10b083d
…H-121939)

This avoids changing the ABI and keeps the field in the private struct.
(cherry picked from commit 81fd625)

Co-authored-by: Sam Gross <[email protected]>
@bedevere-app

bedevere-app Bot commented Jul 17, 2024

Copy link
Copy Markdown

GH-121943 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 Jul 17, 2024
@colesbury
colesbury deleted the gh-121621-abi branch July 17, 2024 22:25
Yhg1s pushed a commit that referenced this pull request Jul 18, 2024
@miss-islington @colesbury
[3.13] gh-121621: Move asyncio_running_loop to private struct (GH-121939
afa5321
) (#121943)

gh-121621: Move asyncio_running_loop to private struct (GH-121939)

This avoids changing the ABI and keeps the field in the private struct.
(cherry picked from commit 81fd625)

Co-authored-by: Sam Gross <[email protected]>
@bedevere-bot

bedevere-bot commented Jul 18, 2024

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot s390x Fedora Clang Installed 3.13 has failed when building commit afa5321.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#builders/1392/builds/212) and take a look at the build logs.
  4. Check if the failure is related to this commit (afa5321) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#builders/1392/builds/212

Failed tests:

  • test_pyrepl

Failed subtests:

  • test_inspect_keeps_globals_from_inspected_file - test.test_pyrepl.test_pyrepl.TestMain.test_inspect_keeps_globals_from_inspected_file
  • test_inspect_keeps_globals_from_inspected_module - test.test_pyrepl.test_pyrepl.TestMain.test_inspect_keeps_globals_from_inspected_module

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.13.edelsohn-fedora-z.clang-installed/build/target/lib/python3.13/test/test_pyrepl/test_pyrepl.py", line 995, in _run_repl_globals_test
    self.fail(f"{var}= not found in output")
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: FOO= not found in output


Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.13.edelsohn-fedora-z.clang-installed/build/target/lib/python3.13/test/test_pyrepl/test_pyrepl.py", line 995, in _run_repl_globals_test
    self.fail(f"{var}= not found in output")
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: __name__= not found in output


Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.13.edelsohn-fedora-z.clang-installed/build/target/lib/python3.13/test/test_pyrepl/test_pyrepl.py", line 995, in _run_repl_globals_test
    self.fail(f"{var}= not found in output")
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: __package__= not found in output
All reactions

Sorry, something went wrong.

Uh oh!

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes
@Yhg1s Yhg1s Awaiting requested review from Yhg1s
@Fidget-Spinner Fidget-Spinner Awaiting requested review from Fidget-Spinner
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@gvanrossum gvanrossum Awaiting requested review from gvanrossum
@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

No one assigned

Labels

skip news

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.

4 participants

@colesbury @JelleZijlstra @Yhg1s @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.