Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

gh-127945: fix thread safety of creating instances of ctypes structures #131716

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

Merged
merged 12 commits into from
Mar 30, 2025

Conversation

kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented Mar 25, 2025

In free-threading, concurrent mutations to StgInfo is not thread safe. Therefore to make it thread safe, when modifying StgInfo, this PR adds STGINFO_LOCK and STGINFO_UNLOCK macros which are used to acquire critical section of the StgInfo. The critical section is write only and is acquired when modifying the StgInfo fields and while setting the dict_final bit. Once the dict_final is set, StgInfo is treated as read only and no further modifications are allowed. This allows to avoid acquiring the critical section for most read operations when dict_final is set (general case).

Fixes #128567
Fixes #128570

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @kumaraditya303 for commit b0e06a0 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131716%2Fmerge

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 25, 2025
Copy link
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

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

Looks great! A few minor comments.

Modules/_ctypes/stgdict.c Outdated Show resolved Hide resolved
Modules/_ctypes/ctypes.h Outdated Show resolved Hide resolved
Modules/_ctypes/ctypes.h Outdated Show resolved Hide resolved
Lib/test/test_ctypes/test_free_threading.py Outdated Show resolved Hide resolved
Modules/_ctypes/_ctypes.c Outdated Show resolved Hide resolved
Modules/_ctypes/ctypes.h Outdated Show resolved Hide resolved
@kumaraditya303 kumaraditya303 merged commit bc5a028 into python:main Mar 30, 2025
41 checks passed
@kumaraditya303 kumaraditya303 deleted the critical-ctypes branch March 30, 2025 09:52
seehwan pushed a commit to seehwan/cpython that referenced this pull request Apr 16, 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.

Race in stgdict PyCStructUnionType_update_stginfo under free-threading Race in ctypes PyCFuncPtr_new under free-threading
4 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.