From b365d88465d9228ce4e9e0be20b88e9e4056ad88 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 15 Feb 2023 16:05:07 -0700 Subject: [PATCH] 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. https://github.com/python/cpython/issues/101758 --- Include/internal/pycore_import.h | 3 ++ Lib/test/test_imp.py | 73 ++++++++++++++++++++++++++++++ Modules/_testinternalcapi.c | 15 +++++++ Modules/_testsinglephase.c | 49 +++++++++++++++++++- Python/import.c | 76 +++++++++++++++++++++++++++++++- 5 files changed, 212 insertions(+), 4 deletions(-) diff --git a/Include/internal/pycore_import.h b/Include/internal/pycore_import.h index da766253ef6..6ee7356b41c 100644 --- a/Include/internal/pycore_import.h +++ b/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 diff --git a/Lib/test/test_imp.py b/Lib/test/test_imp.py index c85ab92307d..e81eb6f0a86 100644 --- a/Lib/test/test_imp.py +++ b/Lib/test/test_imp.py @@ -10,10 +10,13 @@ from test.support import import_helper 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 @@ class ImportTests(unittest.TestCase): 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 @@ class ImportTests(unittest.TestCase): 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 diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index ba57719d920..632fac2de0c 100644 --- a/Modules/_testinternalcapi.c +++ b/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 */ }; diff --git a/Modules/_testsinglephase.c b/Modules/_testsinglephase.c index 9e8dd647ee7..565221c887e 100644 --- a/Modules/_testsinglephase.c +++ b/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 @@ error: 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 */ }; diff --git a/Python/import.c b/Python/import.c index ae27aaf5684..87981668a30 100644 --- a/Python/import.c +++ b/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;