Skip to content

Conversation

@colesbury
Copy link
Contributor

@colesburycolesbury commented Dec 3, 2024

We were missing locks around some list operations in the free threading build.

We were missing locks around some list operations in the free threading build.
@colesburycolesbury changed the title gh-127524: Add missing locks in listobject.cgh-127536: Add missing locks in listobject.cDec 3, 2024
@colesburycolesbury marked this pull request as ready for review December 4, 2024 04:31
Copy link
Member

@corona10corona10 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@mpagempage left a comment

Choose a reason for hiding this comment

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

Not for this PR, but do you think it would be worthwhile to audit the functions that we expect to be called with the per-object lock held and add _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED() or the equivalent where they're missing?

@colesbury
Copy link
ContributorAuthor

Yes, I think additional _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED checks would make sense.

@colesburycolesbury added the needs backport to 3.13 bugs and security fixes label Dec 4, 2024
@colesburycolesbury merged commit e51da64 into python:mainDec 4, 2024
45 checks passed
@miss-islington-app
Copy link

Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@colesburycolesbury deleted the gh-127524-assert branch December 4, 2024 19:12
@miss-islington-app
Copy link

Sorry, @colesbury, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker e51da64ac3bc6cd45339864db32d05115af39ead 3.13 

@bedevere-app
Copy link

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

@bedevere-appbedevere-appbot removed the needs backport to 3.13 bugs and security fixes label Dec 4, 2024
colesbury added a commit to colesbury/cpython that referenced this pull request Dec 4, 2024
…27580) We were missing locks around some list operations in the free threading build. (cherry picked from commit e51da64) Co-authored-by: Sam Gross <[email protected]>
colesbury added a commit that referenced this pull request Dec 4, 2024
…27613) We were missing locks around some list operations in the free threading build. (cherry picked from commit e51da64)
@bedevere-bot
Copy link

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

Hi! The buildbot s390x RHEL9 Refleaks 3.13 has failed when building commit 4060ef3.

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

Failed tests:

  • test_site
  • test_audit

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

==

Click to see traceback logs
Traceback (most recent call last): File "/home/buildbot/buildarea/3.13.cstratak-rhel9-s390x.refleak/build/Lib/test/audit-tests.py", line 587, in <module>globals()[test](*sys.argv[2:]) ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^ File "/home/buildbot/buildarea/3.13.cstratak-rhel9-s390x.refleak/build/Lib/test/audit-tests.py", line 541, in test_time time.sleep(0) ~~~~~~~~~~^^^ File "/home/buildbot/buildarea/3.13.cstratak-rhel9-s390x.refleak/build/Lib/test/audit-tests.py", line 538, in hookraiseAssertionError('hook failed') AssertionError: hook failed k 

@bedevere-bot
Copy link

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

Hi! The buildbot s390x RHEL8 Refleaks 3.13 has failed when building commit 4060ef3.

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

Failed tests:

  • test_site
  • test_audit

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

==

Click to see traceback logs
Traceback (most recent call last): File "/home/buildbot/buildarea/3.13.cstratak-rhel8-s390x.refleak/build/Lib/test/audit-tests.py", line 587, in <module>globals()[test](*sys.argv[2:]) ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^ File "/home/buildbot/buildarea/3.13.cstratak-rhel8-s390x.refleak/build/Lib/test/audit-tests.py", line 541, in test_time time.sleep(0) ~~~~~~~~~~^^^ File "/home/buildbot/buildarea/3.13.cstratak-rhel8-s390x.refleak/build/Lib/test/audit-tests.py", line 538, in hookraiseAssertionError('hook failed') AssertionError: hook failed k 

srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
We were missing locks around some list operations in the free threading build.
@colesburycolesbury removed their assignment Feb 14, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@colesbury@bedevere-bot@mpage@corona10