Repository navigation
free-threading: PyImport_AddModuleRef race condition #137422
Description
Bug report
Bug description:
On a NoGIL interpreter, if two threads call PyImport_AddModuleRef for the same module name at the same time, and that module does not exist yet, it is possible that the two calls to the function return two different objects, only one of which is stored in sys.modules.
The race condition is actually in the helper function import_add_module:
Lines 319 to 335 in a50822f
| PyObject *m; | |
| if (PyMapping_GetOptionalItem(modules, name, &m) < 0) { | |
| return NULL; | |
| } | |
| if (m != NULL && PyModule_Check(m)) { | |
| return m; | |
| } | |
| Py_XDECREF(m); | |
| m = PyModule_NewObject(name); | |
| if (m == NULL) | |
| return NULL; | |
| if (PyObject_SetItem(modules, name, m) != 0) { | |
| Py_DECREF(m); | |
| return NULL; | |
| } | |
| return m; |
if both threads call
PyMapping_GetOptionalItem before either has called PyObject_SetItem, you will get two different modules.
Context
Each SWIG module calls PyImport_AddModuleRef to retrieve or initialise a shared global state module with the hardcoded name swig_runtime_data5, and then stores its pointer in a module-local global variable.
SWIG is actually immune from this race condition because the call to PyImport_AddModuleRef always happens inside SWIG_init, which is an alias for PyInit_<modulename> which holds the GIL.
This race condition only affects lazy initialisation patterns, where PyImport_AddModuleRef is executed for the first time after module init.
CPython versions tested on:
CPython main branch
Operating systems tested on:
No response
Activity
serhiy-storchaka commentedon Aug 5, 2025
If
sys.modulesis a dict, we can use atomicPyDict_SetDefaultRef().But there is no such API for general mapping and there cannot be. So we will need to use some external locking. Or just make
sys.modulesalways a dict, I don't think there are still cases for a non-dict.kumaraditya303 commentedon Aug 6, 2025
Looking at the code, I think
interp->modulescan only ever be a dict, if the user code assigns sys.modules then that doesn't affect theinterp->modulesso we can make this thread safe by relying on the fact that it is always a dict.I verified it by adding the following assertion and all tests passed:
Make "PyImport_AddModuleRef" somewhat thread safe (GH-7076)
py-cython: update to 3.2.0.
colesbury commentedon Nov 21, 2025
Yeah, I think that's right, even though it's confusing. It seems like it's been that way since PyImport_AddModule was added in 1.5.
It's kind of weird that if you change
sys.modulesyou affect the Python importlib code path, but not the Python import.c code paths.pythongh-137422: Fix race condition in PyImport_AddModuleRef
gh-137422: Fix race condition in PyImport_AddModuleRef (gh-141822)
pythongh-137422: Fix race condition in PyImport_AddModuleRef (pythong…
colesbury commentedon Nov 21, 2025
This is fixed in main (3.15) and backported to 3.14. I'm not planning on backporting it to 3.13 unless someone asks for it
[3.14] gh-137422: Fix race condition in PyImport_AddModuleRef (gh-141822
pythongh-137422: Fix race condition in PyImport_AddModuleRef (pythong…
pythongh-137422: Fix race condition in PyImport_AddModuleRef (pythong…
chore(deps): update dependency cython to ~=3.2.4 (2024.6)
py-cython: update to 3.2.0.
py-cython: update to 3.2.0.
py-cython: update to 3.2.0.