https://github.com/python/cpython/commit/bd9983cab89cc42eecdbb4556cca0b6d7a7c529c commit: bd9983cab89cc42eecdbb4556cca0b6d7a7c529c branch: 3.13 author: Miss Islington (bot) <[email protected]> committer: ericsnowcurrently <[email protected]> date: 2024-05-27T19:11:29Z summary:
[3.13] gh-119560: Drop an Invalid Assert in PyState_FindModule() (gh-119561) (gh-119632) The assertion was added in gh-118532 but was based on the invalid assumption that PyState_FindModule() would only be called with an already-initialized module def. I've added a test to make sure we don't make that assumption again. (cherry picked from commit 0c5ebe13e9937c446e9947c44f2570737ecca135) Co-authored-by: Eric Snow <[email protected]> files: A Misc/NEWS.d/next/Core and Builtins/2024-05-25-12-52-25.gh-issue-119560.wSlm8q.rst M Lib/test/test_import/__init__.py M Modules/_testsinglephase.c M Python/import.c diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 64282d0f2d0bcf..11eaae5e47e97a 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -2887,6 +2887,13 @@ def test_with_reinit_reloaded(self): self.assertIs(reloaded.snapshot.cached, reloaded.module) + def test_check_state_first(self): + for variant in ['', '_with_reinit', '_with_state']: + name = f'{self.NAME}{variant}_check_cache_first' + with self.subTest(name): + mod = self._load_dynamic(name, self.ORIGIN) + self.assertEqual(mod.__name__, name) + # Currently, for every single-phrase init module loaded # in multiple interpreters, those interpreters share a # PyModuleDef for that object, which can be a problem. diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-05-25-12-52-25.gh-issue-119560.wSlm8q.rst b/Misc/NEWS.d/next/Core and Builtins/2024-05-25-12-52-25.gh-issue-119560.wSlm8q.rst new file mode 100644 index 00000000000000..3a28a94df0f7cf --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-05-25-12-52-25.gh-issue-119560.wSlm8q.rst @@ -0,0 +1,3 @@ +An invalid assert in beta 1 has been removed. The assert would fail if +``PyState_FindModule()`` was used in an extension module's init function +before the module def had been initialized. diff --git a/Modules/_testsinglephase.c b/Modules/_testsinglephase.c index 448be502466e79..bcdb5ba31842fd 100644 --- a/Modules/_testsinglephase.c +++ b/Modules/_testsinglephase.c @@ -1,7 +1,7 @@ /* Testing module for single-phase initialization of extension modules -This file contains 5 distinct modules, meaning each as its own name +This file contains 8 distinct modules, meaning each as its own name and its own init function (PyInit_...). The default import system will only find the one matching the filename: _testsinglephase. To load the others you must do so manually. For example: @@ -14,7 +14,7 @@ spec = importlib.util.spec_from_file_location(name, filename, loader=loader) mod = importlib._bootstrap._load(spec) ``` -Here are the 5 modules: +Here are the 8 modules: * _testsinglephase * def: _testsinglephase_basic, @@ -136,6 +136,32 @@ Here are the 5 modules: 5. increment <global>.initialized_count * functions: see common functions below * import system: same as _testsinglephase_basic_copy +* _testsinglephase_check_cache_first + * def: _testsinglepahse_check_cache_first + * m_name: "_testsinglephase_check_cache_first" + * m_size: -1 + * state: none + * init function: + * tries PyState_FindModule() first + * otherwise creates empty module + * functions: none + * import system: same as _testsinglephase +* _testsinglephase_with_reinit_check_cache_first + * def: _testsinglepahse_with_reinit_check_cache_first + * m_name: "_testsinglephase_with_reinit_check_cache_first" + * m_size: 0 + * state: none + * init function: same as _testsinglephase_check_cache_first + * functions: none + * import system: same as _testsinglephase_with_reinit +* _testsinglephase_with_state_check_cache_first + * def: _testsinglepahse_with_state_check_cache_first + * m_name: "_testsinglephase_with_state_check_cache_first" + * m_size: 42 + * state: none + * init function: same as _testsinglephase_check_cache_first + * functions: none + * import system: same as _testsinglephase_with_state Module state: @@ -650,3 +676,64 @@ PyInit__testsinglephase_with_state(void) finally: return module; } + + +/****************************************************/ +/* the _testsinglephase_*_check_cache_first modules */ +/****************************************************/ + +static struct PyModuleDef _testsinglephase_check_cache_first = { + PyModuleDef_HEAD_INIT, + .m_name = "_testsinglephase_check_cache_first", + .m_doc = PyDoc_STR("Test module _testsinglephase_check_cache_first"), + .m_size = -1, // no module state +}; + +PyMODINIT_FUNC +PyInit__testsinglephase_check_cache_first(void) +{ + assert(_testsinglephase_check_cache_first.m_base.m_index == 0); + PyObject *mod = PyState_FindModule(&_testsinglephase_check_cache_first); + if (mod != NULL) { + return Py_NewRef(mod); + } + return PyModule_Create(&_testsinglephase_check_cache_first); +} + + +static struct PyModuleDef _testsinglephase_with_reinit_check_cache_first = { + PyModuleDef_HEAD_INIT, + .m_name = "_testsinglephase_with_reinit_check_cache_first", + .m_doc = PyDoc_STR("Test module _testsinglephase_with_reinit_check_cache_first"), + .m_size = 0, // no module state +}; + +PyMODINIT_FUNC +PyInit__testsinglephase_with_reinit_check_cache_first(void) +{ + assert(_testsinglephase_with_reinit_check_cache_first.m_base.m_index == 0); + PyObject *mod = PyState_FindModule(&_testsinglephase_with_reinit_check_cache_first); + if (mod != NULL) { + return Py_NewRef(mod); + } + return PyModule_Create(&_testsinglephase_with_reinit_check_cache_first); +} + + +static struct PyModuleDef _testsinglephase_with_state_check_cache_first = { + PyModuleDef_HEAD_INIT, + .m_name = "_testsinglephase_with_state_check_cache_first", + .m_doc = PyDoc_STR("Test module _testsinglephase_with_state_check_cache_first"), + .m_size = 42, // not used +}; + +PyMODINIT_FUNC +PyInit__testsinglephase_with_state_check_cache_first(void) +{ + assert(_testsinglephase_with_state_check_cache_first.m_base.m_index == 0); + PyObject *mod = PyState_FindModule(&_testsinglephase_with_state_check_cache_first); + if (mod != NULL) { + return Py_NewRef(mod); + } + return PyModule_Create(&_testsinglephase_with_state_check_cache_first); +} diff --git a/Python/import.c b/Python/import.c index ba44477318d473..4f3325aa67bd0a 100644 --- a/Python/import.c +++ b/Python/import.c @@ -457,7 +457,6 @@ static Py_ssize_t _get_module_index_from_def(PyModuleDef *def) { Py_ssize_t index = def->m_base.m_index; - assert(index > 0); #ifndef NDEBUG struct extensions_cache_value *cached = _find_cached_def(def); assert(cached == NULL || index == _get_cached_module_index(cached)); @@ -489,7 +488,7 @@ _set_module_index(PyModuleDef *def, Py_ssize_t index) static const char * _modules_by_index_check(PyInterpreterState *interp, Py_ssize_t index) { - if (index == 0) { + if (index <= 0) { return "invalid module index"; } if (MODULES_BY_INDEX(interp) == NULL) { _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3/lists/python-checkins.python.org/ Member address: [email protected]
