importlib: fix data race in imports (PyImport_ImportModuleLevelObject)

PyImport_ImportModuleLevelObject previously used the variable
module.__spec__._initializing to check if a module is currently being
initialized. This access could race with modifications to the module's
dict.

Instead, use the new md_initialized field on PyModule to check if the
module is already initialized. This can mean that a duck-typed module
doesn't get the benefit of the fast-path, but I think other than that
the change should still preserve the important behavior.

Include/internal/pycore_global_strings.h

@@ -449,6 +449,7 @@ struct _Py_global_strings {
         STRUCT_FOR_ID(initial)
         STRUCT_FOR_ID(initial_bytes)
         STRUCT_FOR_ID(initial_value)
+        STRUCT_FOR_ID(initialized)
         STRUCT_FOR_ID(initval)
         STRUCT_FOR_ID(inner_size)
         STRUCT_FOR_ID(input)

Include/internal/pycore_import.h

@@ -22,11 +22,7 @@ struct _import_runtime_state {
        Modules are added there and looked up in _imp.find_extension(). */
     PyObject *extensions;
     /* The global import lock. */
-    struct {
-        PyThread_type_lock mutex;
-        unsigned long thread;
-        int level;
-    } lock;
+    _PyRecursiveMutex lock;
     struct {
         int import_level;
         _PyTime_t accumulated;

Include/internal/pycore_moduleobject.h

@@ -16,6 +16,7 @@ typedef struct {
     PyObject *md_weaklist;
     // for logging purposes after md_dict is cleared
     PyObject *md_name;
+    int md_initialized;
 } PyModuleObject;
 
 static inline PyModuleDef* _PyModule_GetDef(PyObject *mod) {

Include/internal/pycore_runtime_init.h

@@ -35,11 +35,6 @@ extern "C" {
         }, \
         .parser = _parser_runtime_state_INIT, \
         .imports = { \
-            .lock = { \
-                .mutex = NULL, \
-                .thread = PYTHREAD_INVALID_THREAD_ID, \
-                .level = 0, \
-            }, \
             .find_and_load = { \
                 .header = 1, \
             }, \

Include/moduleobject.h

@@ -30,6 +30,8 @@ PyAPI_FUNC(PyObject *) PyModule_GetFilenameObject(PyObject *);
 #ifndef Py_LIMITED_API
 PyAPI_FUNC(void) _PyModule_Clear(PyObject *);
 PyAPI_FUNC(void) _PyModule_ClearDict(PyObject *);
+PyAPI_FUNC(int) _PyModule_IsInitialized(PyObject *);
+PyAPI_FUNC(void) _PyModule_SetInitialized(PyObject *self, int initialized);
 PyAPI_FUNC(int) _PyModuleSpec_IsInitializing(PyObject *);
 #endif
 PyAPI_FUNC(PyModuleDef*) PyModule_GetDef(PyObject*);

Lib/importlib/_bootstrap.py

@@ -671,6 +671,7 @@ def _load_unlocked(spec):
     # (otherwise an optimization shortcut in import.c becomes
     # wrong).
     spec._initializing = True
+    _imp.module_initialized(module, False)
     try:
         sys.modules[spec.name] = module
         try:
@@ -692,6 +693,7 @@ def _load_unlocked(spec):
         # their own.
         module = sys.modules.pop(spec.name)
         sys.modules[spec.name] = module
+        _imp.module_initialized(module, True)
         _verbose_message('import {!r} # {!r}', spec.name, spec.loader)
     finally:
         spec._initializing = False

Lib/test/test_sys.py

@@ -1469,7 +1469,7 @@ def get_gen(): yield 1
         check(int(PyLong_BASE**2-1), vsize('') + 2*self.longdigit)
         check(int(PyLong_BASE**2), vsize('') + 3*self.longdigit)
         # module
-        check(unittest, size('PnPPP'))
+        check(unittest, size('PnPPPi'))
         # None
         check(None, size(''))
         # NotImplementedType

Objects/moduleobject.c

@@ -51,6 +51,22 @@ PyModuleDef_Init(PyModuleDef* def)
     return (PyObject*)def;
 }
 
+int
+_PyModule_IsInitialized(PyObject *self)
+{
+    assert(PyModule_Check(self));
+    PyModuleObject *mod = (PyModuleObject*)self;
+    return _Py_atomic_load_int(&mod->md_initialized);
+}
+
+void
+_PyModule_SetInitialized(PyObject *self, int initialized)
+{
+    assert(PyModule_Check(self));
+    PyModuleObject *mod = (PyModuleObject*)self;
+    _Py_atomic_store_int(&mod->md_initialized, initialized);
+}
+
 static int
 module_init_dict(PyModuleObject *mod, PyObject *md_dict,
                  PyObject *name, PyObject *doc)
@@ -70,7 +86,9 @@ module_init_dict(PyModuleObject *mod, PyObject *md_dict,
     if (PyDict_SetItem(md_dict, &_Py_ID(__spec__), Py_None) != 0)
         return -1;
     if (PyUnicode_CheckExact(name)) {
-        Py_XSETREF(mod->md_name, Py_NewRef(name));
+        Py_INCREF(name);
+        PyUnicode_InternInPlace(&name);
+        Py_XSETREF(mod->md_name, name);
     }
 
     return 0;
@@ -111,6 +129,7 @@ PyModule_NewObject(PyObject *name)
     PyModuleObject *m = new_module_notrack(&PyModule_Type);
     if (m == NULL)
         return NULL;
+    m->md_initialized = 1;
     if (module_init_dict(m, m->md_dict, name, NULL) != 0)
         goto fail;
     PyObject_GC_Track(m);
@@ -125,7 +144,7 @@ PyObject *
 PyModule_New(const char *name)
 {
     PyObject *nameobj, *module;
-    nameobj = PyUnicode_FromString(name);
+    nameobj = PyUnicode_InternFromString(name);
     if (nameobj == NULL)
         return NULL;
     module = PyModule_NewObject(nameobj);
@@ -254,6 +273,7 @@ _PyModule_CreateInitialized(PyModuleDef* module, int module_api_version)
         }
     }
     m->md_def = module;
+    m->md_initialized = 1;
     return (PyObject*)m;
 }
 
@@ -274,6 +294,7 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio
     if (nameobj == NULL) {
         return NULL;
     }
+    PyUnicode_InternInPlace(&nameobj);
     name = PyUnicode_AsUTF8(nameobj);
     if (name == NULL) {
         goto error;