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-120973: Fix thread-safety issues with `threading.local` by mpage · Pull Request #121655 · python/cpython

Conversation

@mpage

@mpage mpage commented Jul 12, 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 is a small refactoring to the current design that allows us to avoid manually iterating over threads.

This should also fix gh-118490.

  • Issue: threading.local() implementation is not thread-safe in the free-threaded build #120973

Sorry, something went wrong.

Uh oh!

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

All reactions
@mpage
Fix thread-safety issues with threading.local
8d41fbc
This is a small refactoring to the current design that allows us to
avoid manually iterating over threads.
@mpage
mpage requested a review from colesbury July 12, 2024 23:36
@bedevere-app bedevere-app Bot mentioned this pull request Jul 12, 2024
threading.local() implementation is not thread-safe in the free-threaded build #120973
Closed
@mpage mpage added the skip news label Jul 12, 2024
@mpage
mpage marked this pull request as ready for review July 13, 2024 03:20
@mpage
mpage requested a review from ericsnowcurrently as a code owner July 13, 2024 03:20
@bedevere-app bedevere-app Bot added the awaiting review label Jul 13, 2024
ambv
ambv approved these changes Jul 15, 2024
View reviewed changes

@ambv ambv left a comment

Copy link
Copy Markdown
Contributor

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.

Tested locally, works for me. Read through the implementation, clever trick with the sentinel. I find it just a tiny bit uglier that thread state now is concerned with thread locals, but it makes sense given the problem this PR is solving.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Modules/_threadmodule.c Resolved

Uh oh!

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

@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Jul 15, 2024
@colesbury colesbury added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Jul 15, 2024
@bedevere-bot

bedevere-bot commented Jul 15, 2024

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @colesbury for commit 8d41fbc 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Jul 15, 2024
@colesbury colesbury added the needs backport to 3.13 only security fixes label Jul 15, 2024
colesbury
colesbury reviewed Jul 15, 2024
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

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.

Overall, I think the design looks good. Some comments below.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Modules/_threadmodule.c Resolved

Uh oh!

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

Comment thread Modules/_threadmodule.c Resolved

Uh oh!

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

Comment thread Modules/_threadmodule.c Resolved

Uh oh!

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

Comment thread Modules/_threadmodule.c Resolved

Uh oh!

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

Comment thread Modules/_threadmodule.c Resolved

Uh oh!

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

Comment thread Modules/_threadmodule.c Resolved

Uh oh!

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

mpage added 6 commits July 15, 2024 16:47
@mpage
Use static inline func for getting thread state
7769605
@mpage
Use PyTuple_SET_ITEM
f51fe1e
@mpage
Clear and restore exceptions around C-API calls in the error path
1fdbd49
@mpage
Use strong references and don't invoke __init__ twice for subclasses
5c249f3
@mpage
Fix formatting
d51559d
@mpage
Merge branch 'main' into pythongh-120973-fix-local-clear
0e9a0bc
@mpage
mpage requested a review from colesbury July 17, 2024 00:00
@mpage

mpage commented Jul 17, 2024

Copy link
Copy Markdown
Contributor Author

@colesbury - I think this should be good to go. Would you please have another look?

All reactions

Sorry, something went wrong.

Uh oh!

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

colesbury
colesbury approved these changes Jul 17, 2024
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

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.

LGTM

I'll wait until today's releases finish before merging this.

Sorry, something went wrong.

Uh oh!

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

mpage reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@colesbury
Merge branch 'main' into pythongh-120973-fix-local-clear
9a7350d
@colesbury

colesbury commented Jul 19, 2024

Copy link
Copy Markdown
Contributor

!buildbot AMD64 Ubuntu NoGIL Refleaks

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot

bedevere-bot commented Jul 19, 2024

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @colesbury for commit 9a7350d 🤖

The command will test the builders whose names match following regular expression: AMD64 Ubuntu NoGIL Refleaks

The builders matched are:

  • AMD64 Ubuntu NoGIL Refleaks PR
All reactions

Sorry, something went wrong.

Uh oh!

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

@colesbury colesbury self-assigned this Jul 19, 2024
@colesbury
colesbury merged commit e059aa6 into python:main Jul 19, 2024
@miss-islington-app

miss-islington-app Bot commented Jul 19, 2024

Copy link
Copy Markdown

Thanks @mpage for the PR, and @colesbury 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.

@bedevere-app bedevere-app Bot removed the awaiting merge label Jul 19, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 19, 2024
@mpage @miss-islington
pythongh-120973: Fix thread-safety issues with threading.local (pyt…
1204d28
…honGH-121655)

This is a small refactoring to the current design that allows us to
avoid manually iterating over threads.

This should also fix pythongh-118490.
(cherry picked from commit e059aa6)

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

bedevere-app Bot commented Jul 19, 2024

Copy link
Copy Markdown

GH-122042 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 19, 2024
colesbury pushed a commit that referenced this pull request Jul 19, 2024
@miss-islington @mpage
[3.13] gh-120973: Fix thread-safety issues with threading.local (GH…
bf7f4f3
…-121655) (#122042)

This is a small refactoring to the current design that allows us to
avoid manually iterating over threads.

This should also fix gh-118490.
(cherry picked from commit e059aa6)

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

bedevere-bot commented Jul 19, 2024

Copy link
Copy Markdown

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

Hi! The buildbot AMD64 Windows11 Non-Debug 3.x has failed when building commit e059aa6.

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/914/builds/4113) and take a look at the build logs.
  4. Check if the failure is related to this commit (e059aa6) 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/914/builds/4113

Failed tests:

  • test_int

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

==

Click to see traceback logs
Traceback (most recent call last):
  File "b:\uildarea\3.x.ware-win11.nondebug\build\Lib\test\test_int.py", line 657, in test_denial_of_service_prevented_str_to_int
    self.assertLessEqual(sw_fail_extra_huge.seconds, sw_convert.seconds/2)
    ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 0.015625 not less than or equal to 0.0078125


Traceback (most recent call last):
  File "b:\uildarea\3.x.ware-win11.nondebug\build\Lib\test\test_int.py", line 646, in test_denial_of_service_prevented_str_to_int
    self.assertLessEqual(sw_fail_huge.seconds, sw_convert.seconds/2)
    ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 0.015625 not less than or equal to 0.0078125
All reactions

Sorry, something went wrong.

Uh oh!

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

@colesbury colesbury removed their assignment Jul 19, 2024
@efimov-mikhail efimov-mikhail mentioned this pull request Nov 1, 2025
gh-140798: Fix memory leak with threading.local when finalizer resurrects threading_local_key #140836
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ambv ambv ambv approved these changes
@colesbury colesbury colesbury approved these changes
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently 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.

test_threading_local_clear_race segmentation fault in free-threaded CI

Uh oh!

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

4 participants

@mpage @bedevere-bot @colesbury @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.