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.

Sign up for GitHub

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

Jump to bottom

Make _struct module thread-safe in --disable-gil builds #112062

Closed
Tracked by #116738
, #108219
...
colesbury opened this issue Nov 14, 2023 · 1 comment · Fixed by #112094
Closed
Tracked by #116738
, #108219
...

Make _struct module thread-safe in --disable-gil builds #112062

colesbury opened this issue Nov 14, 2023 · 1 comment · Fixed by #112094
Assignees
@chgnrdv
Labels
3.13 bugs and security fixes extension-modules C modules in the Modules dir topic-free-threading type-feature A feature request or enhancement

Comments

@colesbury
Copy link
Contributor

colesbury commented Nov 14, 2023 •
edited
Loading

Feature or enhancement

The _struct module has a few small issues:

  • The use of PyDict_GetItemWithError returns a borrowed reference (should use PyDict_GetItemRef)
  • The state->cache is lazily created; we should instead create it during _structmodule_exec
  • We want state->cache to be an immutable reference to a mutable dict. (The dict will be thread-safe.) Use PyDict_Clear to empty the dict instead of clearing the reference.

See the commit from the nogil-3.12 fork for context: colesbury/nogil-3.12@ada9b73feb. Note that in CPython the relevant function is PyDict_GetItemRef not PyDict_FetchItemWithError.

Linked PRs
@colesbury colesbury added type-feature A feature request or enhancement 3.13 bugs and security fixes topic-free-threading labels Nov 14, 2023
@colesbury colesbury mentioned this issue Nov 14, 2023
Open
@AlexWaygood AlexWaygood added the extension-modules C modules in the Modules dir label Nov 14, 2023
chgnrdv added a commit to chgnrdv/cpython that referenced this issue Nov 14, 2023
@chgnrdv
pythongh-112062: Make _struct module thread-safe in --disable-gil b…
bc2dac3 
…uilds

* use `PyDict_GetItemRef` instead of `PyDict_GetItemWithError` in `cache_struct_converter` to avoid usage of borrowed ref with explicit incref
* clear cache dict but preserve the reference in `_clearcache_impl`
* create cache dict in `_structmodule_exec`
@chgnrdv
Copy link
Contributor

chgnrdv commented Nov 14, 2023

I'll submit a PR :)

@chgnrdv chgnrdv mentioned this issue Nov 14, 2023
Merged
kumaraditya303 pushed a commit that referenced this issue Nov 15, 2023
@chgnrdv
gh-112062: Make _struct module thread-safe in --disable-gil builds (
4744f59 
#112094)

* gh-112062: Make `_struct` module thread-safe in --disable-gil builds
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
@chgnrdv @aisk
pythongh-112062: Make _struct module thread-safe in --disable-gil…
06ceee9 
… builds (python#112094)

* pythongh-112062: Make `_struct` module thread-safe in --disable-gil builds
@swtaarrs swtaarrs mentioned this issue Mar 13, 2024
Open
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
@chgnrdv @Glyphack
pythongh-112062: Make _struct module thread-safe in --disable-gil…
630ac12 
… builds (python#112094)

* pythongh-112062: Make `_struct` module thread-safe in --disable-gil builds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chgnrdv chgnrdv

Labels
3.13 bugs and security fixes extension-modules C modules in the Modules dir topic-free-threading type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

gh-112062: Make _struct module thread-safe in --disable-gil builds chgnrdv/cpython
3 participants
@colesbury @chgnrdv @AlexWaygood