gh-101758: Add a Test For Single-Phase Init Modules in Multiple Interpreters (gh-101920)

The test verifies the behavior of single-phase init modules when loaded in multiple interpreters.

#101758

Author: ericsnowcurrently

Include/internal/pycore_import.h

@@ -153,6 +153,9 @@ PyAPI_DATA(const struct _frozen *) _PyImport_FrozenStdlib;
 PyAPI_DATA(const struct _frozen *) _PyImport_FrozenTest;
 extern const struct _module_alias * _PyImport_FrozenAliases;
 
+// for testing
+PyAPI_FUNC(int) _PyImport_ClearExtension(PyObject *name, PyObject *filename);
+
 #ifdef __cplusplus
 }
 #endif

Lib/test/test_imp.py

@@ -10,10 +10,13 @@
 from test.support import os_helper
 from test.support import script_helper
 from test.support import warnings_helper
+import textwrap
 import unittest
 import warnings
 imp = warnings_helper.import_deprecated('imp')
 import _imp
+import _testinternalcapi
+import _xxsubinterpreters as _interpreters
 
 
 OS_PATH_NAME = os.path.__name__
@@ -251,6 +254,71 @@ def test_issue16421_multiple_modules_in_one_dll(self):
         with self.assertRaises(ImportError):
             imp.load_dynamic('nonexistent', pathname)
 
+    @requires_load_dynamic
+    def test_singlephase_multiple_interpreters(self):
+        # Currently, for every single-phrase init module loaded
+        # in multiple interpreters, those interpreters share a
+        # PyModuleDef for that object, which can be a problem.
+
+        # This single-phase module has global state, which is shared
+        # by the interpreters.
+        import _testsinglephase
+        name = _testsinglephase.__name__
+        filename = _testsinglephase.__file__
+
+        del sys.modules[name]
+        _testsinglephase._clear_globals()
+        _testinternalcapi.clear_extension(name, filename)
+        init_count = _testsinglephase.initialized_count()
+        assert init_count == -1, (init_count,)
+
+        def clean_up():
+            _testsinglephase._clear_globals()
+            _testinternalcapi.clear_extension(name, filename)
+        self.addCleanup(clean_up)
+
+        interp1 = _interpreters.create(isolated=False)
+        self.addCleanup(_interpreters.destroy, interp1)
+        interp2 = _interpreters.create(isolated=False)
+        self.addCleanup(_interpreters.destroy, interp2)
+
+        script = textwrap.dedent(f'''
+            import _testsinglephase
+
+            expected = %d
+            init_count =  _testsinglephase.initialized_count()
+            if init_count != expected:
+                raise Exception(init_count)
+
+            lookedup = _testsinglephase.look_up_self()
+            if lookedup is not _testsinglephase:
+                raise Exception((_testsinglephase, lookedup))
+
+            # Attrs set in the module init func are in m_copy.
+            _initialized = _testsinglephase._initialized
+            initialized = _testsinglephase.initialized()
+            if _initialized != initialized:
+                raise Exception((_initialized, initialized))
+
+            # Attrs set after loading are not in m_copy.
+            if hasattr(_testsinglephase, 'spam'):
+                raise Exception(_testsinglephase.spam)
+            _testsinglephase.spam = expected
+            ''')
+
+        # Use an interpreter that gets destroyed right away.
+        ret = support.run_in_subinterp(script % 1)
+        self.assertEqual(ret, 0)
+
+        # The module's init func gets run again.
+        # The module's globals did not get destroyed.
+        _interpreters.run_string(interp1, script % 2)
+
+        # The module's init func is not run again.
+        # The second interpreter copies the module's m_copy.
+        # However, globals are still shared.
+        _interpreters.run_string(interp2, script % 2)
+
     @requires_load_dynamic
     def test_singlephase_variants(self):
         '''Exercise the most meaningful variants described in Python/import.c.'''
@@ -260,6 +328,11 @@ def test_singlephase_variants(self):
         fileobj, pathname, _ = imp.find_module(basename)
         fileobj.close()
 
+        def clean_up():
+            import _testsinglephase
+            _testsinglephase._clear_globals()
+        self.addCleanup(clean_up)
+
         modules = {}
         def load(name):
             assert name not in modules

Modules/_testinternalcapi.c

@@ -671,6 +671,20 @@ get_interp_settings(PyObject *self, PyObject *args)
 }
 
 
+static PyObject *
+clear_extension(PyObject *self, PyObject *args)
+{
+    PyObject *name = NULL, *filename = NULL;
+    if (!PyArg_ParseTuple(args, "OO:clear_extension", &name, &filename)) {
+        return NULL;
+    }
+    if (_PyImport_ClearExtension(name, filename) < 0) {
+        return NULL;
+    }
+    Py_RETURN_NONE;
+}
+
+
 static PyMethodDef module_functions[] = {
     {"get_configs", get_configs, METH_NOARGS},
     {"get_recursion_depth", get_recursion_depth, METH_NOARGS},
@@ -692,6 +706,7 @@ static PyMethodDef module_functions[] = {
     _TESTINTERNALCAPI_COMPILER_CODEGEN_METHODDEF
     _TESTINTERNALCAPI_OPTIMIZE_CFG_METHODDEF
     {"get_interp_settings", get_interp_settings, METH_VARARGS, NULL},
+    {"clear_extension", clear_extension, METH_VARARGS, NULL},
     {NULL, NULL} /* sentinel */
 };
 

Modules/_testsinglephase.c

@@ -17,12 +17,27 @@ typedef struct {
     PyObject *str_const;
 } module_state;
 
+
 /* Process-global state is only used by _testsinglephase
    since it's the only one that does not support re-init. */
 static struct {
     int initialized_count;
     module_state module;
-} global_state = { .initialized_count = -1 };
+} global_state = {
+
+#define NOT_INITIALIZED -1
+    .initialized_count = NOT_INITIALIZED,
+};
+
+static void clear_state(module_state *state);
+
+static void
+clear_global_state(void)
+{
+    clear_state(&global_state.module);
+    global_state.initialized_count = NOT_INITIALIZED;
+}
+
 
 static inline module_state *
 get_module_state(PyObject *module)
@@ -106,6 +121,7 @@ init_state(module_state *state)
     return -1;
 }
 
+
 static int
 init_module(PyObject *module, module_state *state)
 {
@@ -118,6 +134,16 @@ init_module(PyObject *module, module_state *state)
     if (PyModule_AddObjectRef(module, "str_const", state->str_const) != 0) {
         return -1;
     }
+
+    double d = _PyTime_AsSecondsDouble(state->initialized);
+    PyObject *initialized = PyFloat_FromDouble(d);
+    if (initialized == NULL) {
+        return -1;
+    }
+    if (PyModule_AddObjectRef(module, "_initialized", initialized) != 0) {
+        return -1;
+    }
+
     return 0;
 }
 
@@ -198,10 +224,28 @@ basic_initialized_count(PyObject *self, PyObject *Py_UNUSED(ignored))
 }
 
 #define INITIALIZED_COUNT_METHODDEF \
-    {"initialized_count", basic_initialized_count, METH_VARARGS, \
+    {"initialized_count", basic_initialized_count, METH_NOARGS, \
      basic_initialized_count_doc}
 
 
+PyDoc_STRVAR(basic__clear_globals_doc,
+"_clear_globals()\n\
+\n\
+Free all global state and set it to uninitialized.");
+
+static PyObject *
+basic__clear_globals(PyObject *self, PyObject *Py_UNUSED(ignored))
+{
+    assert(PyModule_GetDef(self)->m_size == -1);
+    clear_global_state();
+    Py_RETURN_NONE;
+}
+
+#define _CLEAR_GLOBALS_METHODDEF \
+    {"_clear_globals", basic__clear_globals, METH_NOARGS, \
+     basic__clear_globals_doc}
+
+
 /*********************************************/
 /* the _testsinglephase module (and aliases) */
 /*********************************************/
@@ -223,6 +267,7 @@ static PyMethodDef TestMethods_Basic[] = {
     SUM_METHODDEF,
     INITIALIZED_METHODDEF,
     INITIALIZED_COUNT_METHODDEF,
+    _CLEAR_GLOBALS_METHODDEF,
     {NULL, NULL}           /* sentinel */
 };
 

Python/import.c

@@ -632,6 +632,28 @@ exec_builtin_or_dynamic(PyObject *mod) {
 }
 
 
+static int clear_singlephase_extension(PyInterpreterState *interp,
+                                       PyObject *name, PyObject *filename);
+
+// Currently, this is only used for testing.
+// (See _testinternalcapi.clear_extension().)
+int
+_PyImport_ClearExtension(PyObject *name, PyObject *filename)
+{
+    PyInterpreterState *interp = _PyInterpreterState_GET();
+
+    /* Clearing a module's C globals is up to the module. */
+    if (clear_singlephase_extension(interp, name, filename) < 0) {
+        return -1;
+    }
+
+    // In the future we'll probably also make sure the extension's
+    // file handle (and DL handle) is closed (requires saving it).
+
+    return 0;
+}
+
+
 /*******************/
 
 #if defined(__EMSCRIPTEN__) && defined(PY_CALL_TRAMPOLINE)
@@ -766,8 +788,30 @@ _extensions_cache_set(PyObject *filename, PyObject *name, PyModuleDef *def)
     return 0;
 }
 
+static int
+_extensions_cache_delete(PyObject *filename, PyObject *name)
+{
+    PyObject *extensions = EXTENSIONS;
+    if (extensions == NULL) {
+        return 0;
+    }
+    PyObject *key = PyTuple_Pack(2, filename, name);
+    if (key == NULL) {
+        return -1;
+    }
+    if (PyDict_DelItem(extensions, key) < 0) {
+        if (!PyErr_ExceptionMatches(PyExc_KeyError)) {
+            Py_DECREF(key);
+            return -1;
+        }
+        PyErr_Clear();
+    }
+    Py_DECREF(key);
+    return 0;
+}
+
 static void
-_extensions_cache_clear(void)
+_extensions_cache_clear_all(void)
 {
     Py_CLEAR(EXTENSIONS);
 }
@@ -890,6 +934,34 @@ import_find_extension(PyThreadState *tstate, PyObject *name,
     return mod;
 }
 
+static int
+clear_singlephase_extension(PyInterpreterState *interp,
+                            PyObject *name, PyObject *filename)
+{
+    PyModuleDef *def = _extensions_cache_get(filename, name);
+    if (def == NULL) {
+        if (PyErr_Occurred()) {
+            return -1;
+        }
+        return 0;
+    }
+
+    /* Clear data set when the module was initially loaded. */
+    def->m_base.m_init = NULL;
+    Py_CLEAR(def->m_base.m_copy);
+    // We leave m_index alone since there's no reason to reset it.
+
+    /* Clear the PyState_*Module() cache entry. */
+    if (_modules_by_index_check(interp, def->m_base.m_index) == NULL) {
+        if (_modules_by_index_clear(interp, def) < 0) {
+            return -1;
+        }
+    }
+
+    /* Clear the cached module def. */
+    return _extensions_cache_delete(filename, name);
+}
+
 
 /*******************/
 /* builtin modules */
@@ -2633,7 +2705,7 @@ void
 _PyImport_Fini(void)
 {
     /* Destroy the database used by _PyImport_{Fixup,Find}Extension */
-    _extensions_cache_clear();
+    _extensions_cache_clear_all();
     if (import_lock != NULL) {
         PyThread_free_lock(import_lock);
         import_lock = NULL;