summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Doc/c-api/module.rst44
-rw-r--r--Doc/whatsnew/3.9.rst11
-rw-r--r--Lib/test/test_importlib/extension/test_loader.py23
-rw-r--r--Misc/NEWS.d/next/C API/2020-03-02-11-29-45.bpo-39824.71_ZMn.rst10
-rw-r--r--Modules/_localemodule.c8
-rw-r--r--Modules/_testmultiphase.c54
-rw-r--r--Modules/atexitmodule.c27
-rw-r--r--Modules/audioop.c8
-rw-r--r--Modules/binascii.c14
-rw-r--r--Objects/moduleobject.c36
10 files changed, 91 insertions, 144 deletions
diff --git a/Doc/c-api/module.rst b/Doc/c-api/module.rst
index 57902a9..8d1a0fb 100644
--- a/Doc/c-api/module.rst
+++ b/Doc/c-api/module.rst
@@ -196,23 +196,47 @@ or request "multi-phase initialization" by returning the definition struct itsel
.. c:member:: traverseproc m_traverse
A traversal function to call during GC traversal of the module object, or
- ``NULL`` if not needed. This function may be called before module state
- is allocated (:c:func:`PyModule_GetState()` may return `NULL`),
- and before the :c:member:`Py_mod_exec` function is executed.
+ ``NULL`` if not needed.
+
+ This function is not called if the module state was requested but is not
+ allocated yet. This is the case immediately after the module is created
+ and before the module is executed (:c:data:`Py_mod_exec` function). More
+ precisely, this function is not called if :c:member:`m_size` is greater
+ than 0 and the module state (as returned by :c:func:`PyModule_GetState`)
+ is ``NULL``.
+
+ .. versionchanged:: 3.9
+ No longer called before the module state is allocated.
.. c:member:: inquiry m_clear
A clear function to call during GC clearing of the module object, or
- ``NULL`` if not needed. This function may be called before module state
- is allocated (:c:func:`PyModule_GetState()` may return `NULL`),
- and before the :c:member:`Py_mod_exec` function is executed.
+ ``NULL`` if not needed.
+
+ This function is not called if the module state was requested but is not
+ allocated yet. This is the case immediately after the module is created
+ and before the module is executed (:c:data:`Py_mod_exec` function). More
+ precisely, this function is not called if :c:member:`m_size` is greater
+ than 0 and the module state (as returned by :c:func:`PyModule_GetState`)
+ is ``NULL``.
+
+ .. versionchanged:: 3.9
+ No longer called before the module state is allocated.
.. c:member:: freefunc m_free
- A function to call during deallocation of the module object, or ``NULL`` if
- not needed. This function may be called before module state
- is allocated (:c:func:`PyModule_GetState()` may return `NULL`),
- and before the :c:member:`Py_mod_exec` function is executed.
+ A function to call during deallocation of the module object, or ``NULL``
+ if not needed.
+
+ This function is not called if the module state was requested but is not
+ allocated yet. This is the case immediately after the module is created
+ and before the module is executed (:c:data:`Py_mod_exec` function). More
+ precisely, this function is not called if :c:member:`m_size` is greater
+ than 0 and the module state (as returned by :c:func:`PyModule_GetState`)
+ is ``NULL``.
+
+ .. versionchanged:: 3.9
+ No longer called before the module state is allocated.
Single-phase initialization
...........................
diff --git a/Doc/whatsnew/3.9.rst b/Doc/whatsnew/3.9.rst
index 8c087c6..fd3d333 100644
--- a/Doc/whatsnew/3.9.rst
+++ b/Doc/whatsnew/3.9.rst
@@ -503,6 +503,17 @@ Build and C API Changes
*tstate* parameter (``PyThreadState*``).
(Contributed by Victor Stinner in :issue:`38500`.)
+* Extension modules: :c:member:`~PyModuleDef.m_traverse`,
+ :c:member:`~PyModuleDef.m_clear` and :c:member:`~PyModuleDef.m_free`
+ functions of :c:type:`PyModuleDef` are no longer called if the module state
+ was requested but is not allocated yet. This is the case immediately after
+ the module is created and before the module is executed
+ (:c:data:`Py_mod_exec` function). More precisely, these functions are not called
+ if :c:member:`~PyModuleDef.m_size` is greater than 0 and the module state (as
+ returned by :c:func:`PyModule_GetState`) is ``NULL``.
+
+ Extension modules without module state (``m_size <= 0``) are not affected.
+
Deprecated
==========
diff --git a/Lib/test/test_importlib/extension/test_loader.py b/Lib/test/test_importlib/extension/test_loader.py
index 9ad05fa..abd612f 100644
--- a/Lib/test/test_importlib/extension/test_loader.py
+++ b/Lib/test/test_importlib/extension/test_loader.py
@@ -267,29 +267,6 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests):
self.assertEqual(module.__name__, name)
self.assertEqual(module.__doc__, "Module named in %s" % lang)
- @unittest.skipIf(not hasattr(sys, 'gettotalrefcount'),
- '--with-pydebug has to be enabled for this test')
- def test_bad_traverse(self):
- ''' Issue #32374: Test that traverse fails when accessing per-module
- state before Py_mod_exec was executed.
- (Multiphase initialization modules only)
- '''
- script = """if True:
- try:
- from test import support
- import importlib.util as util
- spec = util.find_spec('_testmultiphase')
- spec.name = '_testmultiphase_with_bad_traverse'
-
- with support.SuppressCrashReport():
- m = spec.loader.create_module(spec)
- except:
- # Prevent Python-level exceptions from
- # ending the process with non-zero status
- # (We are testing for a crash in C-code)
- pass"""
- assert_python_failure("-c", script)
-
(Frozen_MultiPhaseExtensionModuleTests,
Source_MultiPhaseExtensionModuleTests
diff --git a/Misc/NEWS.d/next/C API/2020-03-02-11-29-45.bpo-39824.71_ZMn.rst b/Misc/NEWS.d/next/C API/2020-03-02-11-29-45.bpo-39824.71_ZMn.rst
new file mode 100644
index 0000000..ae0a872
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2020-03-02-11-29-45.bpo-39824.71_ZMn.rst
@@ -0,0 +1,10 @@
+Extension modules: :c:member:`~PyModuleDef.m_traverse`,
+:c:member:`~PyModuleDef.m_clear` and :c:member:`~PyModuleDef.m_free` functions
+of :c:type:`PyModuleDef` are no longer called if the module state was requested
+but is not allocated yet. This is the case immediately after the module is
+created and before the module is executed (:c:data:`Py_mod_exec` function). More
+precisely, these functions are not called if :c:member:`~PyModuleDef.m_size` is
+greater than 0 and the module state (as returned by
+:c:func:`PyModule_GetState`) is ``NULL``.
+
+Extension modules without module state (``m_size <= 0``) are not affected.
diff --git a/Modules/_localemodule.c b/Modules/_localemodule.c
index f68debd..d2202bc 100644
--- a/Modules/_localemodule.c
+++ b/Modules/_localemodule.c
@@ -778,9 +778,7 @@ static int
locale_traverse(PyObject *m, visitproc visit, void *arg)
{
_locale_state *state = (_locale_state*)PyModule_GetState(m);
- if (state) {
- Py_VISIT(state->Error);
- }
+ Py_VISIT(state->Error);
return 0;
}
@@ -788,9 +786,7 @@ static int
locale_clear(PyObject *m)
{
_locale_state *state = (_locale_state*)PyModule_GetState(m);
- if (state) {
- Py_CLEAR(state->Error);
- }
+ Py_CLEAR(state->Error);
return 0;
}
diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c
index 4933abb..eadc46f 100644
--- a/Modules/_testmultiphase.c
+++ b/Modules/_testmultiphase.c
@@ -225,20 +225,18 @@ static int execfunc(PyObject *m)
/* Helper for module definitions; there'll be a lot of them */
-#define TEST_MODULE_DEF_EX(name, slots, methods, statesize, traversefunc) { \
+#define TEST_MODULE_DEF(name, slots, methods) { \
PyModuleDef_HEAD_INIT, /* m_base */ \
name, /* m_name */ \
PyDoc_STR("Test module " name), /* m_doc */ \
- statesize, /* m_size */ \
+ 0, /* m_size */ \
methods, /* m_methods */ \
slots, /* m_slots */ \
- traversefunc, /* m_traverse */ \
+ NULL, /* m_traverse */ \
NULL, /* m_clear */ \
NULL, /* m_free */ \
}
-#define TEST_MODULE_DEF(name, slots, methods) TEST_MODULE_DEF_EX(name, slots, methods, 0, NULL)
-
static PyModuleDef_Slot main_slots[] = {
{Py_mod_exec, execfunc},
{0, NULL},
@@ -622,52 +620,6 @@ PyInit__testmultiphase_exec_unreported_exception(PyObject *spec)
return PyModuleDef_Init(&def_exec_unreported_exception);
}
-static int
-bad_traverse(PyObject *self, visitproc visit, void *arg) {
- testmultiphase_state *m_state;
-
- m_state = PyModule_GetState(self);
-
- /* The following assertion mimics any traversal function that doesn't correctly handle
- * the case during module creation where the module state hasn't been created yet.
- *
- * The check that it is used to test only runs in debug mode, so it is OK that the
- * assert() will get compiled out in fully optimised release builds.
- */
- assert(m_state != NULL);
- Py_VISIT(m_state->integer);
- return 0;
-}
-
-static int
-execfunc_with_bad_traverse(PyObject *mod) {
- testmultiphase_state *m_state;
-
- m_state = PyModule_GetState(mod);
- if (m_state == NULL) {
- return -1;
- }
-
- m_state->integer = PyLong_FromLong(0x7fffffff);
- Py_INCREF(m_state->integer);
-
- return 0;
-}
-
-static PyModuleDef_Slot slots_with_bad_traverse[] = {
- {Py_mod_exec, execfunc_with_bad_traverse},
- {0, NULL}
-};
-
-static PyModuleDef def_with_bad_traverse = TEST_MODULE_DEF_EX(
- "_testmultiphase_with_bad_traverse", slots_with_bad_traverse, NULL,
- sizeof(testmultiphase_state), bad_traverse);
-
-PyMODINIT_FUNC
-PyInit__testmultiphase_with_bad_traverse(PyObject *spec) {
- return PyModuleDef_Init(&def_with_bad_traverse);
-}
-
/*** Helper for imp test ***/
static PyModuleDef imp_dummy_def = TEST_MODULE_DEF("imp_dummy", main_slots, testexport_methods);
diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c
index c150e44..8cef64c 100644
--- a/Modules/atexitmodule.c
+++ b/Modules/atexitmodule.c
@@ -229,15 +229,14 @@ atexit_m_traverse(PyObject *self, visitproc visit, void *arg)
atexitmodule_state *modstate;
modstate = (atexitmodule_state *)PyModule_GetState(self);
- if (modstate != NULL) {
- for (i = 0; i < modstate->ncallbacks; i++) {
- atexit_callback *cb = modstate->atexit_callbacks[i];
- if (cb == NULL)
- continue;
- Py_VISIT(cb->func);
- Py_VISIT(cb->args);
- Py_VISIT(cb->kwargs);
- }
+
+ for (i = 0; i < modstate->ncallbacks; i++) {
+ atexit_callback *cb = modstate->atexit_callbacks[i];
+ if (cb == NULL)
+ continue;
+ Py_VISIT(cb->func);
+ Py_VISIT(cb->args);
+ Py_VISIT(cb->kwargs);
}
return 0;
}
@@ -247,9 +246,7 @@ atexit_m_clear(PyObject *self)
{
atexitmodule_state *modstate;
modstate = (atexitmodule_state *)PyModule_GetState(self);
- if (modstate != NULL) {
- atexit_cleanup(modstate);
- }
+ atexit_cleanup(modstate);
return 0;
}
@@ -258,10 +255,8 @@ atexit_free(PyObject *m)
{
atexitmodule_state *modstate;
modstate = (atexitmodule_state *)PyModule_GetState(m);
- if (modstate != NULL) {
- atexit_cleanup(modstate);
- PyMem_Free(modstate->atexit_callbacks);
- }
+ atexit_cleanup(modstate);
+ PyMem_Free(modstate->atexit_callbacks);
}
PyDoc_STRVAR(atexit_unregister__doc__,
diff --git a/Modules/audioop.c b/Modules/audioop.c
index 467bd63..64cf981 100644
--- a/Modules/audioop.c
+++ b/Modules/audioop.c
@@ -1926,9 +1926,7 @@ static int
audioop_traverse(PyObject *module, visitproc visit, void *arg)
{
audioop_state *state = (audioop_state *)PyModule_GetState(module);
- if (state) {
- Py_VISIT(state->AudioopError);
- }
+ Py_VISIT(state->AudioopError);
return 0;
}
@@ -1936,9 +1934,7 @@ static int
audioop_clear(PyObject *module)
{
audioop_state *state = (audioop_state *)PyModule_GetState(module);
- if (state) {
- Py_CLEAR(state->AudioopError);
- }
+ Py_CLEAR(state->AudioopError);
return 0;
}
diff --git a/Modules/binascii.c b/Modules/binascii.c
index c63f3ba..f59cb7d 100644
--- a/Modules/binascii.c
+++ b/Modules/binascii.c
@@ -1653,10 +1653,8 @@ static int
binascii_traverse(PyObject *module, visitproc visit, void *arg)
{
binascii_state *state = get_binascii_state(module);
- if (state) {
- Py_VISIT(state->Error);
- Py_VISIT(state->Incomplete);
- }
+ Py_VISIT(state->Error);
+ Py_VISIT(state->Incomplete);
return 0;
}
@@ -1664,10 +1662,8 @@ static int
binascii_clear(PyObject *module)
{
binascii_state *state = get_binascii_state(module);
- if (state) {
- Py_CLEAR(state->Error);
- Py_CLEAR(state->Incomplete);
- }
+ Py_CLEAR(state->Error);
+ Py_CLEAR(state->Incomplete);
return 0;
}
@@ -1686,7 +1682,7 @@ static struct PyModuleDef binasciimodule = {
binascii_slots,
binascii_traverse,
binascii_clear,
- binascii_free
+ binascii_free
};
PyMODINIT_FUNC
diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c
index c581951..f02ca75 100644
--- a/Objects/moduleobject.c
+++ b/Objects/moduleobject.c
@@ -26,16 +26,6 @@ static PyMemberDef module_members[] = {
};
-/* Helper for sanity check for traverse not handling m_state == NULL
- * Issue #32374 */
-#ifdef Py_DEBUG
-static int
-bad_traverse_test(PyObject *self, void *arg) {
- assert(self != NULL);
- return 0;
-}
-#endif
-
PyTypeObject PyModuleDef_Type = {
PyVarObject_HEAD_INIT(&PyType_Type, 0)
"moduledef", /* tp_name */
@@ -360,16 +350,6 @@ PyModule_FromDefAndSpec2(struct PyModuleDef* def, PyObject *spec, int module_api
}
}
- /* Sanity check for traverse not handling m_state == NULL
- * This doesn't catch all possible cases, but in many cases it should
- * make many cases of invalid code crash or raise Valgrind issues
- * sooner than they would otherwise.
- * Issue #32374 */
-#ifdef Py_DEBUG
- if (def->m_traverse != NULL) {
- def->m_traverse(m, bad_traverse_test, NULL);
- }
-#endif
Py_DECREF(nameobj);
return m;
@@ -687,8 +667,12 @@ module_dealloc(PyModuleObject *m)
}
if (m->md_weaklist != NULL)
PyObject_ClearWeakRefs((PyObject *) m);
- if (m->md_def && m->md_def->m_free)
+ /* bpo-39824: Don't call m_free() if m_size > 0 and md_state=NULL */
+ if (m->md_def && m->md_def->m_free
+ && (m->md_def->m_size <= 0 || m->md_state != NULL))
+ {
m->md_def->m_free(m);
+ }
Py_XDECREF(m->md_dict);
Py_XDECREF(m->md_name);
if (m->md_state != NULL)
@@ -770,7 +754,10 @@ module_getattro(PyModuleObject *m, PyObject *name)
static int
module_traverse(PyModuleObject *m, visitproc visit, void *arg)
{
- if (m->md_def && m->md_def->m_traverse) {
+ /* bpo-39824: Don't call m_traverse() if m_size > 0 and md_state=NULL */
+ if (m->md_def && m->md_def->m_traverse
+ && (m->md_def->m_size <= 0 || m->md_state != NULL))
+ {
int res = m->md_def->m_traverse((PyObject*)m, visit, arg);
if (res)
return res;
@@ -782,7 +769,10 @@ module_traverse(PyModuleObject *m, visitproc visit, void *arg)
static int
module_clear(PyModuleObject *m)
{
- if (m->md_def && m->md_def->m_clear) {
+ /* bpo-39824: Don't call m_clear() if m_size > 0 and md_state=NULL */
+ if (m->md_def && m->md_def->m_clear
+ && (m->md_def->m_size <= 0 || m->md_state != NULL))
+ {
int res = m->md_def->m_clear((PyObject*)m);
if (PyErr_Occurred()) {
PySys_FormatStderr("Exception ignored in m_clear of module%s%V\n",