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;