Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-127536: Add missing locks in listobject.c #127580

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

colesbury
Copy link
Contributor

@colesbury colesbury 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.
@colesbury colesbury changed the title gh-127524: Add missing locks in listobject.c gh-127536: Add missing locks in listobject.c Dec 3, 2024
@colesbury colesbury marked this pull request as ready for review December 4, 2024 04:31
@colesbury colesbury requested review from corona10 and mpage December 4, 2024 04:31
Copy link
Member

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

@mpage mpage 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
Contributor Author

Yes, I think additional _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED checks would make sense.

@colesbury colesbury added the needs backport to 3.13 bugs and security fixes label Dec 4, 2024
@colesbury colesbury merged commit e51da64 into python:main Dec 4, 2024
49 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!

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

bedevere-app bot commented Dec 4, 2024

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

@bedevere-app bedevere-app bot 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 <colesbury@gmail.com>
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 hook
    raise AssertionError('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 hook
    raise AssertionError('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.
@colesbury colesbury removed their assignment Feb 14, 2025
Sign up for free to 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