gh-112075: Make _PyDict_LoadGlobal thread safe - #117529
Conversation
Sorry, something went wrong.
|
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 548a7c2 🤖 If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
Sorry, something went wrong.
colesbury
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
| * key hash failed, key comparison failed, ...). Return NULL if the key doesn't | ||
| * exist. Return the value if the key exists. | ||
| * | ||
| * Returns a new reference. |
There was a problem hiding this comment.
I think we will want it to return a possibly deferred reference soon, but this is good for now.
Sorry, something went wrong.
| #ifdef Py_GIL_DISABLED | ||
| /* namespace 1: globals */ | ||
| ix = _Py_dict_lookup(globals, key, hash, &value); | ||
| ix = _Py_dict_lookup_threadsafe(globals, key, hash, &value); |
There was a problem hiding this comment.
I think this pattern might be simpler if we define _Py_dict_lookup_threadsafe in the default build as _Py_dict_lookup_threadsafe + Py_XNewRef().
We could use it in dict_subscript and dict_get_impl as well.
Sorry, something went wrong.
Make _PyDict_LoadGlobal threadsafe
Currently _PyDict_LoadGlobal is using the non-thread safe _Py_dict_lookup and isn't locking the dictionaries. This switches to using the thread safe version and modifies the function to return a new reference.
It also adds an assertion for _Py_dict_lookup that the dictionary should be locked, and fixes up ordered dict to use the thread safe version where the assertion trips.