Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-119560: Drop an Invalid Assert in PyState_FindModule() #119561

Merged
merged 4 commits into from
May 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Lib/test/test_import/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
91 changes: 89 additions & 2 deletions Modules/_testsinglephase.c
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -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,
Expand Down Expand Up @@ -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:

Expand Down Expand Up @@ -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);
}
3 changes: 1 addition & 2 deletions Python/import.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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) {
Expand Down
Loading