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

unicode: make _PyUnicode_FromId thread-safe in --disable-gil builds #111971

Closed
Tracked by #108219
colesbury opened this issue Nov 10, 2023 · 4 comments
Closed
Tracked by #108219

unicode: make _PyUnicode_FromId thread-safe in --disable-gil builds #111971

colesbury opened this issue Nov 10, 2023 · 4 comments
Assignees
Labels
3.13 bugs and security fixes topic-free-threading type-feature A feature request or enhancement

Comments

@colesbury
Copy link
Contributor

colesbury commented Nov 10, 2023

Feature or enhancement

The _PyUnicode_FromId(_Py_Identifier *id) function gets a Python str object (i.e., a PyUnicodeObject) from a static C string. Subsequent calls to _PyUnicode_FromId() return the same object. The initialization is not currently thread-safe without the GIL.

Mostly for my own reference, here is the implementation from nogil-3.12: colesbury/nogil-3.12@6540bf3e6a. We will want to do things a bit differently in CPython 3.13.

Linked PRs

@colesbury colesbury added type-feature A feature request or enhancement 3.13 bugs and security fixes topic-free-threading labels Nov 10, 2023
@colesbury colesbury changed the title Make _PyUnicode_FromId thread-safe in --disable-gil builds unicode: make _PyUnicode_FromId thread-safe in --disable-gil builds Nov 10, 2023
@corona10
Copy link
Member

@colesbury Can I take a look at this issue too?

@colesbury
Copy link
Contributor Author

@corona10, sure. I think this requires some investigation to figure out what the right thing to do. From a quick glance, we've gotten rid of _Py_IDENTIFIER() in CPython, which is great! Since we only are keeping it to support "naught PyPI modules" we might be able to get away with just some simple locking.

@corona10 corona10 self-assigned this Dec 20, 2023
@corona10
Copy link
Member

@colesbury
You are right.
Internally, we are not using _Py_IDENTIFIER() anymore.

Since we only are keeping it to support "naught PyPI modules" we might be able to get away with just some simple locking.

For sync our implementation strategy:
So you mean that adding a mutex or lock to _Py_IDENTIFIER struct and then use the new lock from _PyUnicode_FromId side? (Allow for slight performance loss in naught PyPI modules.)

@corona10
Copy link
Member

Now we can close this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes topic-free-threading type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants