From 65828422c6006b698ba10743a210ddcda58738f1 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 21 May 2025 15:11:12 +0200 Subject: [PATCH 01/51] Define PyMODEXPORT_FUNC --- Include/exports.h | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/Include/exports.h b/Include/exports.h index 0c646d5beb6ad6..62feb09ed2b433 100644 --- a/Include/exports.h +++ b/Include/exports.h @@ -9,6 +9,7 @@ inside the Python core, they are private to the core. If in an extension module, it may be declared with external linkage depending on the platform. + PyMODEXPORT_FUNC: Like PyMODINIT_FUNC, but for a slots array As a number of platforms support/require "__declspec(dllimport/dllexport)", we support a HAVE_DECLSPEC_DLL macro to save duplication. @@ -62,9 +63,9 @@ /* module init functions inside the core need no external linkage */ /* except for Cygwin to handle embedding */ # if defined(__CYGWIN__) -# define PyMODINIT_FUNC Py_EXPORTED_SYMBOL PyObject* +# define _PyINIT_FUNC_DECLSPEC Py_EXPORTED_SYMBOL # else /* __CYGWIN__ */ -# define PyMODINIT_FUNC PyObject* +# define _PyINIT_FUNC_DECLSPEC # endif /* __CYGWIN__ */ # else /* Py_BUILD_CORE */ /* Building an extension module, or an embedded situation */ @@ -78,9 +79,9 @@ # define PyAPI_DATA(RTYPE) extern Py_IMPORTED_SYMBOL RTYPE /* module init functions outside the core must be exported */ # if defined(__cplusplus) -# define PyMODINIT_FUNC extern "C" Py_EXPORTED_SYMBOL PyObject* +# define _PyINIT_FUNC_DECLSPEC extern "C" Py_EXPORTED_SYMBOL # else /* __cplusplus */ -# define PyMODINIT_FUNC Py_EXPORTED_SYMBOL PyObject* +# define _PyINIT_FUNC_DECLSPEC Py_EXPORTED_SYMBOL # endif /* __cplusplus */ # endif /* Py_BUILD_CORE */ # endif /* HAVE_DECLSPEC_DLL */ @@ -93,13 +94,15 @@ #ifndef PyAPI_DATA # define PyAPI_DATA(RTYPE) extern Py_EXPORTED_SYMBOL RTYPE #endif -#ifndef PyMODINIT_FUNC +#ifndef _PyINIT_FUNC_DECLSPEC # if defined(__cplusplus) -# define PyMODINIT_FUNC extern "C" Py_EXPORTED_SYMBOL PyObject* +# define _PyINIT_FUNC_DECLSPEC extern "C" Py_EXPORTED_SYMBOL # else /* __cplusplus */ -# define PyMODINIT_FUNC Py_EXPORTED_SYMBOL PyObject* +# define _PyINIT_FUNC_DECLSPEC Py_EXPORTED_SYMBOL # endif /* __cplusplus */ #endif +#define PyMODINIT_FUNC _PyINIT_FUNC_DECLSPEC PyObject* +#define PyMODEXPORT_FUNC _PyINIT_FUNC_DECLSPEC PyModuleDef_Slot* #endif /* Py_EXPORTS_H */ From 45cc9167d31a1fa8cc2c5ddc186e45f933cd9346 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 21 May 2025 15:13:48 +0200 Subject: [PATCH 02/51] Add new state to module object --- Include/internal/pycore_moduleobject.h | 6 ++++++ Objects/moduleobject.c | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/Include/internal/pycore_moduleobject.h b/Include/internal/pycore_moduleobject.h index b170d7bce702c6..382d11c923481f 100644 --- a/Include/internal/pycore_moduleobject.h +++ b/Include/internal/pycore_moduleobject.h @@ -27,6 +27,12 @@ typedef struct { #ifdef Py_GIL_DISABLED void *md_gil; #endif + Py_ssize_t md_size; + traverseproc md_traverse; + inquiry md_clear; + freefunc md_free; + void *md_token; + int (*md_exec)(PyObject *); } PyModuleObject; static inline PyModuleDef* _PyModule_GetDef(PyObject *mod) { diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index ba86b41e945e9d..e716683bb00ea1 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -96,6 +96,12 @@ new_module_notrack(PyTypeObject *mt) m->md_state = NULL; m->md_weaklist = NULL; m->md_name = NULL; + m->md_size = 0; + m->md_traverse = NULL; + m->md_clear = NULL; + m->md_free = NULL; + m->md_exec = NULL; + m->md_token = NULL; m->md_dict = PyDict_New(); if (m->md_dict == NULL) { Py_DECREF(m); From fc121c2d0cc86f121616ff434dfd38d9b359b741 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 21 May 2025 15:18:36 +0200 Subject: [PATCH 03/51] Make sure the module def matches the state --- Objects/moduleobject.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index e716683bb00ea1..27a58fd58b9108 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -26,6 +26,18 @@ static PyMemberDef module_members[] = { {0} }; +static void +assert_def_missing_or_redundant(PyModuleObject *m) { + if (m->md_def) { +#define DO_ASSERT(F) assert (m->md_def->m_ ## F == m->md_ ## F); + DO_ASSERT(size); + DO_ASSERT(traverse); + DO_ASSERT(clear); + DO_ASSERT(free); +#undef DO_ASSERT + } +} + PyTypeObject PyModuleDef_Type = { PyVarObject_HEAD_INIT(&PyType_Type, 0) @@ -216,6 +228,16 @@ PyModule_Create2(PyModuleDef* module, int module_api_version) return _PyModule_CreateInitialized(module, module_api_version); } +static void +module_set_def(PyModuleObject *md, PyModuleDef *def) +{ + md->md_def = def; + md->md_size = def->m_size; + md->md_traverse = def->m_traverse; + md->md_clear = def->m_clear; + md->md_free = def->m_free; +} + PyObject * _PyModule_CreateInitialized(PyModuleDef* module, int module_api_version) { @@ -262,7 +284,7 @@ _PyModule_CreateInitialized(PyModuleDef* module, int module_api_version) return NULL; } } - m->md_def = module; + module_set_def(m, module); #ifdef Py_GIL_DISABLED m->md_gil = Py_MOD_GIL_USED; #endif @@ -403,7 +425,7 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio if (PyModule_Check(m)) { ((PyModuleObject*)m)->md_state = NULL; - ((PyModuleObject*)m)->md_def = def; + module_set_def(((PyModuleObject*)m), def); #ifdef Py_GIL_DISABLED ((PyModuleObject*)m)->md_gil = gil_slot; #else @@ -835,6 +857,7 @@ module_dealloc(PyObject *self) if (m->md_weaklist != NULL) PyObject_ClearWeakRefs((PyObject *) m); + assert_def_missing_or_redundant(m); /* 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)) @@ -1153,6 +1176,7 @@ module_traverse(PyObject *self, visitproc visit, void *arg) { PyModuleObject *m = _PyModule_CAST(self); + assert_def_missing_or_redundant(m); /* 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)) @@ -1171,6 +1195,7 @@ module_clear(PyObject *self) { PyModuleObject *m = _PyModule_CAST(self); + assert_def_missing_or_redundant(m); /* 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)) From a77d28aa5e372c3671bb256630285d210fe363c9 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 21 May 2025 15:25:58 +0200 Subject: [PATCH 04/51] Add simple test case --- Lib/test/test_capi/test_modexport.py | 29 ++++++++++++++++++++++++++++ Modules/_testmultiphase.c | 10 ++++++++++ 2 files changed, 39 insertions(+) create mode 100644 Lib/test/test_capi/test_modexport.py diff --git a/Lib/test/test_capi/test_modexport.py b/Lib/test/test_capi/test_modexport.py new file mode 100644 index 00000000000000..b89e1ab3109194 --- /dev/null +++ b/Lib/test/test_capi/test_modexport.py @@ -0,0 +1,29 @@ +import unittest +import importlib +import sys +from importlib.machinery import ExtensionFileLoader + +try: + import _testmultiphase +except ImportError: + _testmultiphase = None + +def import_extension_from_file(modname, filename, *, put_in_sys_modules=True): + loader = ExtensionFileLoader(modname, filename) + spec = importlib.util.spec_from_loader(modname, loader) + module = importlib.util.module_from_spec(spec) + loader.exec_module(module) + if put_in_sys_modules: + sys.modules[modname] = module + return module + + +class TestModExport(unittest.TestCase): + @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + def test_modexport(self): + modname = '_test_modexport' + filename = _testmultiphase.__file__ + module = import_extension_from_file(modname, filename, + put_in_sys_modules=False) + + self.assertEqual(module.__name__, modname) diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index bfec0678e2c669..991cb7ca528f7d 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -993,3 +993,13 @@ PyInit__test_no_multiple_interpreter_slot(void) { return PyModuleDef_Init(&no_multiple_interpreter_slot_def); } + +PyMODEXPORT_FUNC +PyModExport__test_from_modexport(PyObject *spec) +{ + static PyModuleDef_Slot from_modexport_slots[] = { + {Py_mod_name, "_test_from_modexport"}, + {0}, + }; + return from_modexport_slots; +} From 6a6812e0a1fb68ce5d7eaabfde24870057b8f2ab Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 21 May 2025 15:26:46 +0200 Subject: [PATCH 05/51] Add new slot definitions --- Include/moduleobject.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Include/moduleobject.h b/Include/moduleobject.h index 2a17c891dda811..3383087c4a3ef7 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -80,9 +80,20 @@ struct PyModuleDef_Slot { # define Py_mod_gil 4 #endif +#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= _Py_PACK_VERSION(3, 15) +#define Py_mod_name 5 +#define Py_mod_doc 6 +#define Py_mod_size 7 +#define Py_mod_methods 8 +#define Py_mod_traverse 9 +#define Py_mod_clear 10 +#define Py_mod_free 11 +#define Py_mod_token 12 +#endif + #ifndef Py_LIMITED_API -#define _Py_mod_LAST_SLOT 4 +#define _Py_mod_LAST_SLOT 12 #endif #endif /* New in 3.5 */ From 6b20e01548364eab17603fef0472e6d81bf6dfdb Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 21 May 2025 15:36:48 +0200 Subject: [PATCH 06/51] Add a test for PyModule_FromSlotsAndSpec --- Modules/Setup.stdlib.in | 2 +- Modules/_testcapi/module.c | 30 ++++++++++++++++++++++++++++++ Modules/_testcapi/parts.h | 1 + Modules/_testcapimodule.c | 3 +++ 4 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 Modules/_testcapi/module.c diff --git a/Modules/Setup.stdlib.in b/Modules/Setup.stdlib.in index 3a38a60a152e8c..ac2fc3abbe28bb 100644 --- a/Modules/Setup.stdlib.in +++ b/Modules/Setup.stdlib.in @@ -178,7 +178,7 @@ @MODULE__XXTESTFUZZ_TRUE@_xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c @MODULE__TESTBUFFER_TRUE@_testbuffer _testbuffer.c @MODULE__TESTINTERNALCAPI_TRUE@_testinternalcapi _testinternalcapi.c _testinternalcapi/test_lock.c _testinternalcapi/pytime.c _testinternalcapi/set.c _testinternalcapi/test_critical_sections.c _testinternalcapi/complex.c -@MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/heaptype.c _testcapi/abstract.c _testcapi/unicode.c _testcapi/dict.c _testcapi/set.c _testcapi/list.c _testcapi/tuple.c _testcapi/getargs.c _testcapi/datetime.c _testcapi/docstring.c _testcapi/mem.c _testcapi/watchers.c _testcapi/long.c _testcapi/float.c _testcapi/complex.c _testcapi/numbers.c _testcapi/structmember.c _testcapi/exceptions.c _testcapi/code.c _testcapi/buffer.c _testcapi/pyatomic.c _testcapi/run.c _testcapi/file.c _testcapi/codec.c _testcapi/immortal.c _testcapi/gc.c _testcapi/hash.c _testcapi/time.c _testcapi/bytes.c _testcapi/object.c _testcapi/monitoring.c _testcapi/config.c _testcapi/import.c _testcapi/frame.c _testcapi/type.c _testcapi/function.c +@MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/heaptype.c _testcapi/abstract.c _testcapi/unicode.c _testcapi/dict.c _testcapi/set.c _testcapi/list.c _testcapi/tuple.c _testcapi/getargs.c _testcapi/datetime.c _testcapi/docstring.c _testcapi/mem.c _testcapi/watchers.c _testcapi/long.c _testcapi/float.c _testcapi/complex.c _testcapi/numbers.c _testcapi/structmember.c _testcapi/exceptions.c _testcapi/code.c _testcapi/buffer.c _testcapi/pyatomic.c _testcapi/run.c _testcapi/file.c _testcapi/codec.c _testcapi/immortal.c _testcapi/gc.c _testcapi/hash.c _testcapi/time.c _testcapi/bytes.c _testcapi/object.c _testcapi/monitoring.c _testcapi/config.c _testcapi/import.c _testcapi/frame.c _testcapi/type.c _testcapi/function.c _testcapi/module.c @MODULE__TESTLIMITEDCAPI_TRUE@_testlimitedcapi _testlimitedcapi.c _testlimitedcapi/abstract.c _testlimitedcapi/bytearray.c _testlimitedcapi/bytes.c _testlimitedcapi/codec.c _testlimitedcapi/complex.c _testlimitedcapi/dict.c _testlimitedcapi/eval.c _testlimitedcapi/float.c _testlimitedcapi/heaptype_relative.c _testlimitedcapi/import.c _testlimitedcapi/list.c _testlimitedcapi/long.c _testlimitedcapi/object.c _testlimitedcapi/pyos.c _testlimitedcapi/set.c _testlimitedcapi/sys.c _testlimitedcapi/tuple.c _testlimitedcapi/unicode.c _testlimitedcapi/vectorcall_limited.c _testlimitedcapi/version.c _testlimitedcapi/file.c @MODULE__TESTCLINIC_TRUE@_testclinic _testclinic.c @MODULE__TESTCLINIC_LIMITED_TRUE@_testclinic_limited _testclinic_limited.c diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c new file mode 100644 index 00000000000000..a797894134bca2 --- /dev/null +++ b/Modules/_testcapi/module.c @@ -0,0 +1,30 @@ +#include "parts.h" +#include "util.h" + +// Test PyModule_* API + +static PyObject * +module_from_slots_and_spec(PyObject *self, PyObject *py_slots) +{ + assert(PyList_Check(py_slots)); + Py_ssize_t n_slots = PyList_GET_SIZE(py_slots); + PyModuleDef_Slot *slots = PyMem_Calloc(n_slots + 1, + sizeof(PyModuleDef_Slot)); + if (!slots) { + return PyErr_NoMemory(); + } + + return PyModule_FromSlotsAndSpec(slots, Py_None); +} + + +static PyMethodDef test_methods[] = { + {"module_from_slots_and_spec", module_from_slots_and_spec, METH_O}, + {NULL}, +}; + +int +_PyTestCapi_Init_Module(PyObject *m) +{ + return PyModule_AddFunctions(m, test_methods); +} diff --git a/Modules/_testcapi/parts.h b/Modules/_testcapi/parts.h index af6400162daf2b..af845957308c01 100644 --- a/Modules/_testcapi/parts.h +++ b/Modules/_testcapi/parts.h @@ -65,5 +65,6 @@ int _PyTestCapi_Init_Import(PyObject *mod); int _PyTestCapi_Init_Frame(PyObject *mod); int _PyTestCapi_Init_Type(PyObject *mod); int _PyTestCapi_Init_Function(PyObject *mod); +int _PyTestCapi_Init_Module(PyObject *mod); #endif // Py_TESTCAPI_PARTS_H diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 281c5b41137ac2..ccb4fbc8a7f53a 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3477,6 +3477,9 @@ PyInit__testcapi(void) if (_PyTestCapi_Init_Function(m) < 0) { return NULL; } + if (_PyTestCapi_Init_Module(m) < 0) { + return NULL; + } PyState_AddModule(m, &_testcapimodule); return m; From 25e41165bf8696b827d3f8205443937d2856fee6 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 21 May 2025 15:39:21 +0200 Subject: [PATCH 07/51] Add PyModule_FromSlotsAndSpec --- Include/moduleobject.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Include/moduleobject.h b/Include/moduleobject.h index 3383087c4a3ef7..7f64ff7ec74a7f 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -115,6 +115,12 @@ struct PyModuleDef_Slot { PyAPI_FUNC(int) PyUnstable_Module_SetGIL(PyObject *module, void *gil); #endif +#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= _Py_PACK_VERSION(3, 15) +PyAPI_FUNC(PyObject *) PyModule_FromSlotsAndSpec(PyModuleDef_Slot *, + PyObject *spec); +#endif + + struct PyModuleDef { PyModuleDef_Base m_base; const char* m_name; From 33b963a2e011fb92995d922662128ba553a8f9b9 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 21 May 2025 15:44:05 +0200 Subject: [PATCH 08/51] Add PyModule_FromSlotsAndSpec empty function --- Objects/moduleobject.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 27a58fd58b9108..e76c9e473a4113 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -472,6 +472,14 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio return NULL; } +PyObject * +PyModule_FromSlotsAndSpec(PyModuleDef_Slot *slots, PyObject *spec) +{ + PyObject *result = NULL; + return result; +} + + #ifdef Py_GIL_DISABLED int PyUnstable_Module_SetGIL(PyObject *module, void *gil) From 43c6d13199e67bbaf9ff195868485be6c36f6ea5 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 21 May 2025 15:47:58 +0200 Subject: [PATCH 09/51] Add module test --- Lib/test/test_capi/test_module.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 Lib/test/test_capi/test_module.py diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py new file mode 100644 index 00000000000000..cf67360b878012 --- /dev/null +++ b/Lib/test/test_capi/test_module.py @@ -0,0 +1,12 @@ +import unittest +import types +from test.support import import_helper + +# Skip this test if the _testcapi module isn't available. +_testcapi = import_helper.import_module('_testcapi') + + +class TestModExport(unittest.TestCase): + def test_modexport(self): + mod = _testcapi.module_from_slots_and_spec([]) + self.assertIsInstance(mod, types.ModuleType) From 892d095d4e749adb13b930b19ac976b3bc9fa77d Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 21 May 2025 15:53:12 +0200 Subject: [PATCH 10/51] Fill in enough of a PyModuleDef to pass to common machinery --- Objects/moduleobject.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index e76c9e473a4113..edae7a69ddaef7 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -476,6 +476,24 @@ PyObject * PyModule_FromSlotsAndSpec(PyModuleDef_Slot *slots, PyObject *spec) { PyObject *result = NULL; + if (!slots) { + PyErr_BadArgument(); + } + PyObject *nameobj = PyObject_GetAttrString(spec, "name"); + if (nameobj == NULL) { + goto finally; + } + const char *name = PyUnicode_AsUTF8(nameobj); + if (name == NULL) { + goto finally; + } + + // Fill in enough of a PyModuleDef to pass to common machinery + PyModuleDef def_like = {.m_slots = slots}; + + result = PyModule_FromDefAndSpec2(&def_like, spec, 3); +finally: + Py_XDECREF(nameobj); return result; } From 62b8c9e8ad89651e07fcef8fbc27d83c5f7361c0 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 21 May 2025 15:57:20 +0200 Subject: [PATCH 11/51] Pass in a spec --- Lib/test/test_capi/test_module.py | 10 +++++++--- Modules/_testcapi/module.c | 15 ++++++++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index cf67360b878012..b42d2f64576e82 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -6,7 +6,11 @@ _testcapi = import_helper.import_module('_testcapi') -class TestModExport(unittest.TestCase): - def test_modexport(self): - mod = _testcapi.module_from_slots_and_spec([]) +class TestModFromSlotsAndSpec(unittest.TestCase): + def test_empty(self): + spec_like = types.SimpleNamespace( + name='testmod', + ) + mod = _testcapi.module_from_slots_and_spec([], spec_like) self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(mod.__name__, 'testmod') diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index a797894134bca2..3aa9e2888a18c5 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -4,8 +4,15 @@ // Test PyModule_* API static PyObject * -module_from_slots_and_spec(PyObject *self, PyObject *py_slots) +module_from_slots_and_spec(PyObject *self, PyObject *args) { + PyObject *spec; + PyObject *py_slots; + if(PyArg_UnpackTuple(args, "module_from_slots_and_spec", 2, 2, + &py_slots, &spec) < 1) + { + return NULL; + } assert(PyList_Check(py_slots)); Py_ssize_t n_slots = PyList_GET_SIZE(py_slots); PyModuleDef_Slot *slots = PyMem_Calloc(n_slots + 1, @@ -14,12 +21,14 @@ module_from_slots_and_spec(PyObject *self, PyObject *py_slots) return PyErr_NoMemory(); } - return PyModule_FromSlotsAndSpec(slots, Py_None); + PyObject *result = PyModule_FromSlotsAndSpec(slots, spec); + PyMem_Free(slots); + return result; } static PyMethodDef test_methods[] = { - {"module_from_slots_and_spec", module_from_slots_and_spec, METH_O}, + {"module_from_slots_and_spec", module_from_slots_and_spec, METH_VARARGS}, {NULL}, }; From 53a556bd059899a60fd8b9e1b87ba063384ed0e8 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 10:54:21 +0200 Subject: [PATCH 12/51] Rename field which can now be NULL --- Include/internal/pycore_moduleobject.h | 8 +++++--- Objects/moduleobject.c | 27 ++++++++++++-------------- Objects/typeobject.c | 4 ++-- Python/import.c | 20 ++++++++++--------- Python/importdl.c | 2 +- 5 files changed, 31 insertions(+), 30 deletions(-) diff --git a/Include/internal/pycore_moduleobject.h b/Include/internal/pycore_moduleobject.h index 382d11c923481f..430eeced831244 100644 --- a/Include/internal/pycore_moduleobject.h +++ b/Include/internal/pycore_moduleobject.h @@ -19,7 +19,9 @@ extern int _PyModule_IsExtension(PyObject *obj); typedef struct { PyObject_HEAD PyObject *md_dict; - PyModuleDef *md_def; + // The PyModuleDef used to define the module, if any. + // (used to be `md_def` when all extension modules had one) + PyModuleDef *md_def_or_null; void *md_state; PyObject *md_weaklist; // for logging purposes after md_dict is cleared @@ -35,9 +37,9 @@ typedef struct { int (*md_exec)(PyObject *); } PyModuleObject; -static inline PyModuleDef* _PyModule_GetDef(PyObject *mod) { +static inline PyModuleDef* _PyModule_GetDefOrNull(PyObject *mod) { assert(PyModule_Check(mod)); - return ((PyModuleObject *)mod)->md_def; + return ((PyModuleObject *)mod)->md_def_or_null; } static inline void* _PyModule_GetState(PyObject* mod) { diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index edae7a69ddaef7..8486da29f3cb04 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -28,8 +28,8 @@ static PyMemberDef module_members[] = { static void assert_def_missing_or_redundant(PyModuleObject *m) { - if (m->md_def) { -#define DO_ASSERT(F) assert (m->md_def->m_ ## F == m->md_ ## F); + if (m->md_def_or_null) { +#define DO_ASSERT(F) assert (m->md_def_or_null->m_ ## F == m->md_ ## F); DO_ASSERT(size); DO_ASSERT(traverse); DO_ASSERT(clear); @@ -55,7 +55,7 @@ _PyModule_IsExtension(PyObject *obj) } PyModuleObject *module = (PyModuleObject*)obj; - PyModuleDef *def = module->md_def; + PyModuleDef *def = module->md_def_or_null; return (def != NULL && def->m_methods != NULL); } @@ -104,7 +104,7 @@ new_module_notrack(PyTypeObject *mt) m = (PyModuleObject *)_PyType_AllocNoTrack(mt, 0); if (m == NULL) return NULL; - m->md_def = NULL; + m->md_def_or_null = NULL; m->md_state = NULL; m->md_weaklist = NULL; m->md_name = NULL; @@ -231,7 +231,7 @@ PyModule_Create2(PyModuleDef* module, int module_api_version) static void module_set_def(PyModuleObject *md, PyModuleDef *def) { - md->md_def = def; + md->md_def_or_null = def; md->md_size = def->m_size; md->md_traverse = def->m_traverse; md->md_clear = def->m_clear; @@ -758,7 +758,7 @@ PyModule_GetDef(PyObject* m) PyErr_BadArgument(); return NULL; } - return _PyModule_GetDef(m); + return _PyModule_GetDefOrNull(m); } void* @@ -885,10 +885,9 @@ module_dealloc(PyObject *self) assert_def_missing_or_redundant(m); /* 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)) + if (m->md_free && (m->md_size <= 0 || m->md_state != NULL)) { - m->md_def->m_free(m); + m->md_free(m); } Py_XDECREF(m->md_dict); @@ -1204,10 +1203,9 @@ module_traverse(PyObject *self, visitproc visit, void *arg) assert_def_missing_or_redundant(m); /* 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)) + if (m->md_traverse && (m->md_size <= 0 || m->md_state != NULL)) { - int res = m->md_def->m_traverse((PyObject*)m, visit, arg); + int res = m->md_traverse((PyObject*)m, visit, arg); if (res) return res; } @@ -1223,10 +1221,9 @@ module_clear(PyObject *self) assert_def_missing_or_redundant(m); /* 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)) + if (m->md_clear && (m->md_size <= 0 || m->md_state != NULL)) { - int res = m->md_def->m_clear((PyObject*)m); + int res = m->md_clear((PyObject*)m); if (PyErr_Occurred()) { PyErr_FormatUnraisable("Exception ignored in m_clear of module%s%V", m->md_name ? " " : "", diff --git a/Objects/typeobject.c b/Objects/typeobject.c index b9d549610693c1..7c7443f6220cbf 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5715,7 +5715,7 @@ PyType_GetModuleByDef(PyTypeObject *type, PyModuleDef *def) else { PyHeapTypeObject *ht = (PyHeapTypeObject*)type; PyObject *module = ht->ht_module; - if (module && _PyModule_GetDef(module) == def) { + if (module && _PyModule_GetDefOrNull(module) == def) { return module; } } @@ -5743,7 +5743,7 @@ PyType_GetModuleByDef(PyTypeObject *type, PyModuleDef *def) PyHeapTypeObject *ht = (PyHeapTypeObject*)super; PyObject *module = ht->ht_module; - if (module && _PyModule_GetDef(module) == def) { + if (module && _PyModule_GetDefOrNull(module) == def) { res = module; break; } diff --git a/Python/import.c b/Python/import.c index 73b94d0dd2a1b1..fdaebc07cd80e7 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1801,7 +1801,7 @@ finish_singlephase_extension(PyThreadState *tstate, PyObject *mod, PyObject *name, PyObject *modules) { assert(mod != NULL && PyModule_Check(mod)); - assert(cached->def == _PyModule_GetDef(mod)); + assert(cached->def == _PyModule_GetDefOrNull(mod)); Py_ssize_t index = _get_cached_module_index(cached); if (_modules_by_index_set(tstate->interp, index, mod) < 0) { @@ -1879,8 +1879,8 @@ reload_singlephase_extension(PyThreadState *tstate, * due to violating interpreter isolation. * See the note in set_cached_m_dict(). * Until that is solved, we leave md_def set to NULL. */ - assert(_PyModule_GetDef(mod) == NULL - || _PyModule_GetDef(mod) == def); + assert(_PyModule_GetDefOrNull(mod) == NULL + || _PyModule_GetDefOrNull(mod) == def); } else { assert(cached->m_dict == NULL); @@ -2253,9 +2253,11 @@ _PyImport_FixupBuiltin(PyThreadState *tstate, PyObject *mod, const char *name, return -1; } - PyModuleDef *def = PyModule_GetDef(mod); + PyModuleDef *def = _PyModule_GetDefOrNull(mod); if (def == NULL) { - PyErr_BadInternalCall(); + if (!PyErr_Occurred()) { + PyErr_BadInternalCall(); + } goto finally; } @@ -2336,8 +2338,8 @@ create_builtin(PyThreadState *tstate, PyObject *name, PyObject *spec) assert(!_PyErr_Occurred(tstate)); assert(cached != NULL); /* The module might not have md_def set in certain reload cases. */ - assert(_PyModule_GetDef(mod) == NULL - || cached->def == _PyModule_GetDef(mod)); + assert(_PyModule_GetDefOrNull(mod) == NULL + || cached->def == _PyModule_GetDefOrNull(mod)); assert_singlephase(cached); goto finally; } @@ -4692,8 +4694,8 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file) assert(!_PyErr_Occurred(tstate)); assert(cached != NULL); /* The module might not have md_def set in certain reload cases. */ - assert(_PyModule_GetDef(mod) == NULL - || cached->def == _PyModule_GetDef(mod)); + assert(_PyModule_GetDefOrNull(mod) == NULL + || cached->def == _PyModule_GetDefOrNull(mod)); assert_singlephase(cached); goto finally; } diff --git a/Python/importdl.c b/Python/importdl.c index 802843fe7b9dce..b640f649aa1c65 100644 --- a/Python/importdl.c +++ b/Python/importdl.c @@ -496,7 +496,7 @@ _PyImport_RunModInitFunc(PyModInitFunction p0, goto error; } - res.def = _PyModule_GetDef(m); + res.def = _PyModule_GetDefOrNull(m); if (res.def == NULL) { PyErr_Clear(); _Py_ext_module_loader_result_set_error( From 3fda0f38cb1da837b2449b4dc5932d794544299c Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 11:28:13 +0200 Subject: [PATCH 13/51] Common implementation for PyModule_From{Def,Slots}AndSpec This uses a "def-like" structure: a PyModuleDef* that's not a valid Python object. --- Objects/moduleobject.c | 63 +++++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 22 deletions(-) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 8486da29f3cb04..e5b209ef24f029 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -229,13 +229,15 @@ PyModule_Create2(PyModuleDef* module, int module_api_version) } static void -module_set_def(PyModuleObject *md, PyModuleDef *def) +module_copy_members_from_deflike( + PyModuleObject *md, + PyModuleDef *def_like /* not necessarily a valid Python object */) { - md->md_def_or_null = def; - md->md_size = def->m_size; - md->md_traverse = def->m_traverse; - md->md_clear = def->m_clear; - md->md_free = def->m_free; + /* def may not be a valid PyObject*, see */ + md->md_size = def_like->m_size; + md->md_traverse = def_like->m_traverse; + md->md_clear = def_like->m_clear; + md->md_free = def_like->m_free; } PyObject * @@ -284,7 +286,8 @@ _PyModule_CreateInitialized(PyModuleDef* module, int module_api_version) return NULL; } } - module_set_def(m, module); + m->md_def_or_null = module; + module_copy_members_from_deflike(m, module); #ifdef Py_GIL_DISABLED m->md_gil = Py_MOD_GIL_USED; #endif @@ -292,7 +295,11 @@ _PyModule_CreateInitialized(PyModuleDef* module, int module_api_version) } PyObject * -PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_version) +module_from_def_and_spec( + PyModuleDef* def_like, /* not necessarily a valid Python object */ + PyObject *spec, + int module_api_version, + PyModuleDef* original_def /* NULL if not defined by a def */) { PyModuleDef_Slot* cur_slot; PyObject *(*create)(PyObject *, PyModuleDef*) = NULL; @@ -307,8 +314,6 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio int ret; PyInterpreterState *interp = _PyInterpreterState_GET(); - PyModuleDef_Init(def); - nameobj = PyObject_GetAttrString(spec, "name"); if (nameobj == NULL) { return NULL; @@ -322,7 +327,7 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio goto error; } - if (def->m_size < 0) { + if (def_like->m_size < 0) { PyErr_Format( PyExc_SystemError, "module %s: m_size may not be negative for multi-phase initialization", @@ -330,7 +335,7 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio goto error; } - for (cur_slot = def->m_slots; cur_slot && cur_slot->slot; cur_slot++) { + for (cur_slot = def_like->m_slots; cur_slot && cur_slot->slot; cur_slot++) { switch (cur_slot->slot) { case Py_mod_create: if (create) { @@ -398,7 +403,7 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio } if (create) { - m = create(spec, def); + m = create(spec, def_like); if (m == NULL) { if (!PyErr_Occurred()) { PyErr_Format( @@ -424,15 +429,21 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio } if (PyModule_Check(m)) { - ((PyModuleObject*)m)->md_state = NULL; - module_set_def(((PyModuleObject*)m), def); + PyModuleObject *mod = (PyModuleObject*)m; + mod->md_state = NULL; + module_copy_members_from_deflike(mod, def_like); + if (original_def) { + mod->md_def_or_null = original_def; + } #ifdef Py_GIL_DISABLED - ((PyModuleObject*)m)->md_gil = gil_slot; + mod->md_gil = gil_slot; #else (void)gil_slot; #endif } else { - if (def->m_size > 0 || def->m_traverse || def->m_clear || def->m_free) { + if (def_like->m_size > 0 || def_like->m_traverse || def_like->m_clear + || def_like->m_free) + { PyErr_Format( PyExc_SystemError, "module %s is not a module object, but requests module state", @@ -449,15 +460,15 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio } } - if (def->m_methods != NULL) { - ret = _add_methods_to_object(m, nameobj, def->m_methods); + if (def_like->m_methods != NULL) { + ret = _add_methods_to_object(m, nameobj, def_like->m_methods); if (ret != 0) { goto error; } } - if (def->m_doc != NULL) { - ret = PyModule_SetDocString(m, def->m_doc); + if (def_like->m_doc != NULL) { + ret = PyModule_SetDocString(m, def_like->m_doc); if (ret != 0) { goto error; } @@ -472,6 +483,13 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio return NULL; } +PyObject * +PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_version) +{ + PyModuleDef_Init(def); + return module_from_def_and_spec(def, spec, module_api_version, def); +} + PyObject * PyModule_FromSlotsAndSpec(PyModuleDef_Slot *slots, PyObject *spec) { @@ -491,7 +509,8 @@ PyModule_FromSlotsAndSpec(PyModuleDef_Slot *slots, PyObject *spec) // Fill in enough of a PyModuleDef to pass to common machinery PyModuleDef def_like = {.m_slots = slots}; - result = PyModule_FromDefAndSpec2(&def_like, spec, 3); + result = module_from_def_and_spec(&def_like, spec, PYTHON_API_VERSION, + NULL); finally: Py_XDECREF(nameobj); return result; From df180ec0cfa33d05a368433a4b8de440e7201f02 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 12:03:00 +0200 Subject: [PATCH 14/51] Add test for Py_mod_name slot --- Lib/test/test_capi/test_module.py | 17 +++++++++++---- Modules/_testcapi/module.c | 35 ++++++++++++++----------------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index b42d2f64576e82..29a2bdd6aa37c1 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -6,11 +6,20 @@ _testcapi = import_helper.import_module('_testcapi') +class FakeSpec: + name = 'testmod' + + class TestModFromSlotsAndSpec(unittest.TestCase): def test_empty(self): - spec_like = types.SimpleNamespace( - name='testmod', - ) - mod = _testcapi.module_from_slots_and_spec([], spec_like) + mod = _testcapi.module_from_slots_empty(FakeSpec()) + self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(mod.__name__, 'testmod') + + def test_name(self): + # Py_mod_name (and PyModuleDef.m_name) are currently ignored when + # spec is given. + # We still test that it's accepted. + mod = _testcapi.module_from_slots_name(FakeSpec()) self.assertIsInstance(mod, types.ModuleType) self.assertEqual(mod.__name__, 'testmod') diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 3aa9e2888a18c5..8e6035b6d45e72 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -4,31 +4,28 @@ // Test PyModule_* API static PyObject * -module_from_slots_and_spec(PyObject *self, PyObject *args) +module_from_slots_empty(PyObject *self, PyObject *spec) { - PyObject *spec; - PyObject *py_slots; - if(PyArg_UnpackTuple(args, "module_from_slots_and_spec", 2, 2, - &py_slots, &spec) < 1) - { - return NULL; - } - assert(PyList_Check(py_slots)); - Py_ssize_t n_slots = PyList_GET_SIZE(py_slots); - PyModuleDef_Slot *slots = PyMem_Calloc(n_slots + 1, - sizeof(PyModuleDef_Slot)); - if (!slots) { - return PyErr_NoMemory(); - } + PyModuleDef_Slot slots[] = { + {0}, + }; + return PyModule_FromSlotsAndSpec(slots, spec); +} - PyObject *result = PyModule_FromSlotsAndSpec(slots, spec); - PyMem_Free(slots); - return result; +static PyObject * +module_from_slots_name(PyObject *self, PyObject *spec) +{ + PyModuleDef_Slot slots[] = { + {Py_mod_name, "currently ignored..."}, + {0}, + }; + return PyModule_FromSlotsAndSpec(slots, spec); } static PyMethodDef test_methods[] = { - {"module_from_slots_and_spec", module_from_slots_and_spec, METH_VARARGS}, + {"module_from_slots_empty", module_from_slots_empty, METH_O}, + {"module_from_slots_name", module_from_slots_name, METH_O}, {NULL}, }; From 55543ff9e6d1a37d8e50c20d435969b44cc74438 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 12:40:33 +0200 Subject: [PATCH 15/51] Handle Py_mod_name --- Objects/moduleobject.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index e5b209ef24f029..0436578c7f5609 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -372,6 +372,23 @@ module_from_def_and_spec( gil_slot = cur_slot->value; has_gil_slot = 1; break; + case Py_mod_name: + if (original_def) { + PyErr_Format( + PyExc_SystemError, + "module %s: Py_mod_name used with PyModuleDef", + name); + goto error; + } + if (def_like->m_name) { + PyErr_Format( + PyExc_SystemError, + "module %s has more than one 'gil' slot", + name); + goto error; + } + def_like->m_name = cur_slot->value; + break; default: assert(cur_slot->slot < 0 || cur_slot->slot > _Py_mod_LAST_SLOT); PyErr_Format( From 88d9e7f6ba42f4365a86842584083fb843608e12 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 12:42:45 +0200 Subject: [PATCH 16/51] Test repeated slot --- Lib/test/test_capi/test_module.py | 4 ++++ Modules/_testcapi/module.c | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 29a2bdd6aa37c1..0ade485931e225 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -23,3 +23,7 @@ def test_name(self): mod = _testcapi.module_from_slots_name(FakeSpec()) self.assertIsInstance(mod, types.ModuleType) self.assertEqual(mod.__name__, 'testmod') + + def test_repeat_name(self): + with self.assertRaises(SystemError): + _testcapi.module_from_slots_repeat_name(FakeSpec()) diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 8e6035b6d45e72..4b138f6d9e93f6 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -22,10 +22,22 @@ module_from_slots_name(PyObject *self, PyObject *spec) return PyModule_FromSlotsAndSpec(slots, spec); } +static PyObject * +module_from_slots_repeat_name(PyObject *self, PyObject *spec) +{ + PyModuleDef_Slot slots[] = { + {Py_mod_name, "currently ignored..."}, + {Py_mod_name, "currently ignored..."}, + {0}, + }; + return PyModule_FromSlotsAndSpec(slots, spec); +} + static PyMethodDef test_methods[] = { {"module_from_slots_empty", module_from_slots_empty, METH_O}, {"module_from_slots_name", module_from_slots_name, METH_O}, + {"module_from_slots_repeat_name", module_from_slots_repeat_name, METH_O}, {NULL}, }; From 344634749ed629d3ee2e62490f857a960a07ef55 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 12:47:04 +0200 Subject: [PATCH 17/51] Test Py_mod_name with a def --- Lib/test/test_capi/test_module.py | 10 +++++++++- Modules/_testcapi/module.c | 16 ++++++++++++++++ Objects/moduleobject.c | 2 +- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 0ade485931e225..a736a4796344e3 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -24,6 +24,14 @@ def test_name(self): self.assertIsInstance(mod, types.ModuleType) self.assertEqual(mod.__name__, 'testmod') + def test_def_name(self): + with self.assertRaises(SystemError) as cm: + _testcapi.module_from_def_name(FakeSpec()) + self.assertIn("Py_mod_name", str(cm.exception),) + self.assertIn("PyModuleDef", str(cm.exception), ) + def test_repeat_name(self): - with self.assertRaises(SystemError): + with self.assertRaises(SystemError) as cm: _testcapi.module_from_slots_repeat_name(FakeSpec()) + self.assertIn("Py_mod_name", str(cm.exception),) + self.assertIn("repeated", str(cm.exception), ) diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 4b138f6d9e93f6..bcb18386b5b5ec 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -34,10 +34,26 @@ module_from_slots_repeat_name(PyObject *self, PyObject *spec) } + +static PyObject * +module_from_def_name(PyObject *self, PyObject *spec) +{ + PyModuleDef_Slot slots[] = { + {Py_mod_name, "currently ignored..."}, + {0}, + }; + PyModuleDef def = { + .m_name = "currently ignored", + .m_slots = slots, + }; + return PyModule_FromDefAndSpec(&def, spec); +} + static PyMethodDef test_methods[] = { {"module_from_slots_empty", module_from_slots_empty, METH_O}, {"module_from_slots_name", module_from_slots_name, METH_O}, {"module_from_slots_repeat_name", module_from_slots_repeat_name, METH_O}, + {"module_from_def_name", module_from_def_name, METH_O}, {NULL}, }; diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 0436578c7f5609..593e5c38dd3637 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -383,7 +383,7 @@ module_from_def_and_spec( if (def_like->m_name) { PyErr_Format( PyExc_SystemError, - "module %s has more than one 'gil' slot", + "module %s: Py_mod_name slot repeated", name); goto error; } From 225ddf4b08a86f4d66327890c692dd2629ff32df Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 12:51:42 +0200 Subject: [PATCH 18/51] Py_mod_doc --- Lib/test/test_capi/test_module.py | 7 ++++++ Modules/_testcapi/module.c | 11 +++++++++ Objects/moduleobject.c | 39 +++++++++++++++++-------------- 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index a736a4796344e3..a96a9e8d3fed9a 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -23,6 +23,13 @@ def test_name(self): mod = _testcapi.module_from_slots_name(FakeSpec()) self.assertIsInstance(mod, types.ModuleType) self.assertEqual(mod.__name__, 'testmod') + self.assertEqual(mod.__doc__, None) + + def test_doc(self): + mod = _testcapi.module_from_slots_doc(FakeSpec()) + self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(mod.__name__, 'testmod') + self.assertEqual(mod.__doc__, 'the docstring') def test_def_name(self): with self.assertRaises(SystemError) as cm: diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index bcb18386b5b5ec..bc1269db7181a6 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -22,6 +22,16 @@ module_from_slots_name(PyObject *self, PyObject *spec) return PyModule_FromSlotsAndSpec(slots, spec); } +static PyObject * +module_from_slots_doc(PyObject *self, PyObject *spec) +{ + PyModuleDef_Slot slots[] = { + {Py_mod_doc, "the docstring"}, + {0}, + }; + return PyModule_FromSlotsAndSpec(slots, spec); +} + static PyObject * module_from_slots_repeat_name(PyObject *self, PyObject *spec) { @@ -52,6 +62,7 @@ module_from_def_name(PyObject *self, PyObject *spec) static PyMethodDef test_methods[] = { {"module_from_slots_empty", module_from_slots_empty, METH_O}, {"module_from_slots_name", module_from_slots_name, METH_O}, + {"module_from_slots_doc", module_from_slots_doc, METH_O}, {"module_from_slots_repeat_name", module_from_slots_repeat_name, METH_O}, {"module_from_def_name", module_from_def_name, METH_O}, {NULL}, diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 593e5c38dd3637..d0184b6b41af7b 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -372,23 +372,28 @@ module_from_def_and_spec( gil_slot = cur_slot->value; has_gil_slot = 1; break; - case Py_mod_name: - if (original_def) { - PyErr_Format( - PyExc_SystemError, - "module %s: Py_mod_name used with PyModuleDef", - name); - goto error; - } - if (def_like->m_name) { - PyErr_Format( - PyExc_SystemError, - "module %s: Py_mod_name slot repeated", - name); - goto error; - } - def_like->m_name = cur_slot->value; - break; +#define COPY_SLOT_TO_DEFLIKE(SLOT, TYPE, DEST) \ + case SLOT: \ + if (original_def) { \ + PyErr_Format( \ + PyExc_SystemError, \ + "module %s: " #SLOT " used with PyModuleDef", \ + name); \ + goto error; \ + } \ + if (def_like->DEST) { \ + PyErr_Format( \ + PyExc_SystemError, \ + "module %s: " #SLOT " slot repeated", \ + name); \ + goto error; \ + } \ + def_like->DEST = (TYPE)(cur_slot->value); \ + break; \ + ///////////////////////////////////////////////////////////// + COPY_SLOT_TO_DEFLIKE(Py_mod_name, char*, m_name); + COPY_SLOT_TO_DEFLIKE(Py_mod_doc, char*, m_doc); +#undef COPY_SLOT_TO_DEFLIKE default: assert(cur_slot->slot < 0 || cur_slot->slot > _Py_mod_LAST_SLOT); PyErr_Format( From ba00d2549cc7c28da1af8e20b73ce4c9b4c7a902 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 12:58:17 +0200 Subject: [PATCH 19/51] Disallow NULL slot value --- Lib/test/test_capi/test_module.py | 6 ++++++ Modules/_testcapi/module.c | 10 ++++++++++ Objects/moduleobject.c | 7 +++++++ 3 files changed, 23 insertions(+) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index a96a9e8d3fed9a..ee7b8c84d23aef 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -42,3 +42,9 @@ def test_repeat_name(self): _testcapi.module_from_slots_repeat_name(FakeSpec()) self.assertIn("Py_mod_name", str(cm.exception),) self.assertIn("repeated", str(cm.exception), ) + + def test_null_name(self): + with self.assertRaises(SystemError) as cm: + _testcapi.module_from_slots_null_name(FakeSpec()) + self.assertIn("Py_mod_name", str(cm.exception),) + self.assertIn("NULL", str(cm.exception), ) diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index bc1269db7181a6..f0d3b439ddaab1 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -44,6 +44,15 @@ module_from_slots_repeat_name(PyObject *self, PyObject *spec) } +static PyObject * +module_from_slots_null_name(PyObject *self, PyObject *spec) +{ + PyModuleDef_Slot slots[] = { + {Py_mod_name, NULL}, + {0}, + }; + return PyModule_FromSlotsAndSpec(slots, spec); +} static PyObject * module_from_def_name(PyObject *self, PyObject *spec) @@ -64,6 +73,7 @@ static PyMethodDef test_methods[] = { {"module_from_slots_name", module_from_slots_name, METH_O}, {"module_from_slots_doc", module_from_slots_doc, METH_O}, {"module_from_slots_repeat_name", module_from_slots_repeat_name, METH_O}, + {"module_from_slots_null_name", module_from_slots_null_name, METH_O}, {"module_from_def_name", module_from_def_name, METH_O}, {NULL}, }; diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index d0184b6b41af7b..724a34af31c006 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -374,6 +374,13 @@ module_from_def_and_spec( break; #define COPY_SLOT_TO_DEFLIKE(SLOT, TYPE, DEST) \ case SLOT: \ + if (!(TYPE)(cur_slot->value)) { \ + PyErr_Format( \ + PyExc_SystemError, \ + "module %s: " #SLOT " must not be NULL", \ + name); \ + goto error; \ + } \ if (original_def) { \ PyErr_Format( \ PyExc_SystemError, \ From f4954a22530acfeb4ccc5c6b8eedcc958ffc28ca Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 13:06:20 +0200 Subject: [PATCH 20/51] Test repeats and NULL for all new slots --- Lib/test/test_capi/test_module.py | 26 +++++++++++------- Modules/_testcapi/module.c | 44 ++++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index ee7b8c84d23aef..0bac5b97ecb85a 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -37,14 +37,22 @@ def test_def_name(self): self.assertIn("Py_mod_name", str(cm.exception),) self.assertIn("PyModuleDef", str(cm.exception), ) - def test_repeat_name(self): - with self.assertRaises(SystemError) as cm: - _testcapi.module_from_slots_repeat_name(FakeSpec()) - self.assertIn("Py_mod_name", str(cm.exception),) - self.assertIn("repeated", str(cm.exception), ) + def test_repeated_new_slot(self): + for name in 'Py_mod_name', 'Py_mod_doc': + with self.subTest(name): + spec = FakeSpec() + spec._test_slot_id = getattr(_testcapi, name) + with self.assertRaises(SystemError) as cm: + _testcapi.module_from_slots_repeat_slot(spec) + self.assertIn(name, str(cm.exception),) + self.assertIn("repeated", str(cm.exception), ) def test_null_name(self): - with self.assertRaises(SystemError) as cm: - _testcapi.module_from_slots_null_name(FakeSpec()) - self.assertIn("Py_mod_name", str(cm.exception),) - self.assertIn("NULL", str(cm.exception), ) + for name in 'Py_mod_name', 'Py_mod_doc': + with self.subTest(name): + spec = FakeSpec() + spec._test_slot_id = getattr(_testcapi, name) + with self.assertRaises(SystemError) as cm: + _testcapi.module_from_slots_null_slot(spec) + self.assertIn(name, str(cm.exception),) + self.assertIn("NULL", str(cm.exception), ) diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index f0d3b439ddaab1..88915f3ba5ba1f 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -33,11 +33,19 @@ module_from_slots_doc(PyObject *self, PyObject *spec) } static PyObject * -module_from_slots_repeat_name(PyObject *self, PyObject *spec) +module_from_slots_repeat_slot(PyObject *self, PyObject *spec) { + PyObject *slot_id_obj = PyObject_GetAttrString(spec, "_test_slot_id"); + if (slot_id_obj == NULL) { + return NULL; + } + int slot_id = PyLong_AsLong(slot_id_obj); + if (PyErr_Occurred()) { + return NULL; + } PyModuleDef_Slot slots[] = { - {Py_mod_name, "currently ignored..."}, - {Py_mod_name, "currently ignored..."}, + {slot_id, "currently ignored..."}, + {slot_id, "currently ignored..."}, {0}, }; return PyModule_FromSlotsAndSpec(slots, spec); @@ -45,10 +53,18 @@ module_from_slots_repeat_name(PyObject *self, PyObject *spec) static PyObject * -module_from_slots_null_name(PyObject *self, PyObject *spec) +module_from_slots_null_slot(PyObject *self, PyObject *spec) { + PyObject *slot_id_obj = PyObject_GetAttrString(spec, "_test_slot_id"); + if (slot_id_obj == NULL) { + return NULL; + } + int slot_id = PyLong_AsLong(slot_id_obj); + if (PyErr_Occurred()) { + return NULL; + } PyModuleDef_Slot slots[] = { - {Py_mod_name, NULL}, + {slot_id, NULL}, {0}, }; return PyModule_FromSlotsAndSpec(slots, spec); @@ -72,8 +88,8 @@ static PyMethodDef test_methods[] = { {"module_from_slots_empty", module_from_slots_empty, METH_O}, {"module_from_slots_name", module_from_slots_name, METH_O}, {"module_from_slots_doc", module_from_slots_doc, METH_O}, - {"module_from_slots_repeat_name", module_from_slots_repeat_name, METH_O}, - {"module_from_slots_null_name", module_from_slots_null_name, METH_O}, + {"module_from_slots_repeat_slot", module_from_slots_repeat_slot, METH_O}, + {"module_from_slots_null_slot", module_from_slots_null_slot, METH_O}, {"module_from_def_name", module_from_def_name, METH_O}, {NULL}, }; @@ -81,5 +97,19 @@ static PyMethodDef test_methods[] = { int _PyTestCapi_Init_Module(PyObject *m) { +#define ADD_INT_MACRO(C) if (PyModule_AddIntConstant(m, #C, C) < 0) return -1; + ADD_INT_MACRO(Py_mod_create); + ADD_INT_MACRO(Py_mod_exec); + ADD_INT_MACRO(Py_mod_multiple_interpreters); + ADD_INT_MACRO(Py_mod_gil); + ADD_INT_MACRO(Py_mod_name); + ADD_INT_MACRO(Py_mod_doc); + ADD_INT_MACRO(Py_mod_size); + ADD_INT_MACRO(Py_mod_methods); + ADD_INT_MACRO(Py_mod_traverse); + ADD_INT_MACRO(Py_mod_clear); + ADD_INT_MACRO(Py_mod_free); + ADD_INT_MACRO(Py_mod_token); +#undef ADD_INT_MACRO return PyModule_AddFunctions(m, test_methods); } From dccffcb5e420dc1c135e58f6949504dc8b8ec62d Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 13:19:24 +0200 Subject: [PATCH 21/51] Add PyModule_GetSize --- Include/moduleobject.h | 1 + Objects/moduleobject.c | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/Include/moduleobject.h b/Include/moduleobject.h index 7f64ff7ec74a7f..972569056fd273 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -118,6 +118,7 @@ PyAPI_FUNC(int) PyUnstable_Module_SetGIL(PyObject *module, void *gil); #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= _Py_PACK_VERSION(3, 15) PyAPI_FUNC(PyObject *) PyModule_FromSlotsAndSpec(PyModuleDef_Slot *, PyObject *spec); +PyAPI_FUNC(int) PyModule_GetSize(PyObject *mod, Py_ssize_t *size_p); #endif diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 724a34af31c006..63c536bd9f6c5d 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -666,6 +666,19 @@ PyModule_GetDict(PyObject *m) return _PyModule_GetDict(m); // borrowed reference } +int +PyModule_GetSize(PyObject *m, Py_ssize_t *size_p) +{ + *size_p = -1; + if (!PyModule_Check(m)) { + PyErr_BadInternalCall(); + return -1; + } + PyModuleObject *mod = (PyModuleObject *)m; + *size_p = mod->md_size; + return 0; +} + PyObject* PyModule_GetNameObject(PyObject *mod) { From a5bbdf4ffa88e488655f9e05f6350cc15d09d768 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 13:17:58 +0200 Subject: [PATCH 22/51] Test size --- Lib/test/test_capi/test_module.py | 7 +++++++ Modules/_testcapi/module.c | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 0bac5b97ecb85a..26375bfc3cec66 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -31,6 +31,13 @@ def test_doc(self): self.assertEqual(mod.__name__, 'testmod') self.assertEqual(mod.__doc__, 'the docstring') + def test_size(self): + mod = _testcapi.module_from_slots_size(FakeSpec()) + self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(mod.__name__, 'testmod') + self.assertEqual(mod.__doc__, None) + self.assertEqual(mod.size, 123) + def test_def_name(self): with self.assertRaises(SystemError) as cm: _testcapi.module_from_def_name(FakeSpec()) diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 88915f3ba5ba1f..486dd35d82b51b 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -32,6 +32,27 @@ module_from_slots_doc(PyObject *self, PyObject *spec) return PyModule_FromSlotsAndSpec(slots, spec); } +static PyObject * +module_from_slots_size(PyObject *self, PyObject *spec) +{ + PyModuleDef_Slot slots[] = { + {Py_mod_size, (void*)123}, + {0}, + }; + PyObject *mod = PyModule_FromSlotsAndSpec(slots, spec); + if (!mod) { + return NULL; + } + Py_ssize_t size; + if (PyModule_GetSize(mod, &size) < 0) { + return NULL; + } + if (PyModule_AddIntConstant(mod, "size", size) < 0) { + return NULL; + } + return mod; +} + static PyObject * module_from_slots_repeat_slot(PyObject *self, PyObject *spec) { @@ -88,6 +109,7 @@ static PyMethodDef test_methods[] = { {"module_from_slots_empty", module_from_slots_empty, METH_O}, {"module_from_slots_name", module_from_slots_name, METH_O}, {"module_from_slots_doc", module_from_slots_doc, METH_O}, + {"module_from_slots_size", module_from_slots_size, METH_O}, {"module_from_slots_repeat_slot", module_from_slots_repeat_slot, METH_O}, {"module_from_slots_null_slot", module_from_slots_null_slot, METH_O}, {"module_from_def_name", module_from_def_name, METH_O}, From dc454a099b5a6460a432dc24d0b8743d15e020a7 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 13:23:45 +0200 Subject: [PATCH 23/51] Handle size --- Objects/moduleobject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 63c536bd9f6c5d..bcb99ad5da719f 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -400,6 +400,7 @@ module_from_def_and_spec( ///////////////////////////////////////////////////////////// COPY_SLOT_TO_DEFLIKE(Py_mod_name, char*, m_name); COPY_SLOT_TO_DEFLIKE(Py_mod_doc, char*, m_doc); + COPY_SLOT_TO_DEFLIKE(Py_mod_size, Py_ssize_t, m_size); #undef COPY_SLOT_TO_DEFLIKE default: assert(cur_slot->slot < 0 || cur_slot->slot > _Py_mod_LAST_SLOT); From d2e9b69aebd9eed9dde4b7db19e13a85d209bc75 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 13:32:19 +0200 Subject: [PATCH 24/51] Test new slots with PyModuleDef --- Lib/test/test_capi/test_module.py | 26 +++++++++++++------ Modules/_testcapi/module.c | 42 +++++++++++++++++++------------ 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 26375bfc3cec66..958a7d8faa846e 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -9,6 +9,10 @@ class FakeSpec: name = 'testmod' +DEF_SLOTS = ( + 'Py_mod_name', 'Py_mod_doc', 'Py_mod_size', +) + class TestModFromSlotsAndSpec(unittest.TestCase): def test_empty(self): @@ -38,13 +42,20 @@ def test_size(self): self.assertEqual(mod.__doc__, None) self.assertEqual(mod.size, 123) - def test_def_name(self): - with self.assertRaises(SystemError) as cm: - _testcapi.module_from_def_name(FakeSpec()) - self.assertIn("Py_mod_name", str(cm.exception),) - self.assertIn("PyModuleDef", str(cm.exception), ) + def test_def_slot(self): + """Slots that replace PyModuleDef fields can't be used with PyModuleDef + """ + for name in 'Py_mod_name', 'Py_mod_doc': + with self.subTest(name): + spec = FakeSpec() + spec._test_slot_id = getattr(_testcapi, name) + with self.assertRaises(SystemError) as cm: + _testcapi.module_from_def_slot(spec) + self.assertIn(name, str(cm.exception),) + self.assertIn("PyModuleDef", str(cm.exception), ) - def test_repeated_new_slot(self): + def test_repeated_def_slot(self): + """Slots that replace PyModuleDef fields can't be repeated""" for name in 'Py_mod_name', 'Py_mod_doc': with self.subTest(name): spec = FakeSpec() @@ -54,7 +65,8 @@ def test_repeated_new_slot(self): self.assertIn(name, str(cm.exception),) self.assertIn("repeated", str(cm.exception), ) - def test_null_name(self): + def test_null_def_slot(self): + """Slots that replace PyModuleDef fields can't be NULL""" for name in 'Py_mod_name', 'Py_mod_doc': with self.subTest(name): spec = FakeSpec() diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 486dd35d82b51b..64bc9a55af7bb3 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -53,35 +53,41 @@ module_from_slots_size(PyObject *self, PyObject *spec) return mod; } -static PyObject * -module_from_slots_repeat_slot(PyObject *self, PyObject *spec) + +static int +slot_from_object(PyObject *obj) { - PyObject *slot_id_obj = PyObject_GetAttrString(spec, "_test_slot_id"); + PyObject *slot_id_obj = PyObject_GetAttrString(obj, "_test_slot_id"); if (slot_id_obj == NULL) { - return NULL; + return -1; } int slot_id = PyLong_AsLong(slot_id_obj); if (PyErr_Occurred()) { + return -1; + } + return slot_id; +} + +static PyObject * +module_from_slots_repeat_slot(PyObject *self, PyObject *spec) +{ + int slot_id = slot_from_object(spec); + if (slot_id < 0) { return NULL; } PyModuleDef_Slot slots[] = { - {slot_id, "currently ignored..."}, - {slot_id, "currently ignored..."}, + {slot_id, "anything"}, + {slot_id, "anything else"}, {0}, }; return PyModule_FromSlotsAndSpec(slots, spec); } - static PyObject * module_from_slots_null_slot(PyObject *self, PyObject *spec) { - PyObject *slot_id_obj = PyObject_GetAttrString(spec, "_test_slot_id"); - if (slot_id_obj == NULL) { - return NULL; - } - int slot_id = PyLong_AsLong(slot_id_obj); - if (PyErr_Occurred()) { + int slot_id = slot_from_object(spec); + if (slot_id < 0) { return NULL; } PyModuleDef_Slot slots[] = { @@ -92,10 +98,14 @@ module_from_slots_null_slot(PyObject *self, PyObject *spec) } static PyObject * -module_from_def_name(PyObject *self, PyObject *spec) +module_from_def_slot(PyObject *self, PyObject *spec) { + int slot_id = slot_from_object(spec); + if (slot_id < 0) { + return NULL; + } PyModuleDef_Slot slots[] = { - {Py_mod_name, "currently ignored..."}, + {slot_id, "anything"}, {0}, }; PyModuleDef def = { @@ -112,7 +122,7 @@ static PyMethodDef test_methods[] = { {"module_from_slots_size", module_from_slots_size, METH_O}, {"module_from_slots_repeat_slot", module_from_slots_repeat_slot, METH_O}, {"module_from_slots_null_slot", module_from_slots_null_slot, METH_O}, - {"module_from_def_name", module_from_def_name, METH_O}, + {"module_from_def_slot", module_from_def_slot, METH_O}, {NULL}, }; From 78182d96dd74122ef8cb72de19660401687eb670 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 13:37:18 +0200 Subject: [PATCH 25/51] Py_mod_methods --- Lib/test/test_capi/test_module.py | 15 +++++++++++---- Modules/_testcapi/module.c | 22 ++++++++++++++++++++++ Objects/moduleobject.c | 1 + 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 958a7d8faa846e..cfbfc57b00a59d 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -10,7 +10,7 @@ class FakeSpec: name = 'testmod' DEF_SLOTS = ( - 'Py_mod_name', 'Py_mod_doc', 'Py_mod_size', + 'Py_mod_name', 'Py_mod_doc', 'Py_mod_size', 'Py_mod_methods', ) @@ -42,10 +42,17 @@ def test_size(self): self.assertEqual(mod.__doc__, None) self.assertEqual(mod.size, 123) + def test_methods(self): + mod = _testcapi.module_from_slots_methods(FakeSpec()) + self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(mod.__name__, 'testmod') + self.assertEqual(mod.__doc__, None) + self.assertEqual(mod.a_method(456), (mod, 456)) + def test_def_slot(self): """Slots that replace PyModuleDef fields can't be used with PyModuleDef """ - for name in 'Py_mod_name', 'Py_mod_doc': + for name in DEF_SLOTS: with self.subTest(name): spec = FakeSpec() spec._test_slot_id = getattr(_testcapi, name) @@ -56,7 +63,7 @@ def test_def_slot(self): def test_repeated_def_slot(self): """Slots that replace PyModuleDef fields can't be repeated""" - for name in 'Py_mod_name', 'Py_mod_doc': + for name in DEF_SLOTS: with self.subTest(name): spec = FakeSpec() spec._test_slot_id = getattr(_testcapi, name) @@ -67,7 +74,7 @@ def test_repeated_def_slot(self): def test_null_def_slot(self): """Slots that replace PyModuleDef fields can't be NULL""" - for name in 'Py_mod_name', 'Py_mod_doc': + for name in DEF_SLOTS: with self.subTest(name): spec = FakeSpec() spec._test_slot_id = getattr(_testcapi, name) diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 64bc9a55af7bb3..86e327a5e037f1 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -53,6 +53,27 @@ module_from_slots_size(PyObject *self, PyObject *spec) return mod; } +static PyObject * +a_method(PyObject *self, PyObject *arg) +{ + return PyTuple_Pack(2, self, arg); +} + +static PyMethodDef a_methoddef_array[] = { + {"a_method", a_method, METH_O}, + {0}, +}; + +static PyObject * +module_from_slots_methods(PyObject *self, PyObject *spec) +{ + PyModuleDef_Slot slots[] = { + {Py_mod_methods, a_methoddef_array}, + {0}, + }; + return PyModule_FromSlotsAndSpec(slots, spec); +} + static int slot_from_object(PyObject *obj) @@ -120,6 +141,7 @@ static PyMethodDef test_methods[] = { {"module_from_slots_name", module_from_slots_name, METH_O}, {"module_from_slots_doc", module_from_slots_doc, METH_O}, {"module_from_slots_size", module_from_slots_size, METH_O}, + {"module_from_slots_methods", module_from_slots_methods, METH_O}, {"module_from_slots_repeat_slot", module_from_slots_repeat_slot, METH_O}, {"module_from_slots_null_slot", module_from_slots_null_slot, METH_O}, {"module_from_def_slot", module_from_def_slot, METH_O}, diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index bcb99ad5da719f..c218d7ec06ce99 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -401,6 +401,7 @@ module_from_def_and_spec( COPY_SLOT_TO_DEFLIKE(Py_mod_name, char*, m_name); COPY_SLOT_TO_DEFLIKE(Py_mod_doc, char*, m_doc); COPY_SLOT_TO_DEFLIKE(Py_mod_size, Py_ssize_t, m_size); + COPY_SLOT_TO_DEFLIKE(Py_mod_methods, PyMethodDef*, m_methods); #undef COPY_SLOT_TO_DEFLIKE default: assert(cur_slot->slot < 0 || cur_slot->slot > _Py_mod_LAST_SLOT); From 5aa11e15519edea00ed699ca3bba72804c6df13c Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 13:48:56 +0200 Subject: [PATCH 26/51] GC hooks --- Include/moduleobject.h | 2 ++ Lib/test/test_capi/test_module.py | 7 +++++++ Modules/_testcapi/module.c | 32 +++++++++++++++++++++++++++++++ Objects/moduleobject.c | 18 +++++++++++++++++ 4 files changed, 59 insertions(+) diff --git a/Include/moduleobject.h b/Include/moduleobject.h index 972569056fd273..880907ac0f6d98 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -94,6 +94,8 @@ struct PyModuleDef_Slot { #ifndef Py_LIMITED_API #define _Py_mod_LAST_SLOT 12 +PyAPI_FUNC(int) _PyModule_GetGCHooks( + PyObject *, traverseproc*, inquiry*, freefunc*); // For testing #endif #endif /* New in 3.5 */ diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index cfbfc57b00a59d..39f98f319704a7 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -11,6 +11,7 @@ class FakeSpec: DEF_SLOTS = ( 'Py_mod_name', 'Py_mod_doc', 'Py_mod_size', 'Py_mod_methods', + 'Py_mod_traverse', 'Py_mod_clear', 'Py_mod_free', ) @@ -49,6 +50,12 @@ def test_methods(self): self.assertEqual(mod.__doc__, None) self.assertEqual(mod.a_method(456), (mod, 456)) + def test_gc(self): + mod = _testcapi.module_from_slots_gc(FakeSpec()) + self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(mod.__name__, 'testmod') + self.assertEqual(mod.__doc__, None) + def test_def_slot(self): """Slots that replace PyModuleDef fields can't be used with PyModuleDef """ diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 86e327a5e037f1..a28c5f715fae2a 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -74,6 +74,37 @@ module_from_slots_methods(PyObject *self, PyObject *spec) return PyModule_FromSlotsAndSpec(slots, spec); } +static int trivial_traverse(PyObject *self, visitproc visit, void *arg) { + return 0; +} +static int trivial_clear(PyObject *self) { return 0; } +static void trivial_free(void *self) { } + +static PyObject * +module_from_slots_gc(PyObject *self, PyObject *spec) +{ + PyModuleDef_Slot slots[] = { + {Py_mod_traverse, trivial_traverse}, + {Py_mod_clear, trivial_clear}, + {Py_mod_free, trivial_free}, + {0}, + }; + PyObject *mod = PyModule_FromSlotsAndSpec(slots, spec); + if (!mod) { + return NULL; + } + traverseproc traverse; + inquiry clear; + freefunc free; + if (_PyModule_GetGCHooks(mod, &traverse, &clear, &free) < 0) { + return NULL; + } + assert(traverse == &trivial_traverse); + assert(clear == &trivial_clear); + assert(free == &trivial_free); + return mod; +} + static int slot_from_object(PyObject *obj) @@ -142,6 +173,7 @@ static PyMethodDef test_methods[] = { {"module_from_slots_doc", module_from_slots_doc, METH_O}, {"module_from_slots_size", module_from_slots_size, METH_O}, {"module_from_slots_methods", module_from_slots_methods, METH_O}, + {"module_from_slots_gc", module_from_slots_gc, METH_O}, {"module_from_slots_repeat_slot", module_from_slots_repeat_slot, METH_O}, {"module_from_slots_null_slot", module_from_slots_null_slot, METH_O}, {"module_from_def_slot", module_from_def_slot, METH_O}, diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index c218d7ec06ce99..77fec81b4d8457 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -402,6 +402,9 @@ module_from_def_and_spec( COPY_SLOT_TO_DEFLIKE(Py_mod_doc, char*, m_doc); COPY_SLOT_TO_DEFLIKE(Py_mod_size, Py_ssize_t, m_size); COPY_SLOT_TO_DEFLIKE(Py_mod_methods, PyMethodDef*, m_methods); + COPY_SLOT_TO_DEFLIKE(Py_mod_traverse, traverseproc, m_traverse); + COPY_SLOT_TO_DEFLIKE(Py_mod_clear, inquiry, m_clear); + COPY_SLOT_TO_DEFLIKE(Py_mod_free, freefunc, m_free); #undef COPY_SLOT_TO_DEFLIKE default: assert(cur_slot->slot < 0 || cur_slot->slot > _Py_mod_LAST_SLOT); @@ -681,6 +684,21 @@ PyModule_GetSize(PyObject *m, Py_ssize_t *size_p) return 0; } +int +_PyModule_GetGCHooks(PyObject *m, traverseproc *traverse, + inquiry *clear, freefunc *free) +{ + if (!PyModule_Check(m)) { + PyErr_BadInternalCall(); + return -1; + } + PyModuleObject *mod = (PyModuleObject *)m; + *traverse = mod->md_traverse; + *clear = mod->md_clear; + *free = mod->md_free; + return 0; +} + PyObject* PyModule_GetNameObject(PyObject *mod) { From 0b8d9e75943cb1792a700a6dc15884b7ad2abece Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 14:26:09 +0200 Subject: [PATCH 27/51] Py_mod_token --- Include/moduleobject.h | 1 + Lib/test/test_capi/test_module.py | 8 ++++- Modules/_testcapi/module.c | 26 ++++++++++++++++ Objects/moduleobject.c | 50 +++++++++++++++++++++++++------ 4 files changed, 75 insertions(+), 10 deletions(-) diff --git a/Include/moduleobject.h b/Include/moduleobject.h index 880907ac0f6d98..e6210e1b5eed9e 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -121,6 +121,7 @@ PyAPI_FUNC(int) PyUnstable_Module_SetGIL(PyObject *module, void *gil); PyAPI_FUNC(PyObject *) PyModule_FromSlotsAndSpec(PyModuleDef_Slot *, PyObject *spec); PyAPI_FUNC(int) PyModule_GetSize(PyObject *mod, Py_ssize_t *size_p); +PyAPI_FUNC(int) PyModule_GetToken(PyObject *, void **token_p); #endif diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 39f98f319704a7..94986c10bbc997 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -11,7 +11,7 @@ class FakeSpec: DEF_SLOTS = ( 'Py_mod_name', 'Py_mod_doc', 'Py_mod_size', 'Py_mod_methods', - 'Py_mod_traverse', 'Py_mod_clear', 'Py_mod_free', + 'Py_mod_traverse', 'Py_mod_clear', 'Py_mod_free', 'Py_mod_token', ) @@ -56,6 +56,12 @@ def test_gc(self): self.assertEqual(mod.__name__, 'testmod') self.assertEqual(mod.__doc__, None) + def test_token(self): + mod = _testcapi.module_from_slots_token(FakeSpec()) + self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(mod.__name__, 'testmod') + self.assertEqual(mod.__doc__, None) + def test_def_slot(self): """Slots that replace PyModuleDef fields can't be used with PyModuleDef """ diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index a28c5f715fae2a..2d3a53fe359572 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -45,9 +45,11 @@ module_from_slots_size(PyObject *self, PyObject *spec) } Py_ssize_t size; if (PyModule_GetSize(mod, &size) < 0) { + Py_DECREF(mod); return NULL; } if (PyModule_AddIntConstant(mod, "size", size) < 0) { + Py_DECREF(mod); return NULL; } return mod; @@ -97,6 +99,7 @@ module_from_slots_gc(PyObject *self, PyObject *spec) inquiry clear; freefunc free; if (_PyModule_GetGCHooks(mod, &traverse, &clear, &free) < 0) { + Py_DECREF(mod); return NULL; } assert(traverse == &trivial_traverse); @@ -105,6 +108,28 @@ module_from_slots_gc(PyObject *self, PyObject *spec) return mod; } +static char test_token; + +static PyObject * +module_from_slots_token(PyObject *self, PyObject *spec) +{ + PyModuleDef_Slot slots[] = { + {Py_mod_token, &test_token}, + {0}, + }; + PyObject *mod = PyModule_FromSlotsAndSpec(slots, spec); + if (!mod) { + return NULL; + } + void *got_token; + if (PyModule_GetToken(mod, &got_token) < 0) { + Py_DECREF(mod); + return NULL; + } + assert(got_token == &test_token); + return mod; +} + static int slot_from_object(PyObject *obj) @@ -174,6 +199,7 @@ static PyMethodDef test_methods[] = { {"module_from_slots_size", module_from_slots_size, METH_O}, {"module_from_slots_methods", module_from_slots_methods, METH_O}, {"module_from_slots_gc", module_from_slots_gc, METH_O}, + {"module_from_slots_token", module_from_slots_token, METH_O}, {"module_from_slots_repeat_slot", module_from_slots_repeat_slot, METH_O}, {"module_from_slots_null_slot", module_from_slots_null_slot, METH_O}, {"module_from_def_slot", module_from_def_slot, METH_O}, diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 77fec81b4d8457..05dc8ac1a9850c 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -312,6 +312,7 @@ module_from_def_and_spec( int has_execution_slots = 0; const char *name; int ret; + void *token = NULL; PyInterpreterState *interp = _PyInterpreterState_GET(); nameobj = PyObject_GetAttrString(spec, "name"); @@ -388,23 +389,26 @@ module_from_def_and_spec( name); \ goto error; \ } \ - if (def_like->DEST) { \ + if (DEST) { \ PyErr_Format( \ PyExc_SystemError, \ "module %s: " #SLOT " slot repeated", \ name); \ goto error; \ } \ - def_like->DEST = (TYPE)(cur_slot->value); \ + DEST = (TYPE)(cur_slot->value); \ break; \ ///////////////////////////////////////////////////////////// - COPY_SLOT_TO_DEFLIKE(Py_mod_name, char*, m_name); - COPY_SLOT_TO_DEFLIKE(Py_mod_doc, char*, m_doc); - COPY_SLOT_TO_DEFLIKE(Py_mod_size, Py_ssize_t, m_size); - COPY_SLOT_TO_DEFLIKE(Py_mod_methods, PyMethodDef*, m_methods); - COPY_SLOT_TO_DEFLIKE(Py_mod_traverse, traverseproc, m_traverse); - COPY_SLOT_TO_DEFLIKE(Py_mod_clear, inquiry, m_clear); - COPY_SLOT_TO_DEFLIKE(Py_mod_free, freefunc, m_free); + COPY_SLOT_TO_DEFLIKE(Py_mod_name, char*, def_like->m_name); + COPY_SLOT_TO_DEFLIKE(Py_mod_doc, char*, def_like->m_doc); + COPY_SLOT_TO_DEFLIKE(Py_mod_size, Py_ssize_t, def_like->m_size); + COPY_SLOT_TO_DEFLIKE(Py_mod_methods, PyMethodDef*, + def_like->m_methods); + COPY_SLOT_TO_DEFLIKE(Py_mod_traverse, traverseproc, + def_like->m_traverse); + COPY_SLOT_TO_DEFLIKE(Py_mod_clear, inquiry, def_like->m_clear); + COPY_SLOT_TO_DEFLIKE(Py_mod_free, freefunc, def_like->m_free); + COPY_SLOT_TO_DEFLIKE(Py_mod_token, void*, token); #undef COPY_SLOT_TO_DEFLIKE default: assert(cur_slot->slot < 0 || cur_slot->slot > _Py_mod_LAST_SLOT); @@ -474,6 +478,13 @@ module_from_def_and_spec( #else (void)gil_slot; #endif + if (original_def) { + mod->md_token = original_def; + assert (!token); + } + else { + mod->md_token = token; + } } else { if (def_like->m_size > 0 || def_like->m_traverse || def_like->m_clear || def_like->m_free) @@ -492,6 +503,14 @@ module_from_def_and_spec( name); goto error; } + if (token) { + PyErr_Format( + PyExc_SystemError, + "module %s specifies a token, but did not create " + "a ModuleType instance", + name); + goto error; + } } if (def_like->m_methods != NULL) { @@ -684,6 +703,19 @@ PyModule_GetSize(PyObject *m, Py_ssize_t *size_p) return 0; } +int +PyModule_GetToken(PyObject *m, void **token_p) +{ + *token_p = NULL; + if (!PyModule_Check(m)) { + PyErr_BadInternalCall(); + return -1; + } + PyModuleObject *mod = (PyModuleObject *)m; + *token_p = mod->md_token; + return 0; +} + int _PyModule_GetGCHooks(PyObject *m, traverseproc *traverse, inquiry *clear, freefunc *free) From dc857d70d71580fcef77df213c72813a61d783bb Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 14:39:55 +0200 Subject: [PATCH 28/51] Py_mod_exec --- Include/moduleobject.h | 1 + Lib/test/test_capi/test_module.py | 11 ++- Modules/_testcapi/module.c | 31 +++++++++ Objects/moduleobject.c | 107 ++++++++++++++++++------------ 4 files changed, 106 insertions(+), 44 deletions(-) diff --git a/Include/moduleobject.h b/Include/moduleobject.h index e6210e1b5eed9e..b0bcd0d7e7eca3 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -120,6 +120,7 @@ PyAPI_FUNC(int) PyUnstable_Module_SetGIL(PyObject *module, void *gil); #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= _Py_PACK_VERSION(3, 15) PyAPI_FUNC(PyObject *) PyModule_FromSlotsAndSpec(PyModuleDef_Slot *, PyObject *spec); +PyAPI_FUNC(int) PyModule_Exec(PyObject *mod); PyAPI_FUNC(int) PyModule_GetSize(PyObject *mod, Py_ssize_t *size_p); PyAPI_FUNC(int) PyModule_GetToken(PyObject *, void **token_p); #endif diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 94986c10bbc997..7ad1a92ff73513 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -62,6 +62,13 @@ def test_token(self): self.assertEqual(mod.__name__, 'testmod') self.assertEqual(mod.__doc__, None) + def test_exec(self): + mod = _testcapi.module_from_slots_exec(FakeSpec()) + self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(mod.__name__, 'testmod') + self.assertEqual(mod.__doc__, None) + self.assertEqual(mod.a_number, 456) + def test_def_slot(self): """Slots that replace PyModuleDef fields can't be used with PyModuleDef """ @@ -76,7 +83,7 @@ def test_def_slot(self): def test_repeated_def_slot(self): """Slots that replace PyModuleDef fields can't be repeated""" - for name in DEF_SLOTS: + for name in (*DEF_SLOTS, 'Py_mod_exec'): with self.subTest(name): spec = FakeSpec() spec._test_slot_id = getattr(_testcapi, name) @@ -87,7 +94,7 @@ def test_repeated_def_slot(self): def test_null_def_slot(self): """Slots that replace PyModuleDef fields can't be NULL""" - for name in DEF_SLOTS: + for name in (*DEF_SLOTS, 'Py_mod_exec'): with self.subTest(name): spec = FakeSpec() spec._test_slot_id = getattr(_testcapi, name) diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 2d3a53fe359572..1ac4b2a67e597a 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -130,6 +130,36 @@ module_from_slots_token(PyObject *self, PyObject *spec) return mod; } +static int +simple_exec(PyObject *module) +{ + return PyModule_AddIntConstant(module, "a_number", 456); +} + +static PyObject * +module_from_slots_exec(PyObject *self, PyObject *spec) +{ + PyModuleDef_Slot slots[] = { + {Py_mod_exec, simple_exec}, + {0}, + }; + PyObject *mod = PyModule_FromSlotsAndSpec(slots, spec); + if (!mod) { + return NULL; + } + int res = PyObject_HasAttrStringWithError(mod, "a_number"); + if (res < 0) { + Py_DECREF(mod); + return NULL; + } + assert(res == 0); + if (PyModule_Exec(mod) < 0) { + Py_DECREF(mod); + return NULL; + } + return mod; +} + static int slot_from_object(PyObject *obj) @@ -200,6 +230,7 @@ static PyMethodDef test_methods[] = { {"module_from_slots_methods", module_from_slots_methods, METH_O}, {"module_from_slots_gc", module_from_slots_gc, METH_O}, {"module_from_slots_token", module_from_slots_token, METH_O}, + {"module_from_slots_exec", module_from_slots_exec, METH_O}, {"module_from_slots_repeat_slot", module_from_slots_repeat_slot, METH_O}, {"module_from_slots_null_slot", module_from_slots_null_slot, METH_O}, {"module_from_def_slot", module_from_def_slot, METH_O}, diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 05dc8ac1a9850c..634f49b93a5956 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -584,69 +584,92 @@ PyUnstable_Module_SetGIL(PyObject *module, void *gil) #endif int -PyModule_ExecDef(PyObject *module, PyModuleDef *def) +run_exec_func(PyObject *module, int (*exec)(PyObject *)) { - PyModuleDef_Slot *cur_slot; - const char *name; - int ret; + int ret = exec(module); + if (ret != 0) { + if (!PyErr_Occurred()) { + PyErr_Format( + PyExc_SystemError, + "execution of %S failed without setting an exception", + module); + } + return -1; + } + if (PyErr_Occurred()) { + _PyErr_FormatFromCause( + PyExc_SystemError, + "execution of module %S raised unreported exception", + module); + return -1; + } + return 0; +} - name = PyModule_GetName(module); - if (name == NULL) { +int +alloc_state(PyObject *module) +{ + if(!PyModule_Check(module)) { + PyErr_Format(PyExc_TypeError, "expected module, got %T", module); return -1; } + PyModuleObject *md = (PyModuleObject*)module; - if (def->m_size >= 0) { - PyModuleObject *md = (PyModuleObject*)module; + if (md->md_size >= 0) { if (md->md_state == NULL) { /* Always set a state pointer; this serves as a marker to skip * multiple initialization (importlib.reload() is no-op) */ - md->md_state = PyMem_Malloc(def->m_size); + md->md_state = PyMem_Malloc(md->md_size); if (!md->md_state) { PyErr_NoMemory(); return -1; } - memset(md->md_state, 0, def->m_size); + memset(md->md_state, 0, md->md_size); } } + return 0; +} + +int +PyModule_Exec(PyObject *module) +{ + if (alloc_state(module) < 0) { + return -1; + } + PyModuleObject *md = (PyModuleObject*)module; + if (md->md_exec) { + assert(!md->md_def_or_null); + return run_exec_func(module, md->md_exec); + } + + if (md->md_def_or_null) { + return PyModule_ExecDef(module, md->md_def_or_null); + } + return 0; +} + +int +PyModule_ExecDef(PyObject *module, PyModuleDef *def) +{ + PyModuleDef_Slot *cur_slot; + + if (alloc_state(module) < 0) { + return -1; + } + + assert(PyModule_Check(module)); if (def->m_slots == NULL) { return 0; } for (cur_slot = def->m_slots; cur_slot && cur_slot->slot; cur_slot++) { - switch (cur_slot->slot) { - case Py_mod_create: - /* handled in PyModule_FromDefAndSpec2 */ - break; - case Py_mod_exec: - ret = ((int (*)(PyObject *))cur_slot->value)(module); - if (ret != 0) { - if (!PyErr_Occurred()) { - PyErr_Format( - PyExc_SystemError, - "execution of module %s failed without setting an exception", - name); - } - return -1; - } - if (PyErr_Occurred()) { - _PyErr_FormatFromCause( - PyExc_SystemError, - "execution of module %s raised unreported exception", - name); - return -1; - } - break; - case Py_mod_multiple_interpreters: - case Py_mod_gil: - /* handled in PyModule_FromDefAndSpec2 */ - break; - default: - PyErr_Format( - PyExc_SystemError, - "module %s initialized with unknown slot %i", - name, cur_slot->slot); + if (cur_slot->slot == Py_mod_exec) { + int (*func)(PyObject *) = cur_slot->value; + if (run_exec_func(module, func) < 0) { return -1; + } + break; } } return 0; From 17bdf1bbedd5f0d0f905549bc5e763e373543052 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 15:04:43 +0200 Subject: [PATCH 29/51] Py_mod_exec impl --- Include/internal/pycore_moduleobject.h | 4 +- Objects/moduleobject.c | 91 ++++++++++++++++---------- 2 files changed, 59 insertions(+), 36 deletions(-) diff --git a/Include/internal/pycore_moduleobject.h b/Include/internal/pycore_moduleobject.h index 430eeced831244..65c27f4bab614c 100644 --- a/Include/internal/pycore_moduleobject.h +++ b/Include/internal/pycore_moduleobject.h @@ -16,6 +16,8 @@ extern int _PyModule_IsPossiblyShadowing(PyObject *); extern int _PyModule_IsExtension(PyObject *obj); +typedef int (*_Py_modexecfunc)(PyObject *); + typedef struct { PyObject_HEAD PyObject *md_dict; @@ -34,7 +36,7 @@ typedef struct { inquiry md_clear; freefunc md_free; void *md_token; - int (*md_exec)(PyObject *); + _Py_modexecfunc md_exec; /* only set if md_def_or_null is NULL */ } PyModuleObject; static inline PyModuleDef* _PyModule_GetDefOrNull(PyObject *mod) { diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 634f49b93a5956..2d083266848289 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -313,6 +313,7 @@ module_from_def_and_spec( const char *name; int ret; void *token = NULL; + _Py_modexecfunc m_exec = NULL; PyInterpreterState *interp = _PyInterpreterState_GET(); nameobj = PyObject_GetAttrString(spec, "name"); @@ -337,6 +338,32 @@ module_from_def_and_spec( } for (cur_slot = def_like->m_slots; cur_slot && cur_slot->slot; cur_slot++) { + // Macro to copy a non-NULL, non-repeatable slot that's unusable with + // PyModuleDef. The destination must be initially NULL. +#define COPY_COMMON_SLOT(SLOT, TYPE, DEST) \ + if (!(TYPE)(cur_slot->value)) { \ + PyErr_Format( \ + PyExc_SystemError, \ + "module %s: " #SLOT " must not be NULL", \ + name); \ + goto error; \ + } \ + if (original_def) { \ + PyErr_Format( \ + PyExc_SystemError, \ + "module %s: " #SLOT " used with PyModuleDef", \ + name); \ + goto error; \ + } \ + if (DEST) { \ + PyErr_Format( \ + PyExc_SystemError, \ + "module %s: " #SLOT " slot repeated", \ + name); \ + goto error; \ + } \ + DEST = (TYPE)(cur_slot->value); \ + ///////////////////////////////////////////////////////////// switch (cur_slot->slot) { case Py_mod_create: if (create) { @@ -350,6 +377,9 @@ module_from_def_and_spec( break; case Py_mod_exec: has_execution_slots = 1; + if (!original_def) { + COPY_COMMON_SLOT(Py_mod_exec, _Py_modexecfunc, m_exec); + } break; case Py_mod_multiple_interpreters: if (has_multiple_interpreters_slot) { @@ -373,43 +403,32 @@ module_from_def_and_spec( gil_slot = cur_slot->value; has_gil_slot = 1; break; -#define COPY_SLOT_TO_DEFLIKE(SLOT, TYPE, DEST) \ - case SLOT: \ - if (!(TYPE)(cur_slot->value)) { \ - PyErr_Format( \ - PyExc_SystemError, \ - "module %s: " #SLOT " must not be NULL", \ - name); \ - goto error; \ - } \ - if (original_def) { \ - PyErr_Format( \ - PyExc_SystemError, \ - "module %s: " #SLOT " used with PyModuleDef", \ - name); \ - goto error; \ - } \ - if (DEST) { \ - PyErr_Format( \ - PyExc_SystemError, \ - "module %s: " #SLOT " slot repeated", \ - name); \ - goto error; \ - } \ - DEST = (TYPE)(cur_slot->value); \ - break; \ - ///////////////////////////////////////////////////////////// - COPY_SLOT_TO_DEFLIKE(Py_mod_name, char*, def_like->m_name); - COPY_SLOT_TO_DEFLIKE(Py_mod_doc, char*, def_like->m_doc); - COPY_SLOT_TO_DEFLIKE(Py_mod_size, Py_ssize_t, def_like->m_size); - COPY_SLOT_TO_DEFLIKE(Py_mod_methods, PyMethodDef*, + case Py_mod_name: + COPY_COMMON_SLOT(Py_mod_name, char*, def_like->m_name); + break; + case Py_mod_doc: + COPY_COMMON_SLOT(Py_mod_doc, char*, def_like->m_doc); + break; + case Py_mod_size: + COPY_COMMON_SLOT(Py_mod_size, Py_ssize_t, def_like->m_size); + break; + case Py_mod_methods: + COPY_COMMON_SLOT(Py_mod_methods, PyMethodDef*, def_like->m_methods); - COPY_SLOT_TO_DEFLIKE(Py_mod_traverse, traverseproc, + break; + case Py_mod_traverse: + COPY_COMMON_SLOT(Py_mod_traverse, traverseproc, def_like->m_traverse); - COPY_SLOT_TO_DEFLIKE(Py_mod_clear, inquiry, def_like->m_clear); - COPY_SLOT_TO_DEFLIKE(Py_mod_free, freefunc, def_like->m_free); - COPY_SLOT_TO_DEFLIKE(Py_mod_token, void*, token); -#undef COPY_SLOT_TO_DEFLIKE + break; + case Py_mod_clear: + COPY_COMMON_SLOT(Py_mod_clear, inquiry, def_like->m_clear); + break; + case Py_mod_free: + COPY_COMMON_SLOT(Py_mod_free, freefunc, def_like->m_free); + break; + case Py_mod_token: + COPY_COMMON_SLOT(Py_mod_token, void*, token); + break; default: assert(cur_slot->slot < 0 || cur_slot->slot > _Py_mod_LAST_SLOT); PyErr_Format( @@ -418,6 +437,7 @@ module_from_def_and_spec( name, cur_slot->slot); goto error; } +#undef COPY_COMMON_SLOT } /* By default, multi-phase init modules are expected @@ -485,6 +505,7 @@ module_from_def_and_spec( else { mod->md_token = token; } + mod->md_exec = m_exec; } else { if (def_like->m_size > 0 || def_like->m_traverse || def_like->m_clear || def_like->m_free) From 0cefc21e5881808d57d893d5acc315c185bae330 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 15:21:24 +0200 Subject: [PATCH 30/51] Py_mod_exec --- Lib/test/test_capi/test_module.py | 19 +++++++++++++------ Modules/_testcapi/module.c | 18 ++++++++++++++++++ Objects/moduleobject.c | 4 ++-- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 7ad1a92ff73513..677a59e56bc9aa 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -69,6 +69,13 @@ def test_exec(self): self.assertEqual(mod.__doc__, None) self.assertEqual(mod.a_number, 456) + def test_create(self): + spec = FakeSpec() + spec._gimme_this = "not a module object" + mod = _testcapi.module_from_slots_create(spec) + self.assertIsInstance(mod, str) + self.assertEqual(mod, "not a module object") + def test_def_slot(self): """Slots that replace PyModuleDef fields can't be used with PyModuleDef """ @@ -78,8 +85,8 @@ def test_def_slot(self): spec._test_slot_id = getattr(_testcapi, name) with self.assertRaises(SystemError) as cm: _testcapi.module_from_def_slot(spec) - self.assertIn(name, str(cm.exception),) - self.assertIn("PyModuleDef", str(cm.exception), ) + self.assertIn(name, str(cm.exception)) + self.assertIn("PyModuleDef", str(cm.exception)) def test_repeated_def_slot(self): """Slots that replace PyModuleDef fields can't be repeated""" @@ -89,8 +96,8 @@ def test_repeated_def_slot(self): spec._test_slot_id = getattr(_testcapi, name) with self.assertRaises(SystemError) as cm: _testcapi.module_from_slots_repeat_slot(spec) - self.assertIn(name, str(cm.exception),) - self.assertIn("repeated", str(cm.exception), ) + self.assertIn(name, str(cm.exception)) + self.assertIn("more than one", str(cm.exception)) def test_null_def_slot(self): """Slots that replace PyModuleDef fields can't be NULL""" @@ -100,5 +107,5 @@ def test_null_def_slot(self): spec._test_slot_id = getattr(_testcapi, name) with self.assertRaises(SystemError) as cm: _testcapi.module_from_slots_null_slot(spec) - self.assertIn(name, str(cm.exception),) - self.assertIn("NULL", str(cm.exception), ) + self.assertIn(name, str(cm.exception)) + self.assertIn("NULL", str(cm.exception)) diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 1ac4b2a67e597a..9912ce17a9d5ce 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -160,6 +160,23 @@ module_from_slots_exec(PyObject *self, PyObject *spec) return mod; } +static PyObject * +create_attr_from_spec(PyObject *spec, PyObject *def) +{ + assert(!def); + return PyObject_GetAttrString(spec, "_gimme_this"); +} + +static PyObject * +module_from_slots_create(PyObject *self, PyObject *spec) +{ + PyModuleDef_Slot slots[] = { + {Py_mod_create, create_attr_from_spec}, + {0}, + }; + return PyModule_FromSlotsAndSpec(slots, spec); +} + static int slot_from_object(PyObject *obj) @@ -231,6 +248,7 @@ static PyMethodDef test_methods[] = { {"module_from_slots_gc", module_from_slots_gc, METH_O}, {"module_from_slots_token", module_from_slots_token, METH_O}, {"module_from_slots_exec", module_from_slots_exec, METH_O}, + {"module_from_slots_create", module_from_slots_create, METH_O}, {"module_from_slots_repeat_slot", module_from_slots_repeat_slot, METH_O}, {"module_from_slots_null_slot", module_from_slots_null_slot, METH_O}, {"module_from_def_slot", module_from_def_slot, METH_O}, diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 2d083266848289..495705fda020b5 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -358,7 +358,7 @@ module_from_def_and_spec( if (DEST) { \ PyErr_Format( \ PyExc_SystemError, \ - "module %s: " #SLOT " slot repeated", \ + "module %s has more than one " #SLOT " slot", \ name); \ goto error; \ } \ @@ -461,7 +461,7 @@ module_from_def_and_spec( } if (create) { - m = create(spec, def_like); + m = create(spec, original_def); if (m == NULL) { if (!PyErr_Occurred()) { PyErr_Format( From 2abc3437292d9eed85da328a091cc27c206ec832 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 15:36:29 +0200 Subject: [PATCH 31/51] Use PyModule_Exec in exec_builtin_or_dynamic --- Include/internal/pycore_moduleobject.h | 2 +- Python/import.c | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/Include/internal/pycore_moduleobject.h b/Include/internal/pycore_moduleobject.h index 65c27f4bab614c..7500ef6600b570 100644 --- a/Include/internal/pycore_moduleobject.h +++ b/Include/internal/pycore_moduleobject.h @@ -22,7 +22,7 @@ typedef struct { PyObject_HEAD PyObject *md_dict; // The PyModuleDef used to define the module, if any. - // (used to be `md_def` when all extension modules had one) + // (used to be `md_def`; renamed because all uses need inspection) PyModuleDef *md_def_or_null; void *md_state; PyObject *md_weaklist; diff --git a/Python/import.c b/Python/import.c index fdaebc07cd80e7..71b187410273eb 100644 --- a/Python/import.c +++ b/Python/import.c @@ -839,25 +839,19 @@ _PyImport_SetDLOpenFlags(PyInterpreterState *interp, int new_val) /* Common implementation for _imp.exec_dynamic and _imp.exec_builtin */ static int exec_builtin_or_dynamic(PyObject *mod) { - PyModuleDef *def; void *state; if (!PyModule_Check(mod)) { return 0; } - def = PyModule_GetDef(mod); - if (def == NULL) { - return 0; - } - state = PyModule_GetState(mod); if (state) { /* Already initialized; skip reload */ return 0; } - return PyModule_ExecDef(mod, def); + return PyModule_Exec(mod); } From c0cfedaafc16d922ab706e83a9699560a3787931 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 15:53:18 +0200 Subject: [PATCH 32/51] PyModExport --- Include/internal/pycore_importdl.h | 17 +++-- Lib/test/test_capi/test_modexport.py | 29 --------- Lib/test/test_import/__init__.py | 9 +++ Python/import.c | 75 ++++++++++++++++++++-- Python/importdl.c | 95 ++++++++++++++++++---------- 5 files changed, 152 insertions(+), 73 deletions(-) delete mode 100644 Lib/test/test_capi/test_modexport.py diff --git a/Include/internal/pycore_importdl.h b/Include/internal/pycore_importdl.h index 3ba9229cc21378..75a32b607dfbbc 100644 --- a/Include/internal/pycore_importdl.h +++ b/Include/internal/pycore_importdl.h @@ -28,6 +28,11 @@ typedef enum ext_module_origin { _Py_ext_module_origin_DYNAMIC = 3, } _Py_ext_module_origin; +struct hook_prefixes { + const char *const init_prefix; + const char *const export_prefix; +}; + /* Input for loading an extension module. */ struct _Py_ext_module_loader_info { PyObject *filename; @@ -40,7 +45,7 @@ struct _Py_ext_module_loader_info { * depending on if it's builtin or not. */ PyObject *path; _Py_ext_module_origin origin; - const char *hook_prefix; + const struct hook_prefixes *hook_prefixes; const char *newcontext; }; extern void _Py_ext_module_loader_info_clear( @@ -62,7 +67,9 @@ extern int _Py_ext_module_loader_info_init_from_spec( PyObject *spec); #endif -/* The result from running an extension module's init function. */ +/* The result from running an extension module's init function. + * Not used for modules defined via PyModExport (slots array). + */ struct _Py_ext_module_loader_result { PyModuleDef *def; PyObject *module; @@ -89,10 +96,12 @@ extern void _Py_ext_module_loader_result_apply_error( /* The module init function. */ typedef PyObject *(*PyModInitFunction)(void); +typedef PyModuleDef_Slot *(*PyModExportFunction)(PyObject *spec); #ifdef HAVE_DYNAMIC_LOADING -extern PyModInitFunction _PyImport_GetModInitFunc( +// function changed signature, the "2" suffix helps avoid ABI issues +extern int _PyImport_GetModInitFunc2( struct _Py_ext_module_loader_info *info, - FILE *fp); + FILE *fp, PyModInitFunction *modinit, PyModExportFunction *modexport); #endif extern int _PyImport_RunModInitFunc( PyModInitFunction p0, diff --git a/Lib/test/test_capi/test_modexport.py b/Lib/test/test_capi/test_modexport.py deleted file mode 100644 index b89e1ab3109194..00000000000000 --- a/Lib/test/test_capi/test_modexport.py +++ /dev/null @@ -1,29 +0,0 @@ -import unittest -import importlib -import sys -from importlib.machinery import ExtensionFileLoader - -try: - import _testmultiphase -except ImportError: - _testmultiphase = None - -def import_extension_from_file(modname, filename, *, put_in_sys_modules=True): - loader = ExtensionFileLoader(modname, filename) - spec = importlib.util.spec_from_loader(modname, loader) - module = importlib.util.module_from_spec(spec) - loader.exec_module(module) - if put_in_sys_modules: - sys.modules[modname] = module - return module - - -class TestModExport(unittest.TestCase): - @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") - def test_modexport(self): - modname = '_test_modexport' - filename = _testmultiphase.__file__ - module = import_extension_from_file(modname, filename, - put_in_sys_modules=False) - - self.assertEqual(module.__name__, modname) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 6e34094c5aa422..9d65ff2319c92c 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -2466,6 +2466,15 @@ def test_multi_init_extension_per_interpreter_gil_compat(self): self.check_compatible_here( modname, filename, strict=False, isolated=False) + @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + def test_from_modexport(self): + modname = '_test_from_modexport' + filename = _testmultiphase.__file__ + module = import_extension_from_file(modname, filename, + put_in_sys_modules=False) + + self.assertEqual(module.__name__, modname) + @unittest.skipIf(_testinternalcapi is None, "requires _testinternalcapi") def test_python_compat(self): module = 'threading' diff --git a/Python/import.c b/Python/import.c index 71b187410273eb..34c97175a71292 100644 --- a/Python/import.c +++ b/Python/import.c @@ -743,7 +743,7 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp) A. noop - ...for multi-phase init modules: + ...for multi-phase init modules from PyModInit_* (PyModuleDef): (6). every time: A. _imp_create_dynamic_impl() -> import_find_extension() (not found) @@ -753,6 +753,9 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp) E. import_run_extension() -> _PyImport_RunModInitFunc() F. _PyImport_RunModInitFunc(): call G. import_run_extension() -> PyModule_FromDefAndSpec() + + PyModule_FromDefAndSpec(): + H. PyModule_FromDefAndSpec(): gather/check moduledef slots I. if there's a Py_mod_create slot: 1. PyModule_FromDefAndSpec(): call its function @@ -765,10 +768,29 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp) (10). every time: A. _imp_exec_dynamic_impl() -> exec_builtin_or_dynamic() B. if mod->md_state == NULL (including if m_size == 0): - 1. exec_builtin_or_dynamic() -> PyModule_ExecDef() - 2. PyModule_ExecDef(): allocate mod->md_state + 1. exec_builtin_or_dynamic() -> PyModule_Exec() + 2. PyModule_Exec(): allocate mod->md_state 3. if there's a Py_mod_exec slot: - 1. PyModule_ExecDef(): call its function + 1. PyModule_Exec(): call its function + + + ...for multi-phase init modules from PyModExport_* (slots array): + + (6). every time: + + A. _imp_create_dynamic_impl() -> import_find_extension() (not found) + B. _imp_create_dynamic_impl() -> _PyImport_GetModInitFunc() + C. _PyImport_GetModInitFunc(): load + D. _imp_create_dynamic_impl() -> import_run_modexport() + E. import_run_modexport(): call + F. import_run_modexport() -> PyModule_FromSlotsAndSpec() + G. PyModule_FromSlotsAndSpec(): create temporary PyModuleDef-like + H. PyModule_FromSlotsAndSpec() -> PyModule_FromDefAndSpec() + + (PyModule_FromDefAndSpec behaves as for PyModInit_*, above) + + (10). every time: as for PyModInit_*, above + */ @@ -1961,6 +1983,42 @@ import_find_extension(PyThreadState *tstate, return mod; } +static PyObject * +import_run_modexport(PyThreadState *tstate, PyModExportFunction ex0, + struct _Py_ext_module_loader_info *info, + PyObject *spec) +{ + /* This is like import_run_extension, but avoids interpreter switching + * and code for for single-phase modules. + */ + PyModuleDef_Slot *slots = ex0(spec); + if (!slots) { + if (!PyErr_Occurred()) { + PyErr_Format( + PyExc_SystemError, + "slot export function for module %s failed without setting an exception", + info->name); + } + return NULL; + } + if (PyErr_Occurred()) { + PyErr_Format( + PyExc_SystemError, + "slot export function for module %s raised unreported exception", + info->name); + } + PyObject *result = PyModule_FromSlotsAndSpec(slots, spec); + if (!result) { + return NULL; + } + assert(PyModule_Check(result)); + PyModuleObject *mod = (PyModuleObject *)result; + if (mod && !mod->md_token) { + mod->md_token = slots; + } + return result; +} + static PyObject * import_run_extension(PyThreadState *tstate, PyModInitFunction p0, struct _Py_ext_module_loader_info *info, @@ -4727,7 +4785,13 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file) fp = NULL; } - PyModInitFunction p0 = _PyImport_GetModInitFunc(&info, fp); + PyModInitFunction p0 = NULL; + PyModExportFunction ex0 = NULL; + _PyImport_GetModInitFunc2(&info, fp, &p0, &ex0); + if (ex0) { + mod = import_run_modexport(tstate, ex0, &info, spec); + goto cleanup; + } if (p0 == NULL) { goto finally; } @@ -4749,6 +4813,7 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file) } #endif +cleanup: // XXX Shouldn't this happen in the error cases too (i.e. in "finally")? if (fp) { fclose(fp); diff --git a/Python/importdl.c b/Python/importdl.c index b640f649aa1c65..02f50c0c970708 100644 --- a/Python/importdl.c +++ b/Python/importdl.c @@ -5,7 +5,7 @@ #include "pycore_call.h" // _PyObject_CallMethod() #include "pycore_import.h" // _PyImport_SwapPackageContext() #include "pycore_importdl.h" -#include "pycore_moduleobject.h" // _PyModule_GetDef() +#include "pycore_moduleobject.h" // _PyModule_GetDefOrNull() #include "pycore_pyerrors.h" // _PyErr_FormatFromCause() #include "pycore_runtime.h" // _Py_ID() @@ -35,8 +35,10 @@ extern dl_funcptr _PyImport_FindSharedFuncptr(const char *prefix, /* module info to use when loading */ /***********************************/ -static const char * const ascii_only_prefix = "PyInit"; -static const char * const nonascii_prefix = "PyInitU"; +static const struct hook_prefixes ascii_only_prefixes = { + "PyInit", "PyModExport"}; +static const struct hook_prefixes nonascii_prefixes = { + "PyInitU", "PyModExportU"}; /* Get the variable part of a module's export symbol name. * Returns a bytes instance. For non-ASCII-named modules, the name is @@ -45,7 +47,7 @@ static const char * const nonascii_prefix = "PyInitU"; * nonascii_prefix, as appropriate. */ static PyObject * -get_encoded_name(PyObject *name, const char **hook_prefix) { +get_encoded_name(PyObject *name, const struct hook_prefixes **hook_prefixes) { PyObject *tmp; PyObject *encoded = NULL; PyObject *modname = NULL; @@ -72,7 +74,7 @@ get_encoded_name(PyObject *name, const char **hook_prefix) { /* Encode to ASCII or Punycode, as needed */ encoded = PyUnicode_AsEncodedString(name, "ascii", NULL); if (encoded != NULL) { - *hook_prefix = ascii_only_prefix; + *hook_prefixes = &ascii_only_prefixes; } else { if (PyErr_ExceptionMatches(PyExc_UnicodeEncodeError)) { PyErr_Clear(); @@ -80,7 +82,7 @@ get_encoded_name(PyObject *name, const char **hook_prefix) { if (encoded == NULL) { goto error; } - *hook_prefix = nonascii_prefix; + *hook_prefixes = &nonascii_prefixes; } else { goto error; } @@ -130,7 +132,7 @@ _Py_ext_module_loader_info_init(struct _Py_ext_module_loader_info *p_info, assert(PyUnicode_GetLength(name) > 0); info.name = Py_NewRef(name); - info.name_encoded = get_encoded_name(info.name, &info.hook_prefix); + info.name_encoded = get_encoded_name(info.name, &info.hook_prefixes); if (info.name_encoded == NULL) { _Py_ext_module_loader_info_clear(&info); return -1; @@ -189,7 +191,7 @@ _Py_ext_module_loader_info_init_for_builtin( /* We won't need filename. */ .path=name, .origin=_Py_ext_module_origin_BUILTIN, - .hook_prefix=ascii_only_prefix, + .hook_prefixes=&ascii_only_prefixes, .newcontext=NULL, }; return 0; @@ -377,39 +379,62 @@ _Py_ext_module_loader_result_apply_error( /********************************************/ #ifdef HAVE_DYNAMIC_LOADING -PyModInitFunction -_PyImport_GetModInitFunc(struct _Py_ext_module_loader_info *info, - FILE *fp) +static dl_funcptr +findfuncptr(const char *prefix, const char *name_buf, + struct _Py_ext_module_loader_info *info, + FILE *fp) { - const char *name_buf = PyBytes_AS_STRING(info->name_encoded); - dl_funcptr exportfunc; #ifdef MS_WINDOWS - exportfunc = _PyImport_FindSharedFuncptrWindows( - info->hook_prefix, name_buf, info->filename, fp); + return _PyImport_FindSharedFuncptrWindows( + prefix, name_buf, info->filename, fp); #else - { - const char *path_buf = PyBytes_AS_STRING(info->filename_encoded); - exportfunc = _PyImport_FindSharedFuncptr( - info->hook_prefix, name_buf, path_buf, fp); - } + const char *path_buf = PyBytes_AS_STRING(info->filename_encoded); + return _PyImport_FindSharedFuncptr( + prefix, name_buf, path_buf, fp); #endif +} - if (exportfunc == NULL) { - if (!PyErr_Occurred()) { - PyObject *msg; - msg = PyUnicode_FromFormat( - "dynamic module does not define " - "module export function (%s_%s)", - info->hook_prefix, name_buf); - if (msg != NULL) { - PyErr_SetImportError(msg, info->name, info->filename); - Py_DECREF(msg); - } - } - return NULL; +int +_PyImport_GetModInitFunc2(struct _Py_ext_module_loader_info *info, + FILE *fp, + PyModInitFunction *modinit, + PyModExportFunction *modexport) +{ + *modinit = NULL; + *modexport = NULL; + + const char *name_buf = PyBytes_AS_STRING(info->name_encoded); + dl_funcptr exportfunc; + + exportfunc = findfuncptr( + info->hook_prefixes->export_prefix, + name_buf, info, fp); + if (exportfunc) { + *modexport = (PyModExportFunction)exportfunc; + return 2; + } + + exportfunc = findfuncptr( + info->hook_prefixes->init_prefix, + name_buf, info, fp); + if (exportfunc) { + *modinit = (PyModInitFunction)exportfunc; + return 1; } - return (PyModInitFunction)exportfunc; + if (!PyErr_Occurred()) { + PyObject *msg; + msg = PyUnicode_FromFormat( + "dynamic module does not define " + "module export function (%s_%s or %s_%s)", + info->hook_prefixes->export_prefix, name_buf, + info->hook_prefixes->init_prefix, name_buf); + if (msg != NULL) { + PyErr_SetImportError(msg, info->name, info->filename); + Py_DECREF(msg); + } + } + return -1; } #endif /* HAVE_DYNAMIC_LOADING */ @@ -477,7 +502,7 @@ _PyImport_RunModInitFunc(PyModInitFunction p0, res.def = (PyModuleDef *)m; /* Run PyModule_FromDefAndSpec() to finish loading the module. */ } - else if (info->hook_prefix == nonascii_prefix) { + else if (info->hook_prefixes == &nonascii_prefixes) { /* Non-ASCII is only supported for multi-phase init. */ res.kind = _Py_ext_module_kind_MULTIPHASE; /* Don't allow legacy init for non-ASCII module names. */ From 392d4bb7357794c49238f736555c2e2d1ee40717 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 16:48:18 +0200 Subject: [PATCH 33/51] _PyModule_IsExtension: Check md_exex --- Objects/moduleobject.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 495705fda020b5..b9ff20bd98cd7d 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -55,6 +55,9 @@ _PyModule_IsExtension(PyObject *obj) } PyModuleObject *module = (PyModuleObject*)obj; + if (module->md_exec) { + return 1; + } PyModuleDef *def = module->md_def_or_null; return (def != NULL && def->m_methods != NULL); } From 1e74b62c530136304e5d7b2431de5e3577b18fde Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 22 May 2025 16:50:38 +0200 Subject: [PATCH 34/51] Small style/doc fixups --- Doc/c-api/module.rst | 4 ++++ Modules/_testsinglephase.c | 2 ++ Objects/moduleobject.c | 8 +++++--- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Doc/c-api/module.rst b/Doc/c-api/module.rst index c8edcecc5b419f..1089bc34369fae 100644 --- a/Doc/c-api/module.rst +++ b/Doc/c-api/module.rst @@ -102,6 +102,10 @@ Module Objects Return a pointer to the :c:type:`PyModuleDef` struct from which the module was created, or ``NULL`` if the module wasn't created from a definition. + On error, return ``NULL`` with an exception set. + Use :c:func:`PyErr_Occurred` to tell this case apart from a mising + :c:type:`!PyModuleDef`. + .. c:function:: PyObject* PyModule_GetFilenameObject(PyObject *module) diff --git a/Modules/_testsinglephase.c b/Modules/_testsinglephase.c index 2c59085d15b5be..ee38d61b43a82a 100644 --- a/Modules/_testsinglephase.c +++ b/Modules/_testsinglephase.c @@ -244,6 +244,8 @@ static inline module_state * get_module_state(PyObject *module) { PyModuleDef *def = PyModule_GetDef(module); + assert(def); + if (def->m_size == -1) { return &global_state.module; } diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index b9ff20bd98cd7d..5ae947c40e0b1c 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -8,7 +8,7 @@ #include "pycore_interp.h" // PyInterpreterState.importlib #include "pycore_long.h" // _PyLong_GetOne() #include "pycore_modsupport.h" // _PyModule_CreateInitialized() -#include "pycore_moduleobject.h" // _PyModule_GetDef() +#include "pycore_moduleobject.h" // _PyModule_GetDefOrNull() #include "pycore_object.h" // _PyType_AllocNoTrack #include "pycore_pyerrors.h" // _PyErr_FormatFromCause() #include "pycore_pystate.h" // _PyInterpreterState_GET() @@ -1052,8 +1052,9 @@ module_dealloc(PyObject *self) Py_XDECREF(m->md_dict); Py_XDECREF(m->md_name); - if (m->md_state != NULL) + if (m->md_state != NULL) { PyMem_Free(m->md_state); + } Py_TYPE(m)->tp_free((PyObject *)m); } @@ -1389,8 +1390,9 @@ module_clear(PyObject *self) m->md_name ? " " : "", m->md_name, ""); } - if (res) + if (res) { return res; + } } Py_CLEAR(m->md_dict); return 0; From 4b3441060f42691a57497e1f9e169f98d994a532 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 23 May 2025 15:06:08 +0200 Subject: [PATCH 35/51] NULL slots & bad spec; some light refactoring --- Lib/test/test_capi/test_module.py | 13 +++++++++++++ Modules/_testcapi/module.c | 11 +++++++++++ Objects/moduleobject.c | 23 ++++++----------------- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 677a59e56bc9aa..51bdff9491d190 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -14,6 +14,8 @@ class FakeSpec: 'Py_mod_traverse', 'Py_mod_clear', 'Py_mod_free', 'Py_mod_token', ) +# The C functions used by this module are in: +# Modules/_testcapi/module.c class TestModFromSlotsAndSpec(unittest.TestCase): def test_empty(self): @@ -21,6 +23,17 @@ def test_empty(self): self.assertIsInstance(mod, types.ModuleType) self.assertEqual(mod.__name__, 'testmod') + def test_null_slots(self): + with self.assertRaises(SystemError): + _testcapi.module_from_slots_null(FakeSpec()) + + def test_none_spec(self): + # The spec currently must contain a name + with self.assertRaises(AttributeError): + _testcapi.module_from_slots_empty(None) + with self.assertRaises(AttributeError): + _testcapi.module_from_slots_name(None) + def test_name(self): # Py_mod_name (and PyModuleDef.m_name) are currently ignored when # spec is given. diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 9912ce17a9d5ce..3d1eca0ef81292 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -3,6 +3,10 @@ // Test PyModule_* API +/* unittest Cases that use these functions are in: + * Lib/test/test_capi/test_module.py + */ + static PyObject * module_from_slots_empty(PyObject *self, PyObject *spec) { @@ -12,6 +16,12 @@ module_from_slots_empty(PyObject *self, PyObject *spec) return PyModule_FromSlotsAndSpec(slots, spec); } +static PyObject * +module_from_slots_null(PyObject *self, PyObject *spec) +{ + return PyModule_FromSlotsAndSpec(NULL, spec); +} + static PyObject * module_from_slots_name(PyObject *self, PyObject *spec) { @@ -241,6 +251,7 @@ module_from_def_slot(PyObject *self, PyObject *spec) static PyMethodDef test_methods[] = { {"module_from_slots_empty", module_from_slots_empty, METH_O}, + {"module_from_slots_null", module_from_slots_null, METH_O}, {"module_from_slots_name", module_from_slots_name, METH_O}, {"module_from_slots_doc", module_from_slots_doc, METH_O}, {"module_from_slots_size", module_from_slots_size, METH_O}, diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 5ae947c40e0b1c..7ea3a0c32e8475 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -570,30 +570,19 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio PyObject * PyModule_FromSlotsAndSpec(PyModuleDef_Slot *slots, PyObject *spec) { - PyObject *result = NULL; if (!slots) { - PyErr_BadArgument(); - } - PyObject *nameobj = PyObject_GetAttrString(spec, "name"); - if (nameobj == NULL) { - goto finally; - } - const char *name = PyUnicode_AsUTF8(nameobj); - if (name == NULL) { - goto finally; + PyErr_Format( + PyExc_SystemError, + "PyModule_FromSlotsAndSpec called with NULL slots"); + return NULL; } - // Fill in enough of a PyModuleDef to pass to common machinery PyModuleDef def_like = {.m_slots = slots}; - result = module_from_def_and_spec(&def_like, spec, PYTHON_API_VERSION, - NULL); -finally: - Py_XDECREF(nameobj); - return result; + return module_from_def_and_spec(&def_like, spec, PYTHON_API_VERSION, + NULL); } - #ifdef Py_GIL_DISABLED int PyUnstable_Module_SetGIL(PyObject *module, void *gil) From ee55a1e139c5d7b74baf22e812ea1483f0ca724f Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 23 May 2025 15:57:09 +0200 Subject: [PATCH 36/51] Allow non-Module instances --- Python/import.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Python/import.c b/Python/import.c index 34c97175a71292..e358d7c8510c5a 100644 --- a/Python/import.c +++ b/Python/import.c @@ -2011,10 +2011,11 @@ import_run_modexport(PyThreadState *tstate, PyModExportFunction ex0, if (!result) { return NULL; } - assert(PyModule_Check(result)); - PyModuleObject *mod = (PyModuleObject *)result; - if (mod && !mod->md_token) { - mod->md_token = slots; + if (PyModule_Check(result)) { + PyModuleObject *mod = (PyModuleObject *)result; + if (mod && !mod->md_token) { + mod->md_token = slots; + } } return result; } From 16056d98e226cbaa0c9c518e14cb4535fc7e86b1 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 23 May 2025 15:57:34 +0200 Subject: [PATCH 37/51] Tests for the export hook --- Lib/test/test_import/__init__.py | 36 ++++++++++ Modules/_testmultiphase.c | 120 ++++++++++++++++++++++++++++++- 2 files changed, 154 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 9d65ff2319c92c..8e60df96be628e 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -2475,6 +2475,42 @@ def test_from_modexport(self): self.assertEqual(module.__name__, modname) + @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + def test_from_modexport_null(self): + modname = '_test_from_modexport_null' + filename = _testmultiphase.__file__ + with self.assertRaises(SystemError): + import_extension_from_file(modname, filename, + put_in_sys_modules=False) + + @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + def test_from_modexport_exception(self): + modname = '_test_from_modexport_exception' + filename = _testmultiphase.__file__ + with self.assertRaises(ValueError): + import_extension_from_file(modname, filename, + put_in_sys_modules=False) + + @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + def test_from_modexport_create_nonmodule(self): + modname = '_test_from_modexport_create_nonmodule' + filename = _testmultiphase.__file__ + module = import_extension_from_file(modname, filename, + put_in_sys_modules=False) + self.assertIsInstance(module, str) + + @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + def test_from_modexport_smoke(self): + # General positive test for sundry features + # (PyModule_FromSlotsAndSpec tests exercise test these more carefully) + modname = '_test_from_modexport_smoke' + filename = _testmultiphase.__file__ + module = import_extension_from_file(modname, filename, + put_in_sys_modules=False) + self.assertEqual(module.__doc__, "the expected docstring") + self.assertEqual(module.number, 147) + self.assertEqual(module.get_state_int(), 258) + @unittest.skipIf(_testinternalcapi is None, "requires _testinternalcapi") def test_python_compat(self): module = 'threading' diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 991cb7ca528f7d..fa9d5e38897a9c 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -994,12 +994,128 @@ PyInit__test_no_multiple_interpreter_slot(void) return PyModuleDef_Init(&no_multiple_interpreter_slot_def); } + +/* PyModExport_* hooks */ + PyMODEXPORT_FUNC PyModExport__test_from_modexport(PyObject *spec) { - static PyModuleDef_Slot from_modexport_slots[] = { + static PyModuleDef_Slot slots[] = { {Py_mod_name, "_test_from_modexport"}, {0}, }; - return from_modexport_slots; + return slots; +} + +PyMODEXPORT_FUNC +PyModExport__test_from_modexport_null(PyObject *spec) +{ + PyObject *exc; + if (PyObject_GetOptionalAttrString(spec, "_test_exception", &exc) < 0) { + return NULL; + } + if (exc) { + PyErr_SetObject((PyObject*)Py_TYPE(exc), exc); + } + Py_XDECREF(exc); + return NULL; +} + +PyMODINIT_FUNC +PyModInit__test_from_modexport_null(PyObject *spec) +{ + // This is not called as fallback for failed PyModExport_* + assert(0); + PyErr_SetString(PyExc_AssertionError, "PyInit_ fallback called"); + return NULL; +} + +PyMODEXPORT_FUNC +PyModExport__test_from_modexport_exception(PyObject *spec) +{ + PyErr_SetString(PyExc_ValueError, "failed as requested"); + return NULL; +} + +PyMODINIT_FUNC +PyModInit__test_from_modexport_exception(PyObject *spec) +{ + // This is not called as fallback for failed PyModExport_* + assert(0); + PyErr_SetString(PyExc_AssertionError, "PyInit_ fallback called"); + return NULL; +} + +static PyObject * +modexport_create_string(PyObject *spec, PyObject *def) +{ + assert(def == NULL); + return PyUnicode_FromString("is this \xf0\x9f\xa6\x8b... a module?"); +} + +PyMODEXPORT_FUNC +PyModExport__test_from_modexport_create_nonmodule(PyObject *spec) +{ + static PyModuleDef_Slot slots[] = { + {Py_mod_name, "_test_from_modexport_create_nonmodule"}, + {Py_mod_create, modexport_create_string}, + {0}, + }; + return slots; +} + +static int +modexport_smoke_exec(PyObject *mod) +{ + // "magic" values 147 & 258 are expected in the test + if (PyModule_AddIntConstant(mod, "number", 147) < 0) { + return 0; + } + int *state = PyModule_GetState(mod); + if (!state) { + return -1; + } + *state = 258; + Py_INCREF(mod); // do be cleared in modexport_smoke_free + return 0; +} + +static PyObject * +modexport_smoke_get_state_int(PyObject *mod, PyObject *arg) +{ + int *state = PyModule_GetState(mod); + if (!state) { + return NULL; + } + return PyLong_FromLong(*state); +} + +static void +modexport_smoke_free(PyObject *mod) +{ + int *state = PyModule_GetState(mod); + if (!state) { + PyErr_WriteUnraisable(mod); + } + assert(*state == 258); + Py_DECREF(mod); +} + +PyMODEXPORT_FUNC +PyModExport__test_from_modexport_smoke(PyObject *spec) +{ + static PyMethodDef methods[] = { + {"get_state_int", modexport_smoke_get_state_int, METH_NOARGS}, + {0}, + }; + static PyModuleDef_Slot slots[] = { + {Py_mod_name, "_test_from_modexport_smoke"}, + {Py_mod_doc, "the expected docstring"}, + {Py_mod_exec, modexport_smoke_exec}, + {Py_mod_size, (void*)sizeof(int)}, + {Py_mod_methods, methods}, + {Py_mod_state_free, modexport_smoke_free}, + {0}, + }; + return slots; } From 4585fc9625c1ff9ca60f566324b25164d8a63763 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 23 May 2025 16:18:25 +0200 Subject: [PATCH 38/51] `state_` in slot names --- Include/moduleobject.h | 8 ++++---- Lib/test/test_capi/test_module.py | 5 +++-- Modules/_testcapi/module.c | 16 ++++++++-------- Modules/_testmultiphase.c | 2 +- Objects/moduleobject.c | 19 +++++++++++-------- 5 files changed, 27 insertions(+), 23 deletions(-) diff --git a/Include/moduleobject.h b/Include/moduleobject.h index b0bcd0d7e7eca3..57d5b689b7151d 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -83,11 +83,11 @@ struct PyModuleDef_Slot { #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= _Py_PACK_VERSION(3, 15) #define Py_mod_name 5 #define Py_mod_doc 6 -#define Py_mod_size 7 +#define Py_mod_state_size 7 #define Py_mod_methods 8 -#define Py_mod_traverse 9 -#define Py_mod_clear 10 -#define Py_mod_free 11 +#define Py_mod_state_traverse 9 +#define Py_mod_state_clear 10 +#define Py_mod_state_free 11 #define Py_mod_token 12 #endif diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 51bdff9491d190..311eeafca2a568 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -10,8 +10,9 @@ class FakeSpec: name = 'testmod' DEF_SLOTS = ( - 'Py_mod_name', 'Py_mod_doc', 'Py_mod_size', 'Py_mod_methods', - 'Py_mod_traverse', 'Py_mod_clear', 'Py_mod_free', 'Py_mod_token', + 'Py_mod_name', 'Py_mod_doc', 'Py_mod_state_size', 'Py_mod_methods', + 'Py_mod_state_traverse', 'Py_mod_state_clear', 'Py_mod_state_free', + 'Py_mod_token', ) # The C functions used by this module are in: diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 3d1eca0ef81292..2a235e3f9db1ab 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -46,7 +46,7 @@ static PyObject * module_from_slots_size(PyObject *self, PyObject *spec) { PyModuleDef_Slot slots[] = { - {Py_mod_size, (void*)123}, + {Py_mod_state_size, (void*)123}, {0}, }; PyObject *mod = PyModule_FromSlotsAndSpec(slots, spec); @@ -96,9 +96,9 @@ static PyObject * module_from_slots_gc(PyObject *self, PyObject *spec) { PyModuleDef_Slot slots[] = { - {Py_mod_traverse, trivial_traverse}, - {Py_mod_clear, trivial_clear}, - {Py_mod_free, trivial_free}, + {Py_mod_state_traverse, trivial_traverse}, + {Py_mod_state_clear, trivial_clear}, + {Py_mod_state_free, trivial_free}, {0}, }; PyObject *mod = PyModule_FromSlotsAndSpec(slots, spec); @@ -276,11 +276,11 @@ _PyTestCapi_Init_Module(PyObject *m) ADD_INT_MACRO(Py_mod_gil); ADD_INT_MACRO(Py_mod_name); ADD_INT_MACRO(Py_mod_doc); - ADD_INT_MACRO(Py_mod_size); + ADD_INT_MACRO(Py_mod_state_size); ADD_INT_MACRO(Py_mod_methods); - ADD_INT_MACRO(Py_mod_traverse); - ADD_INT_MACRO(Py_mod_clear); - ADD_INT_MACRO(Py_mod_free); + ADD_INT_MACRO(Py_mod_state_traverse); + ADD_INT_MACRO(Py_mod_state_clear); + ADD_INT_MACRO(Py_mod_state_free); ADD_INT_MACRO(Py_mod_token); #undef ADD_INT_MACRO return PyModule_AddFunctions(m, test_methods); diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index fa9d5e38897a9c..68a9f3e839e194 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -1112,7 +1112,7 @@ PyModExport__test_from_modexport_smoke(PyObject *spec) {Py_mod_name, "_test_from_modexport_smoke"}, {Py_mod_doc, "the expected docstring"}, {Py_mod_exec, modexport_smoke_exec}, - {Py_mod_size, (void*)sizeof(int)}, + {Py_mod_state_size, (void*)sizeof(int)}, {Py_mod_methods, methods}, {Py_mod_state_free, modexport_smoke_free}, {0}, diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 7ea3a0c32e8475..8636ad5cc63b47 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -412,22 +412,25 @@ module_from_def_and_spec( case Py_mod_doc: COPY_COMMON_SLOT(Py_mod_doc, char*, def_like->m_doc); break; - case Py_mod_size: - COPY_COMMON_SLOT(Py_mod_size, Py_ssize_t, def_like->m_size); + case Py_mod_state_size: + COPY_COMMON_SLOT(Py_mod_state_size, Py_ssize_t, + def_like->m_size); break; case Py_mod_methods: COPY_COMMON_SLOT(Py_mod_methods, PyMethodDef*, def_like->m_methods); break; - case Py_mod_traverse: - COPY_COMMON_SLOT(Py_mod_traverse, traverseproc, + case Py_mod_state_traverse: + COPY_COMMON_SLOT(Py_mod_state_traverse, traverseproc, def_like->m_traverse); break; - case Py_mod_clear: - COPY_COMMON_SLOT(Py_mod_clear, inquiry, def_like->m_clear); + case Py_mod_state_clear: + COPY_COMMON_SLOT(Py_mod_state_clear, inquiry, + def_like->m_clear); break; - case Py_mod_free: - COPY_COMMON_SLOT(Py_mod_free, freefunc, def_like->m_free); + case Py_mod_state_free: + COPY_COMMON_SLOT(Py_mod_state_free, freefunc, + def_like->m_free); break; case Py_mod_token: COPY_COMMON_SLOT(Py_mod_token, void*, token); From 8311cc6c2c42a4a5f810cb8dc5430544dd1f59ad Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 23 May 2025 16:47:29 +0200 Subject: [PATCH 39/51] Fix multiple exec slots --- Lib/test/test_import/__init__.py | 15 +++++++++++++++ Modules/_testmultiphase.c | 20 ++++++++++++++++++++ Objects/moduleobject.c | 2 +- 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 8e60df96be628e..bc3ab78e489056 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -2466,6 +2466,21 @@ def test_multi_init_extension_per_interpreter_gil_compat(self): self.check_compatible_here( modname, filename, strict=False, isolated=False) + @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + def test_testmultiphase_exec_multiple(self): + modname = '_testmultiphase_exec_multiple' + filename = _testmultiphase.__file__ + module = import_extension_from_file(modname, filename, + put_in_sys_modules=False) + # All three exec's were called. + self.assertEqual(module.a, 1) + self.assertEqual(module.b, 2) + self.assertEqual(module.c, 3) + # They were called in order. + keys = list(module.__dict__) + self.assertLess(keys.index('a'), keys.index('b')) + self.assertLess(keys.index('b'), keys.index('c')) + @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") def test_from_modexport(self): modname = '_test_from_modexport' diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 68a9f3e839e194..17e1e308d20597 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -850,6 +850,26 @@ PyInit__testmultiphase_exec_unreported_exception(void) return PyModuleDef_Init(&def_exec_unreported_exception); } +static int execfunc_a1(PyObject*m) {return PyModule_AddIntConstant(m, "a", 1);} +static int execfunc_b2(PyObject*m) {return PyModule_AddIntConstant(m, "b", 2);} +static int execfunc_c3(PyObject*m) {return PyModule_AddIntConstant(m, "c", 3);} + +PyMODINIT_FUNC +PyInit__testmultiphase_exec_multiple(void) +{ + static PyModuleDef_Slot slots[] = { + {Py_mod_exec, execfunc_a1}, + {Py_mod_exec, execfunc_b2}, + {Py_mod_exec, execfunc_c3}, + {0} + }; + static PyModuleDef def = { + .m_name="_testmultiphase_exec_multiple", + .m_slots=slots, + }; + return PyModuleDef_Init(&def); +} + static int meth_state_access_exec(PyObject *m) { diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 8636ad5cc63b47..1820cdb6b18961 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -685,7 +685,7 @@ PyModule_ExecDef(PyObject *module, PyModuleDef *def) if (run_exec_func(module, func) < 0) { return -1; } - break; + continue; } } return 0; From 61fc3bbafab4c6aabc5d0d0b48739d11ba4713d5 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 23 May 2025 17:41:15 +0200 Subject: [PATCH 40/51] Modules are bigger sue me --- Lib/test/test_sys.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 39e62027f03e5a..a9080ca8a1ee31 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1707,9 +1707,10 @@ def get_gen(): yield 1 check(int(PyLong_BASE**2), vsize('') + 3*self.longdigit) # module if support.Py_GIL_DISABLED: - check(unittest, size('PPPPPP')) + md_gil = 'P' else: - check(unittest, size('PPPPP')) + md_gil = '' + check(unittest, size('PPPPP' + md_gil + 'NPPPPP')) # None check(None, size('')) # NotImplementedType From caa10fc76e2f410d123ec7938be27357190d1036 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Sat, 7 Jun 2025 18:04:45 +0200 Subject: [PATCH 41/51] Test PyModule_Exec with PyModuleDef-defined module --- Lib/test/test_capi/test_module.py | 11 ++++++ Modules/_testcapi/module.c | 57 +++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 311eeafca2a568..427a01e41897f7 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -123,3 +123,14 @@ def test_null_def_slot(self): _testcapi.module_from_slots_null_slot(spec) self.assertIn(name, str(cm.exception)) self.assertIn("NULL", str(cm.exception)) + + def test_def_multiple_exec(self): + """PyModule_Exec runs all exec slots of PyModuleDef-defined module""" + mod = _testcapi.module_from_def_multiple_exec(FakeSpec()) + self.assertFalse(hasattr(mod, 'a_number')) + _testcapi.pymodule_exec(mod) + self.assertEqual(mod.a_number, 456) + self.assertEqual(mod.another_number, 789) + _testcapi.pymodule_exec(mod) + self.assertEqual(mod.a_number, 456) + self.assertEqual(mod.another_number, -789) diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 2a235e3f9db1ab..e59b3f5af32b26 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -246,9 +246,64 @@ module_from_def_slot(PyObject *self, PyObject *spec) .m_name = "currently ignored", .m_slots = slots, }; + // PyModuleDef is normally static; the real requirement is that it + // must outlive its module. + // Here, module creation fails, so it's fine on the stack. + PyObject *result = PyModule_FromDefAndSpec(&def, spec); + assert(result == NULL); + return result; +} + +static int +another_exec(PyObject *module) +{ + /* Make sure simple_exec was called */ + assert(PyObject_HasAttrString(module, "a_number")); + + /* Add or negate a global called 'another_number' */ + PyObject *another_number; + if (PyObject_GetOptionalAttrString(module, "another_number", + &another_number) < 0) { + return -1; + } + if (!another_number) { + return PyModule_AddIntConstant(module, "another_number", 789); + } + PyObject *neg_number = PyNumber_Negative(another_number); + Py_DECREF(another_number); + if (!neg_number) { + return -1; + } + int result = PyObject_SetAttrString(module, "another_number", + neg_number); + Py_DECREF(neg_number); + return result; +} + +static PyObject * +module_from_def_multiple_exec(PyObject *self, PyObject *spec) +{ + static PyModuleDef_Slot slots[] = { + {Py_mod_exec, simple_exec}, + {Py_mod_exec, another_exec}, + {0}, + }; + static PyModuleDef def = { + .m_name = "currently ignored", + .m_slots = slots, + }; return PyModule_FromDefAndSpec(&def, spec); } +static PyObject * +pymodule_exec(PyObject *self, PyObject *module) +{ + if (PyModule_Exec(module) < 0) { + return NULL; + } + Py_RETURN_NONE; +} + static PyMethodDef test_methods[] = { {"module_from_slots_empty", module_from_slots_empty, METH_O}, {"module_from_slots_null", module_from_slots_null, METH_O}, @@ -262,7 +317,9 @@ static PyMethodDef test_methods[] = { {"module_from_slots_create", module_from_slots_create, METH_O}, {"module_from_slots_repeat_slot", module_from_slots_repeat_slot, METH_O}, {"module_from_slots_null_slot", module_from_slots_null_slot, METH_O}, + {"module_from_def_multiple_exec", module_from_def_multiple_exec, METH_O}, {"module_from_def_slot", module_from_def_slot, METH_O}, + {"pymodule_exec", pymodule_exec, METH_O}, {NULL}, }; From 422f82a9906926277ee3dd07359c25d9a29fda35 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 17 Jun 2025 15:40:05 +0200 Subject: [PATCH 42/51] Test PyModule_GetToken, token setting, and default tokens --- Lib/test/test_capi/test_module.py | 27 +++++++++++++++++++++++++ Lib/test/test_import/__init__.py | 33 ++++++++++++++++++++++++++++++- Modules/_testcapi/module.c | 27 +++++++++++++++++++++++++ Modules/_testmultiphase.c | 31 +++++++++++++++++++++++++++++ Objects/moduleobject.c | 4 ++-- 5 files changed, 119 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 427a01e41897f7..09350c7614ed82 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -18,10 +18,17 @@ class FakeSpec: # The C functions used by this module are in: # Modules/_testcapi/module.c +def def_and_token(mod): + return ( + _testcapi.pymodule_get_def(mod), + _testcapi.pymodule_get_token(mod), + ) + class TestModFromSlotsAndSpec(unittest.TestCase): def test_empty(self): mod = _testcapi.module_from_slots_empty(FakeSpec()) self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(def_and_token(mod), (0, 0)) self.assertEqual(mod.__name__, 'testmod') def test_null_slots(self): @@ -41,18 +48,21 @@ def test_name(self): # We still test that it's accepted. mod = _testcapi.module_from_slots_name(FakeSpec()) self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(def_and_token(mod), (0, 0)) self.assertEqual(mod.__name__, 'testmod') self.assertEqual(mod.__doc__, None) def test_doc(self): mod = _testcapi.module_from_slots_doc(FakeSpec()) self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(def_and_token(mod), (0, 0)) self.assertEqual(mod.__name__, 'testmod') self.assertEqual(mod.__doc__, 'the docstring') def test_size(self): mod = _testcapi.module_from_slots_size(FakeSpec()) self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(def_and_token(mod), (0, 0)) self.assertEqual(mod.__name__, 'testmod') self.assertEqual(mod.__doc__, None) self.assertEqual(mod.size, 123) @@ -60,6 +70,7 @@ def test_size(self): def test_methods(self): mod = _testcapi.module_from_slots_methods(FakeSpec()) self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(def_and_token(mod), (0, 0)) self.assertEqual(mod.__name__, 'testmod') self.assertEqual(mod.__doc__, None) self.assertEqual(mod.a_method(456), (mod, 456)) @@ -67,18 +78,21 @@ def test_methods(self): def test_gc(self): mod = _testcapi.module_from_slots_gc(FakeSpec()) self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(def_and_token(mod), (0, 0)) self.assertEqual(mod.__name__, 'testmod') self.assertEqual(mod.__doc__, None) def test_token(self): mod = _testcapi.module_from_slots_token(FakeSpec()) self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(def_and_token(mod), (0, _testcapi.module_test_token)) self.assertEqual(mod.__name__, 'testmod') self.assertEqual(mod.__doc__, None) def test_exec(self): mod = _testcapi.module_from_slots_exec(FakeSpec()) self.assertIsInstance(mod, types.ModuleType) + self.assertEqual(def_and_token(mod), (0, 0)) self.assertEqual(mod.__name__, 'testmod') self.assertEqual(mod.__doc__, None) self.assertEqual(mod.a_number, 456) @@ -89,6 +103,10 @@ def test_create(self): mod = _testcapi.module_from_slots_create(spec) self.assertIsInstance(mod, str) self.assertEqual(mod, "not a module object") + with self.assertRaises(TypeError): + _testcapi.pymodule_get_def(mod), + with self.assertRaises(TypeError): + _testcapi.pymodule_get_token(mod) def test_def_slot(self): """Slots that replace PyModuleDef fields can't be used with PyModuleDef @@ -134,3 +152,12 @@ def test_def_multiple_exec(self): _testcapi.pymodule_exec(mod) self.assertEqual(mod.a_number, 456) self.assertEqual(mod.another_number, -789) + def_ptr, token = def_and_token(mod) + self.assertEqual(def_ptr, token) + + def test_def_token(self): + """In PyModuleDef-defined modules, the def is the token""" + mod = _testcapi.module_from_def_multiple_exec(FakeSpec()) + def_ptr, token = def_and_token(mod) + self.assertEqual(def_ptr, token) + self.assertGreater(def_ptr, 0) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index bc3ab78e489056..0d6bea865cb4f2 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -2517,7 +2517,7 @@ def test_from_modexport_create_nonmodule(self): @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") def test_from_modexport_smoke(self): # General positive test for sundry features - # (PyModule_FromSlotsAndSpec tests exercise test these more carefully) + # (PyModule_FromSlotsAndSpec tests exercise these more carefully) modname = '_test_from_modexport_smoke' filename = _testmultiphase.__file__ module = import_extension_from_file(modname, filename, @@ -2525,6 +2525,37 @@ def test_from_modexport_smoke(self): self.assertEqual(module.__doc__, "the expected docstring") self.assertEqual(module.number, 147) self.assertEqual(module.get_state_int(), 258) + self.assertGreater(module.get_test_token(), 0) + + @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + def test_from_modexport_smoke_token(self): + _testcapi = import_module("_testcapi") + + modname = '_test_from_modexport_smoke' + filename = _testmultiphase.__file__ + module = import_extension_from_file(modname, filename, + put_in_sys_modules=False) + self.assertEqual(_testcapi.pymodule_get_token(module), + module.get_test_token()) + + @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") + def test_from_modexport_empty_slots(self): + # Module to test that: + # - no slots are mandatory for PyModExport + # - the slots array is used as the default token + modname = '_test_from_modexport_empty_slots' + filename = _testmultiphase.__file__ + module = import_extension_from_file( + modname, filename, put_in_sys_modules=False) + + self.assertEqual(module.__name__, modname) + self.assertEqual(module.__doc__, None) + + _testcapi = import_module("_testcapi") + smoke_mod = import_extension_from_file( + '_test_from_modexport_smoke', filename, put_in_sys_modules=False) + self.assertEqual(_testcapi.pymodule_get_token(module), + smoke_mod.get_modexport_empty_slots()) @unittest.skipIf(_testinternalcapi is None, "requires _testinternalcapi") def test_python_compat(self): diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index e59b3f5af32b26..20567eafc79c46 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -304,6 +304,26 @@ pymodule_exec(PyObject *self, PyObject *module) Py_RETURN_NONE; } +static PyObject * +pymodule_get_token(PyObject *self, PyObject *module) +{ + void *token; + if (PyModule_GetToken(module, &token) < 0) { + return NULL; + } + return PyLong_FromVoidPtr(token); +} + +static PyObject * +pymodule_get_def(PyObject *self, PyObject *module) +{ + PyModuleDef *def = PyModule_GetDef(module); + if (!def && PyErr_Occurred()) { + return NULL; + } + return PyLong_FromVoidPtr(def); +} + static PyMethodDef test_methods[] = { {"module_from_slots_empty", module_from_slots_empty, METH_O}, {"module_from_slots_null", module_from_slots_null, METH_O}, @@ -319,6 +339,8 @@ static PyMethodDef test_methods[] = { {"module_from_slots_null_slot", module_from_slots_null_slot, METH_O}, {"module_from_def_multiple_exec", module_from_def_multiple_exec, METH_O}, {"module_from_def_slot", module_from_def_slot, METH_O}, + {"pymodule_get_token", pymodule_get_token, METH_O}, + {"pymodule_get_def", pymodule_get_def, METH_O}, {"pymodule_exec", pymodule_exec, METH_O}, {NULL}, }; @@ -340,5 +362,10 @@ _PyTestCapi_Init_Module(PyObject *m) ADD_INT_MACRO(Py_mod_state_free); ADD_INT_MACRO(Py_mod_token); #undef ADD_INT_MACRO + if (PyModule_Add(m, "module_test_token", + PyLong_FromVoidPtr(&test_token)) < 0) + { + return -1; + } return PyModule_AddFunctions(m, test_methods); } diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 17e1e308d20597..61f22c9a1d1e42 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -1084,6 +1084,16 @@ PyModExport__test_from_modexport_create_nonmodule(PyObject *spec) return slots; } +static PyModuleDef_Slot modexport_empty_slots[] = { + {0}, +}; + +PyMODEXPORT_FUNC +PyModExport__test_from_modexport_empty_slots(PyObject *spec) +{ + return modexport_empty_slots; +} + static int modexport_smoke_exec(PyObject *mod) { @@ -1110,6 +1120,24 @@ modexport_smoke_get_state_int(PyObject *mod, PyObject *arg) return PyLong_FromLong(*state); } +static char modexport_smoke_test_token; + +static PyObject * +modexport_smoke_get_test_token(PyObject *mod, PyObject *arg) +{ + return PyLong_FromVoidPtr(&modexport_smoke_test_token); +} + +static PyObject * +modexport_get_empty_slots(PyObject *mod, PyObject *arg) +{ + /* Get the address of modexport_empty_slots. + * This method would be in the `_test_from_modexport_empty_slots` module, + * if it had a methods slot. + */ + return PyLong_FromVoidPtr(&modexport_empty_slots); +} + static void modexport_smoke_free(PyObject *mod) { @@ -1126,6 +1154,8 @@ PyModExport__test_from_modexport_smoke(PyObject *spec) { static PyMethodDef methods[] = { {"get_state_int", modexport_smoke_get_state_int, METH_NOARGS}, + {"get_test_token", modexport_smoke_get_test_token, METH_NOARGS}, + {"get_modexport_empty_slots", modexport_get_empty_slots, METH_NOARGS}, {0}, }; static PyModuleDef_Slot slots[] = { @@ -1135,6 +1165,7 @@ PyModExport__test_from_modexport_smoke(PyObject *spec) {Py_mod_state_size, (void*)sizeof(int)}, {Py_mod_methods, methods}, {Py_mod_state_free, modexport_smoke_free}, + {Py_mod_token, (void*)&modexport_smoke_test_token}, {0}, }; return slots; diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 1820cdb6b18961..c81c9b4edae2b3 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -734,7 +734,7 @@ PyModule_GetSize(PyObject *m, Py_ssize_t *size_p) { *size_p = -1; if (!PyModule_Check(m)) { - PyErr_BadInternalCall(); + PyErr_Format(PyExc_TypeError, "expected module, got %T", m); return -1; } PyModuleObject *mod = (PyModuleObject *)m; @@ -747,7 +747,7 @@ PyModule_GetToken(PyObject *m, void **token_p) { *token_p = NULL; if (!PyModule_Check(m)) { - PyErr_BadInternalCall(); + PyErr_Format(PyExc_TypeError, "expected module, got %T", m); return -1; } PyModuleObject *mod = (PyModuleObject *)m; From 7dc1f8fdd49451ab94fb51a80ef10407461fe71b Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 17 Jun 2025 16:32:56 +0200 Subject: [PATCH 43/51] Add PyType_GetModuleByToken --- Include/internal/pycore_moduleobject.h | 9 +++++++++ Include/object.h | 4 ++++ Lib/test/test_capi/test_type.py | 18 ++++++++++++++++++ Lib/test/test_import/__init__.py | 10 ++++++++-- Modules/_testcapi/heaptype.c | 16 ++++++++++++++++ Modules/_testmultiphase.c | 5 +++++ Objects/moduleobject.c | 4 ++-- Objects/typeobject.c | 22 +++++++++++++++++----- 8 files changed, 79 insertions(+), 9 deletions(-) diff --git a/Include/internal/pycore_moduleobject.h b/Include/internal/pycore_moduleobject.h index 7500ef6600b570..2567429ac1b6dc 100644 --- a/Include/internal/pycore_moduleobject.h +++ b/Include/internal/pycore_moduleobject.h @@ -44,6 +44,15 @@ static inline PyModuleDef* _PyModule_GetDefOrNull(PyObject *mod) { return ((PyModuleObject *)mod)->md_def_or_null; } +static inline PyModuleDef* _PyModule_GetToken(PyObject *arg) { + assert(PyModule_Check(arg)); + PyModuleObject *mod = (PyModuleObject *)arg; + if (mod->md_def_or_null) { + assert(mod->md_def_or_null == mod->md_token); + } + return mod->md_token; +} + static inline void* _PyModule_GetState(PyObject* mod) { assert(PyModule_Check(mod)); return ((PyModuleObject *)mod)->md_state; diff --git a/Include/object.h b/Include/object.h index c75e9db0cbd935..708a1753cd7048 100644 --- a/Include/object.h +++ b/Include/object.h @@ -826,6 +826,10 @@ PyAPI_FUNC(PyObject *) PyType_GetModuleByDef(PyTypeObject *, PyModuleDef *); PyAPI_FUNC(int) PyType_Freeze(PyTypeObject *type); #endif +#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= _Py_PACK_VERSION(3, 15) +PyAPI_FUNC(PyObject *) PyType_GetModuleByToken(PyTypeObject *, void *); +#endif + #ifdef __cplusplus } #endif diff --git a/Lib/test/test_capi/test_type.py b/Lib/test/test_capi/test_type.py index 15fb4a93e2ad74..450f57046f7cbc 100644 --- a/Lib/test/test_capi/test_type.py +++ b/Lib/test/test_capi/test_type.py @@ -195,6 +195,24 @@ class H2(int): pass with self.assertRaises(TypeError): _testcapi.pytype_getmodulebydef(H2) + def test_get_module_by_token(self): + token = _testcapi.pymodule_get_token(_testcapi) + + heaptype = _testcapi.create_type_with_token('_testcapi.H', 0) + mod = _testcapi.pytype_getmodulebytoken(heaptype, token) + self.assertIs(mod, _testcapi) + + class H1(heaptype): pass + mod = _testcapi.pytype_getmodulebytoken(H1, token) + self.assertIs(mod, _testcapi) + + with self.assertRaises(TypeError): + _testcapi.pytype_getmodulebytoken(int, token) + + class H2(int): pass + with self.assertRaises(TypeError): + _testcapi.pytype_getmodulebytoken(H2, token) + def test_freeze(self): # test PyType_Freeze() type_freeze = _testcapi.type_freeze diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 0d6bea865cb4f2..236643f7930708 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -2535,8 +2535,14 @@ def test_from_modexport_smoke_token(self): filename = _testmultiphase.__file__ module = import_extension_from_file(modname, filename, put_in_sys_modules=False) - self.assertEqual(_testcapi.pymodule_get_token(module), - module.get_test_token()) + token = module.get_test_token() + self.assertEqual(_testcapi.pymodule_get_token(module), token) + + tp = module.Example + self.assertEqual(_testcapi.pytype_getmodulebytoken(tp, token), module) + class Sub(tp): + pass + self.assertEqual(_testcapi.pytype_getmodulebytoken(Sub, token), module) @unittest.skipIf(_testmultiphase is None, "test requires _testmultiphase module") def test_from_modexport_empty_slots(self): diff --git a/Modules/_testcapi/heaptype.c b/Modules/_testcapi/heaptype.c index 257e0256655976..7963ea56a9b48c 100644 --- a/Modules/_testcapi/heaptype.c +++ b/Modules/_testcapi/heaptype.c @@ -528,6 +528,21 @@ pytype_getmodulebydef(PyObject *self, PyObject *type) return Py_XNewRef(mod); } +static PyObject * +pytype_getmodulebytoken(PyObject *self, PyObject *args) +{ + PyObject *type; + PyObject *py_token; + if (!PyArg_ParseTuple(args, "OO", &type, &py_token)) { + return NULL; + } + void *token = PyLong_AsVoidPtr(py_token); + if ((!token) && PyErr_Occurred()) { + return NULL; + } + return PyType_GetModuleByToken((PyTypeObject *)type, token); +} + static PyMethodDef TestMethods[] = { {"pytype_fromspec_meta", pytype_fromspec_meta, METH_O}, @@ -546,6 +561,7 @@ static PyMethodDef TestMethods[] = { {"get_tp_token", get_tp_token, METH_O}, {"pytype_getbasebytoken", pytype_getbasebytoken, METH_VARARGS}, {"pytype_getmodulebydef", pytype_getmodulebydef, METH_O}, + {"pytype_getmodulebytoken", pytype_getmodulebytoken, METH_VARARGS}, {NULL}, }; diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 61f22c9a1d1e42..eb6c0ad51ec2da 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -1107,6 +1107,11 @@ modexport_smoke_exec(PyObject *mod) } *state = 258; Py_INCREF(mod); // do be cleared in modexport_smoke_free + + PyModule_AddObjectRef( + mod, "Example", + PyType_FromModuleAndSpec(mod, &StateAccessType_spec, NULL)); + return 0; } diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index c81c9b4edae2b3..749aad02f3ba06 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -290,6 +290,7 @@ _PyModule_CreateInitialized(PyModuleDef* module, int module_api_version) } } m->md_def_or_null = module; + m->md_token = module; module_copy_members_from_deflike(m, module); #ifdef Py_GIL_DISABLED m->md_gil = Py_MOD_GIL_USED; @@ -750,8 +751,7 @@ PyModule_GetToken(PyObject *m, void **token_p) PyErr_Format(PyExc_TypeError, "expected module, got %T", m); return -1; } - PyModuleObject *mod = (PyModuleObject *)m; - *token_p = mod->md_token; + *token_p = _PyModule_GetToken(m); return 0; } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 7c7443f6220cbf..88e25f47c4cc7b 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5699,11 +5699,11 @@ PyType_GetModuleState(PyTypeObject *type) } -/* Get the module of the first superclass where the module has the - * given PyModuleDef. +/* Return borrowed ref to the module of the first superclass where the module + * has the given token. */ PyObject * -PyType_GetModuleByDef(PyTypeObject *type, PyModuleDef *def) +borrow_module_by_token(PyTypeObject *type, void *token) { assert(PyType_Check(type)); @@ -5715,7 +5715,7 @@ PyType_GetModuleByDef(PyTypeObject *type, PyModuleDef *def) else { PyHeapTypeObject *ht = (PyHeapTypeObject*)type; PyObject *module = ht->ht_module; - if (module && _PyModule_GetDefOrNull(module) == def) { + if (module && _PyModule_GetToken(module) == token) { return module; } } @@ -5743,7 +5743,7 @@ PyType_GetModuleByDef(PyTypeObject *type, PyModuleDef *def) PyHeapTypeObject *ht = (PyHeapTypeObject*)super; PyObject *module = ht->ht_module; - if (module && _PyModule_GetDefOrNull(module) == def) { + if (module && _PyModule_GetToken(module) == token) { res = module; break; } @@ -5761,6 +5761,18 @@ PyType_GetModuleByDef(PyTypeObject *type, PyModuleDef *def) return NULL; } +PyObject * +PyType_GetModuleByDef(PyTypeObject *type, PyModuleDef *def) +{ + return borrow_module_by_token(type, def); +} + +PyObject * +PyType_GetModuleByToken(PyTypeObject *type, void *token) +{ + return Py_XNewRef(borrow_module_by_token(type, token)); +} + static PyTypeObject * get_base_by_token_recursive(PyObject *bases, void *token) From b689cce67d29fbebfad5fbca6c141eab4e78297c Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 17 Jun 2025 16:48:05 +0200 Subject: [PATCH 44/51] Fix refcounting --- Modules/_testmultiphase.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index eb6c0ad51ec2da..166db79123becf 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -1106,11 +1106,11 @@ modexport_smoke_exec(PyObject *mod) return -1; } *state = 258; - Py_INCREF(mod); // do be cleared in modexport_smoke_free - PyModule_AddObjectRef( - mod, "Example", - PyType_FromModuleAndSpec(mod, &StateAccessType_spec, NULL)); + PyObject *tp = PyType_FromModuleAndSpec(mod, &StateAccessType_spec, NULL); + if (PyModule_Add(mod, "Example", tp) < 0) { + return -1; + } return 0; } @@ -1151,7 +1151,6 @@ modexport_smoke_free(PyObject *mod) PyErr_WriteUnraisable(mod); } assert(*state == 258); - Py_DECREF(mod); } PyMODEXPORT_FUNC From 55beb5468fe8f4a62f0d03da488421fbd1c268d6 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 19 Jun 2025 13:41:01 +0200 Subject: [PATCH 45/51] Rename and test PyModule_GetStateSize --- Include/moduleobject.h | 2 +- Lib/test/test_capi/test_module.py | 17 +++++++++++++++-- Modules/_testcapi/module.c | 20 +++++++++++--------- Objects/moduleobject.c | 2 +- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/Include/moduleobject.h b/Include/moduleobject.h index 57d5b689b7151d..e1d72eb8ae2613 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -121,7 +121,7 @@ PyAPI_FUNC(int) PyUnstable_Module_SetGIL(PyObject *module, void *gil); PyAPI_FUNC(PyObject *) PyModule_FromSlotsAndSpec(PyModuleDef_Slot *, PyObject *spec); PyAPI_FUNC(int) PyModule_Exec(PyObject *mod); -PyAPI_FUNC(int) PyModule_GetSize(PyObject *mod, Py_ssize_t *size_p); +PyAPI_FUNC(int) PyModule_GetStateSize(PyObject *mod, Py_ssize_t *size_p); PyAPI_FUNC(int) PyModule_GetToken(PyObject *, void **token_p); #endif diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 09350c7614ed82..bc19ef76cd2174 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -1,6 +1,6 @@ import unittest import types -from test.support import import_helper +from test.support import import_helper, subTests # Skip this test if the _testcapi module isn't available. _testcapi = import_helper.import_module('_testcapi') @@ -30,6 +30,8 @@ def test_empty(self): self.assertIsInstance(mod, types.ModuleType) self.assertEqual(def_and_token(mod), (0, 0)) self.assertEqual(mod.__name__, 'testmod') + size = _testcapi.pymodule_get_state_size(mod) + self.assertEqual(size, 0) def test_null_slots(self): with self.assertRaises(SystemError): @@ -65,7 +67,8 @@ def test_size(self): self.assertEqual(def_and_token(mod), (0, 0)) self.assertEqual(mod.__name__, 'testmod') self.assertEqual(mod.__doc__, None) - self.assertEqual(mod.size, 123) + size = _testcapi.pymodule_get_state_size(mod) + self.assertEqual(size, 123) def test_methods(self): mod = _testcapi.module_from_slots_methods(FakeSpec()) @@ -161,3 +164,13 @@ def test_def_token(self): def_ptr, token = def_and_token(mod) self.assertEqual(def_ptr, token) self.assertGreater(def_ptr, 0) + + @subTests('name, expected_size', [ + (__name__, 0), # Python module + ('_testsinglephase', -1), # single-phase init + ('sys', -1), + ]) + def test_get_state_size(self, name, expected_size): + mod = import_helper.import_module(name) + size = _testcapi.pymodule_get_state_size(mod) + self.assertEqual(size, expected_size) diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index 20567eafc79c46..ae4e3db654662a 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -53,15 +53,6 @@ module_from_slots_size(PyObject *self, PyObject *spec) if (!mod) { return NULL; } - Py_ssize_t size; - if (PyModule_GetSize(mod, &size) < 0) { - Py_DECREF(mod); - return NULL; - } - if (PyModule_AddIntConstant(mod, "size", size) < 0) { - Py_DECREF(mod); - return NULL; - } return mod; } @@ -324,6 +315,16 @@ pymodule_get_def(PyObject *self, PyObject *module) return PyLong_FromVoidPtr(def); } +static PyObject * +pymodule_get_state_size(PyObject *self, PyObject *module) +{ + Py_ssize_t size; + if (PyModule_GetStateSize(module, &size) < 0) { + return NULL; + } + return PyLong_FromSsize_t(size); +} + static PyMethodDef test_methods[] = { {"module_from_slots_empty", module_from_slots_empty, METH_O}, {"module_from_slots_null", module_from_slots_null, METH_O}, @@ -341,6 +342,7 @@ static PyMethodDef test_methods[] = { {"module_from_def_slot", module_from_def_slot, METH_O}, {"pymodule_get_token", pymodule_get_token, METH_O}, {"pymodule_get_def", pymodule_get_def, METH_O}, + {"pymodule_get_state_size", pymodule_get_state_size, METH_O}, {"pymodule_exec", pymodule_exec, METH_O}, {NULL}, }; diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 749aad02f3ba06..9a4d2733186550 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -731,7 +731,7 @@ PyModule_GetDict(PyObject *m) } int -PyModule_GetSize(PyObject *m, Py_ssize_t *size_p) +PyModule_GetStateSize(PyObject *m, Py_ssize_t *size_p) { *size_p = -1; if (!PyModule_Check(m)) { From d48deedd60a3e6f6d66dc867e94600427630dcad Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 19 Jun 2025 14:06:51 +0200 Subject: [PATCH 46/51] Rename new internal module members --- Include/internal/pycore_moduleobject.h | 8 ++--- Objects/moduleobject.c | 44 +++++++++++++------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/Include/internal/pycore_moduleobject.h b/Include/internal/pycore_moduleobject.h index 2567429ac1b6dc..bab4844a79e058 100644 --- a/Include/internal/pycore_moduleobject.h +++ b/Include/internal/pycore_moduleobject.h @@ -31,10 +31,10 @@ typedef struct { #ifdef Py_GIL_DISABLED void *md_gil; #endif - Py_ssize_t md_size; - traverseproc md_traverse; - inquiry md_clear; - freefunc md_free; + Py_ssize_t md_state_size; + traverseproc md_state_traverse; + inquiry md_state_clear; + freefunc md_state_free; void *md_token; _Py_modexecfunc md_exec; /* only set if md_def_or_null is NULL */ } PyModuleObject; diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 9a4d2733186550..dca61ea8c1b71e 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -29,7 +29,7 @@ static PyMemberDef module_members[] = { static void assert_def_missing_or_redundant(PyModuleObject *m) { if (m->md_def_or_null) { -#define DO_ASSERT(F) assert (m->md_def_or_null->m_ ## F == m->md_ ## F); +#define DO_ASSERT(F) assert (m->md_def_or_null->m_ ## F == m->md_state_ ## F); DO_ASSERT(size); DO_ASSERT(traverse); DO_ASSERT(clear); @@ -111,10 +111,10 @@ new_module_notrack(PyTypeObject *mt) m->md_state = NULL; m->md_weaklist = NULL; m->md_name = NULL; - m->md_size = 0; - m->md_traverse = NULL; - m->md_clear = NULL; - m->md_free = NULL; + m->md_state_size = 0; + m->md_state_traverse = NULL; + m->md_state_clear = NULL; + m->md_state_free = NULL; m->md_exec = NULL; m->md_token = NULL; m->md_dict = PyDict_New(); @@ -237,10 +237,10 @@ module_copy_members_from_deflike( PyModuleDef *def_like /* not necessarily a valid Python object */) { /* def may not be a valid PyObject*, see */ - md->md_size = def_like->m_size; - md->md_traverse = def_like->m_traverse; - md->md_clear = def_like->m_clear; - md->md_free = def_like->m_free; + md->md_state_size = def_like->m_size; + md->md_state_traverse = def_like->m_traverse; + md->md_state_clear = def_like->m_clear; + md->md_state_free = def_like->m_free; } PyObject * @@ -632,16 +632,16 @@ alloc_state(PyObject *module) } PyModuleObject *md = (PyModuleObject*)module; - if (md->md_size >= 0) { + if (md->md_state_size >= 0) { if (md->md_state == NULL) { /* Always set a state pointer; this serves as a marker to skip * multiple initialization (importlib.reload() is no-op) */ - md->md_state = PyMem_Malloc(md->md_size); + md->md_state = PyMem_Malloc(md->md_state_size); if (!md->md_state) { PyErr_NoMemory(); return -1; } - memset(md->md_state, 0, md->md_size); + memset(md->md_state, 0, md->md_state_size); } } return 0; @@ -739,7 +739,7 @@ PyModule_GetStateSize(PyObject *m, Py_ssize_t *size_p) return -1; } PyModuleObject *mod = (PyModuleObject *)m; - *size_p = mod->md_size; + *size_p = mod->md_state_size; return 0; } @@ -764,9 +764,9 @@ _PyModule_GetGCHooks(PyObject *m, traverseproc *traverse, return -1; } PyModuleObject *mod = (PyModuleObject *)m; - *traverse = mod->md_traverse; - *clear = mod->md_clear; - *free = mod->md_free; + *traverse = mod->md_state_traverse; + *clear = mod->md_state_clear; + *free = mod->md_state_free; return 0; } @@ -1037,9 +1037,9 @@ module_dealloc(PyObject *self) assert_def_missing_or_redundant(m); /* bpo-39824: Don't call m_free() if m_size > 0 and md_state=NULL */ - if (m->md_free && (m->md_size <= 0 || m->md_state != NULL)) + if (m->md_state_free && (m->md_state_size <= 0 || m->md_state != NULL)) { - m->md_free(m); + m->md_state_free(m); } Py_XDECREF(m->md_dict); @@ -1356,9 +1356,9 @@ module_traverse(PyObject *self, visitproc visit, void *arg) assert_def_missing_or_redundant(m); /* bpo-39824: Don't call m_traverse() if m_size > 0 and md_state=NULL */ - if (m->md_traverse && (m->md_size <= 0 || m->md_state != NULL)) + if (m->md_state_traverse && (m->md_state_size <= 0 || m->md_state != NULL)) { - int res = m->md_traverse((PyObject*)m, visit, arg); + int res = m->md_state_traverse((PyObject*)m, visit, arg); if (res) return res; } @@ -1374,9 +1374,9 @@ module_clear(PyObject *self) assert_def_missing_or_redundant(m); /* bpo-39824: Don't call m_clear() if m_size > 0 and md_state=NULL */ - if (m->md_clear && (m->md_size <= 0 || m->md_state != NULL)) + if (m->md_state_clear && (m->md_state_size <= 0 || m->md_state != NULL)) { - int res = m->md_clear((PyObject*)m); + int res = m->md_state_clear((PyObject*)m); if (PyErr_Occurred()) { PyErr_FormatUnraisable("Exception ignored in m_clear of module%s%V", m->md_name ? " " : "", From 37150a8cf4b37822ea5577a75be89b38e62aa36a Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 19 Jun 2025 14:47:36 +0200 Subject: [PATCH 47/51] Turn md_gil into a boolean md_needs_gil --- Include/internal/pycore_moduleobject.h | 3 ++- Include/moduleobject.h | 1 + Lib/test/test_sys.py | 2 +- Objects/moduleobject.c | 9 ++++++--- Python/import.c | 23 +++++++++++------------ 5 files changed, 21 insertions(+), 17 deletions(-) diff --git a/Include/internal/pycore_moduleobject.h b/Include/internal/pycore_moduleobject.h index bab4844a79e058..72a560cdf9f78d 100644 --- a/Include/internal/pycore_moduleobject.h +++ b/Include/internal/pycore_moduleobject.h @@ -1,5 +1,6 @@ #ifndef Py_INTERNAL_MODULEOBJECT_H #define Py_INTERNAL_MODULEOBJECT_H +#include #ifdef __cplusplus extern "C" { #endif @@ -29,7 +30,7 @@ typedef struct { // for logging purposes after md_dict is cleared PyObject *md_name; #ifdef Py_GIL_DISABLED - void *md_gil; + bool md_needs_gil; #endif Py_ssize_t md_state_size; traverseproc md_state_traverse; diff --git a/Include/moduleobject.h b/Include/moduleobject.h index e1d72eb8ae2613..1bd2cc8b3dd8a4 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -108,6 +108,7 @@ PyAPI_FUNC(int) _PyModule_GetGCHooks( #endif /* for Py_mod_gil: */ +/* (note reversed logic from the internal "md_needs_gil") */ #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030d0000 # define Py_MOD_GIL_USED ((void *)0) # define Py_MOD_GIL_NOT_USED ((void *)1) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index a9080ca8a1ee31..ec74189d508516 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1707,7 +1707,7 @@ def get_gen(): yield 1 check(int(PyLong_BASE**2), vsize('') + 3*self.longdigit) # module if support.Py_GIL_DISABLED: - md_gil = 'P' + md_gil = '?' else: md_gil = '' check(unittest, size('PPPPP' + md_gil + 'NPPPPP')) diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index dca61ea8c1b71e..0d0c2a5588fa2e 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -111,6 +111,9 @@ new_module_notrack(PyTypeObject *mt) m->md_state = NULL; m->md_weaklist = NULL; m->md_name = NULL; +#ifdef Py_GIL_DISABLED + m->md_needs_gil = true; +#endif m->md_state_size = 0; m->md_state_traverse = NULL; m->md_state_clear = NULL; @@ -293,7 +296,7 @@ _PyModule_CreateInitialized(PyModuleDef* module, int module_api_version) m->md_token = module; module_copy_members_from_deflike(m, module); #ifdef Py_GIL_DISABLED - m->md_gil = Py_MOD_GIL_USED; + m->md_needs_gil = true; #endif return (PyObject*)m; } @@ -501,7 +504,7 @@ module_from_def_and_spec( mod->md_def_or_null = original_def; } #ifdef Py_GIL_DISABLED - mod->md_gil = gil_slot; + mod->md_needs_gil = !gil_slot; #else (void)gil_slot; #endif @@ -595,7 +598,7 @@ PyUnstable_Module_SetGIL(PyObject *module, void *gil) PyErr_BadInternalCall(); return -1; } - ((PyModuleObject *)module)->md_gil = gil; + ((PyModuleObject *)module)->md_needs_gil = !gil; return 0; } #endif diff --git a/Python/import.c b/Python/import.c index e358d7c8510c5a..2c807179db79e4 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1031,9 +1031,9 @@ struct extensions_cache_value { _Py_ext_module_origin origin; #ifdef Py_GIL_DISABLED - /* The module's md_gil slot, for legacy modules that are reinitialized from + /* The module's md_needs_gil, for legacy modules that are reinitialized from m_dict rather than calling their initialization function again. */ - void *md_gil; + bool md_needs_gil; #endif }; @@ -1364,7 +1364,7 @@ static struct extensions_cache_value * _extensions_cache_set(PyObject *path, PyObject *name, PyModuleDef *def, PyModInitFunction m_init, Py_ssize_t m_index, PyObject *m_dict, - _Py_ext_module_origin origin, void *md_gil) + _Py_ext_module_origin origin, bool md_needs_gil) { struct extensions_cache_value *value = NULL; void *key = NULL; @@ -1419,11 +1419,11 @@ _extensions_cache_set(PyObject *path, PyObject *name, /* m_dict is set by set_cached_m_dict(). */ .origin=origin, #ifdef Py_GIL_DISABLED - .md_gil=md_gil, + .md_needs_gil=md_needs_gil, #endif }; #ifndef Py_GIL_DISABLED - (void)md_gil; + (void)md_needs_gil; #endif if (init_cached_m_dict(newvalue, m_dict) < 0) { goto finally; @@ -1560,8 +1560,7 @@ _PyImport_CheckGILForModule(PyObject* module, PyObject *module_name) return 0; } - if (!PyModule_Check(module) || - ((PyModuleObject *)module)->md_gil == Py_MOD_GIL_USED) { + if (!PyModule_Check(module) || ((PyModuleObject *)module)->md_needs_gil) { if (_PyEval_EnableGILPermanent(tstate)) { int warn_result = PyErr_WarnFormat( PyExc_RuntimeWarning, @@ -1739,7 +1738,7 @@ struct singlephase_global_update { Py_ssize_t m_index; PyObject *m_dict; _Py_ext_module_origin origin; - void *md_gil; + bool md_needs_gil; }; static struct extensions_cache_value * @@ -1798,7 +1797,7 @@ update_global_state_for_extension(PyThreadState *tstate, #endif cached = _extensions_cache_set( path, name, def, m_init, singlephase->m_index, m_dict, - singlephase->origin, singlephase->md_gil); + singlephase->origin, singlephase->md_needs_gil); if (cached == NULL) { // XXX Ignore this error? Doing so would effectively // mark the module as not loadable. @@ -1887,7 +1886,7 @@ reload_singlephase_extension(PyThreadState *tstate, if (def->m_base.m_copy != NULL) { // For non-core modules, fetch the GIL slot that was stored by // import_run_extension(). - ((PyModuleObject *)mod)->md_gil = cached->md_gil; + ((PyModuleObject *)mod)->md_needs_gil = cached->md_needs_gil; } #endif /* We can't set mod->md_def if it's missing, @@ -2142,7 +2141,7 @@ import_run_extension(PyThreadState *tstate, PyModInitFunction p0, .m_index=def->m_base.m_index, .origin=info->origin, #ifdef Py_GIL_DISABLED - .md_gil=((PyModuleObject *)mod)->md_gil, + .md_needs_gil=((PyModuleObject *)mod)->md_needs_gil, #endif }; // gh-88216: Extensions and def->m_base.m_copy can be updated @@ -2338,7 +2337,7 @@ _PyImport_FixupBuiltin(PyThreadState *tstate, PyObject *mod, const char *name, .origin=_Py_ext_module_origin_CORE, #ifdef Py_GIL_DISABLED /* Unused when m_dict == NULL. */ - .md_gil=NULL, + .md_needs_gil=true, #endif }; cached = update_global_state_for_extension( From 3e84d2f215ede0fd1451d6e80b6c8447491bffc8 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 19 Jun 2025 14:48:44 +0200 Subject: [PATCH 48/51] Share storage between def and token --- Include/internal/pycore_moduleobject.h | 17 ++++++----- Lib/test/test_sys.py | 2 +- Objects/moduleobject.c | 40 +++++++++++++++----------- 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/Include/internal/pycore_moduleobject.h b/Include/internal/pycore_moduleobject.h index 72a560cdf9f78d..a8fed2ae66ed07 100644 --- a/Include/internal/pycore_moduleobject.h +++ b/Include/internal/pycore_moduleobject.h @@ -22,13 +22,11 @@ typedef int (*_Py_modexecfunc)(PyObject *); typedef struct { PyObject_HEAD PyObject *md_dict; - // The PyModuleDef used to define the module, if any. - // (used to be `md_def`; renamed because all uses need inspection) - PyModuleDef *md_def_or_null; void *md_state; PyObject *md_weaklist; // for logging purposes after md_dict is cleared PyObject *md_name; + bool md_token_is_def; /* if true, `md_token` is the PyModuleDef */ #ifdef Py_GIL_DISABLED bool md_needs_gil; #endif @@ -40,17 +38,18 @@ typedef struct { _Py_modexecfunc md_exec; /* only set if md_def_or_null is NULL */ } PyModuleObject; -static inline PyModuleDef* _PyModule_GetDefOrNull(PyObject *mod) { - assert(PyModule_Check(mod)); - return ((PyModuleObject *)mod)->md_def_or_null; +static inline PyModuleDef* _PyModule_GetDefOrNull(PyObject *arg) { + assert(PyModule_Check(arg)); + PyModuleObject *mod = (PyModuleObject *)arg; + if (mod->md_token_is_def) { + return (PyModuleDef*)((PyModuleObject *)mod)->md_token; + } + return NULL; } static inline PyModuleDef* _PyModule_GetToken(PyObject *arg) { assert(PyModule_Check(arg)); PyModuleObject *mod = (PyModuleObject *)arg; - if (mod->md_def_or_null) { - assert(mod->md_def_or_null == mod->md_token); - } return mod->md_token; } diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index ec74189d508516..ea53dd8196d86c 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1710,7 +1710,7 @@ def get_gen(): yield 1 md_gil = '?' else: md_gil = '' - check(unittest, size('PPPPP' + md_gil + 'NPPPPP')) + check(unittest, size('PPPP?' + md_gil + 'NPPPPP')) # None check(None, size('')) # NotImplementedType diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 0d0c2a5588fa2e..32cffd91d1e711 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -28,14 +28,18 @@ static PyMemberDef module_members[] = { static void assert_def_missing_or_redundant(PyModuleObject *m) { - if (m->md_def_or_null) { -#define DO_ASSERT(F) assert (m->md_def_or_null->m_ ## F == m->md_state_ ## F); +#ifdef Py_DEBUG + if (m->md_token_is_def) { + PyModuleDef *def = (PyModuleDef *)m->md_token; + assert(def); +#define DO_ASSERT(F) assert (def->m_ ## F == m->md_state_ ## F); DO_ASSERT(size); DO_ASSERT(traverse); DO_ASSERT(clear); DO_ASSERT(free); #undef DO_ASSERT } +#endif // Py_DEBUG } @@ -58,8 +62,11 @@ _PyModule_IsExtension(PyObject *obj) if (module->md_exec) { return 1; } - PyModuleDef *def = module->md_def_or_null; - return (def != NULL && def->m_methods != NULL); + if (module->md_token_is_def) { + PyModuleDef *def = (PyModuleDef *)module->md_token; + return (module->md_token_is_def && def->m_methods != NULL); + } + return 0; } @@ -107,10 +114,10 @@ new_module_notrack(PyTypeObject *mt) m = (PyModuleObject *)_PyType_AllocNoTrack(mt, 0); if (m == NULL) return NULL; - m->md_def_or_null = NULL; m->md_state = NULL; m->md_weaklist = NULL; m->md_name = NULL; + m->md_token_is_def = false; #ifdef Py_GIL_DISABLED m->md_needs_gil = true; #endif @@ -292,8 +299,8 @@ _PyModule_CreateInitialized(PyModuleDef* module, int module_api_version) return NULL; } } - m->md_def_or_null = module; m->md_token = module; + m->md_token_is_def = true; module_copy_members_from_deflike(m, module); #ifdef Py_GIL_DISABLED m->md_needs_gil = true; @@ -501,20 +508,18 @@ module_from_def_and_spec( mod->md_state = NULL; module_copy_members_from_deflike(mod, def_like); if (original_def) { - mod->md_def_or_null = original_def; + assert (!token); + mod->md_token = original_def; + mod->md_token_is_def = 1; + } + else { + mod->md_token = token; } #ifdef Py_GIL_DISABLED mod->md_needs_gil = !gil_slot; #else (void)gil_slot; #endif - if (original_def) { - mod->md_token = original_def; - assert (!token); - } - else { - mod->md_token = token; - } mod->md_exec = m_exec; } else { if (def_like->m_size > 0 || def_like->m_traverse || def_like->m_clear @@ -658,12 +663,13 @@ PyModule_Exec(PyObject *module) } PyModuleObject *md = (PyModuleObject*)module; if (md->md_exec) { - assert(!md->md_def_or_null); + assert(!md->md_token_is_def); return run_exec_func(module, md->md_exec); } - if (md->md_def_or_null) { - return PyModule_ExecDef(module, md->md_def_or_null); + PyModuleDef *def = _PyModule_GetDefOrNull(module); + if(def) { + return PyModule_ExecDef(module, def); } return 0; } From e65451a48d8b1c306459cc374c8d2a077ea780a5 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 9 Oct 2025 15:44:51 +0200 Subject: [PATCH 49/51] Fix return value --- Modules/_testcapimodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 4f8d7743657ec2..77c582b5cc45f8 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3478,7 +3478,7 @@ _testcapi_exec(PyObject *m) return -1; } if (_PyTestCapi_Init_Module(m) < 0) { - return NULL; + return -1; } return 0; From f60bcda06fcce9f231a83f603e984665dbb9703e Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 9 Oct 2025 15:55:42 +0200 Subject: [PATCH 50/51] Lose the spec argument --- Include/internal/pycore_importdl.h | 2 +- Modules/_testmultiphase.c | 24 ++++++++---------------- Python/import.c | 2 +- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/Include/internal/pycore_importdl.h b/Include/internal/pycore_importdl.h index 75a32b607dfbbc..88367740c3f023 100644 --- a/Include/internal/pycore_importdl.h +++ b/Include/internal/pycore_importdl.h @@ -96,7 +96,7 @@ extern void _Py_ext_module_loader_result_apply_error( /* The module init function. */ typedef PyObject *(*PyModInitFunction)(void); -typedef PyModuleDef_Slot *(*PyModExportFunction)(PyObject *spec); +typedef PyModuleDef_Slot *(*PyModExportFunction)(void); #ifdef HAVE_DYNAMIC_LOADING // function changed signature, the "2" suffix helps avoid ABI issues extern int _PyImport_GetModInitFunc2( diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 166db79123becf..66aa13a2de4c66 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -1018,7 +1018,7 @@ PyInit__test_no_multiple_interpreter_slot(void) /* PyModExport_* hooks */ PyMODEXPORT_FUNC -PyModExport__test_from_modexport(PyObject *spec) +PyModExport__test_from_modexport(void) { static PyModuleDef_Slot slots[] = { {Py_mod_name, "_test_from_modexport"}, @@ -1028,21 +1028,13 @@ PyModExport__test_from_modexport(PyObject *spec) } PyMODEXPORT_FUNC -PyModExport__test_from_modexport_null(PyObject *spec) +PyModExport__test_from_modexport_null(void) { - PyObject *exc; - if (PyObject_GetOptionalAttrString(spec, "_test_exception", &exc) < 0) { - return NULL; - } - if (exc) { - PyErr_SetObject((PyObject*)Py_TYPE(exc), exc); - } - Py_XDECREF(exc); return NULL; } PyMODINIT_FUNC -PyModInit__test_from_modexport_null(PyObject *spec) +PyModInit__test_from_modexport_null(void) { // This is not called as fallback for failed PyModExport_* assert(0); @@ -1051,14 +1043,14 @@ PyModInit__test_from_modexport_null(PyObject *spec) } PyMODEXPORT_FUNC -PyModExport__test_from_modexport_exception(PyObject *spec) +PyModExport__test_from_modexport_exception(void) { PyErr_SetString(PyExc_ValueError, "failed as requested"); return NULL; } PyMODINIT_FUNC -PyModInit__test_from_modexport_exception(PyObject *spec) +PyModInit__test_from_modexport_exception(void) { // This is not called as fallback for failed PyModExport_* assert(0); @@ -1074,7 +1066,7 @@ modexport_create_string(PyObject *spec, PyObject *def) } PyMODEXPORT_FUNC -PyModExport__test_from_modexport_create_nonmodule(PyObject *spec) +PyModExport__test_from_modexport_create_nonmodule(void) { static PyModuleDef_Slot slots[] = { {Py_mod_name, "_test_from_modexport_create_nonmodule"}, @@ -1089,7 +1081,7 @@ static PyModuleDef_Slot modexport_empty_slots[] = { }; PyMODEXPORT_FUNC -PyModExport__test_from_modexport_empty_slots(PyObject *spec) +PyModExport__test_from_modexport_empty_slots(void) { return modexport_empty_slots; } @@ -1154,7 +1146,7 @@ modexport_smoke_free(PyObject *mod) } PyMODEXPORT_FUNC -PyModExport__test_from_modexport_smoke(PyObject *spec) +PyModExport__test_from_modexport_smoke(void) { static PyMethodDef methods[] = { {"get_state_int", modexport_smoke_get_state_int, METH_NOARGS}, diff --git a/Python/import.c b/Python/import.c index b3a3389d54b5ab..860fd19dd1c9d0 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1990,7 +1990,7 @@ import_run_modexport(PyThreadState *tstate, PyModExportFunction ex0, /* This is like import_run_extension, but avoids interpreter switching * and code for for single-phase modules. */ - PyModuleDef_Slot *slots = ex0(spec); + PyModuleDef_Slot *slots = ex0(); if (!slots) { if (!PyErr_Occurred()) { PyErr_Format( From 68ecaf8a3507794d0c93eed627f8f72054ec704f Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 9 Oct 2025 16:00:38 +0200 Subject: [PATCH 51/51] Put `const` in signatures --- Include/moduleobject.h | 2 +- Include/object.h | 2 +- Objects/moduleobject.c | 4 ++-- Objects/typeobject.c | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Include/moduleobject.h b/Include/moduleobject.h index 3ffc638f78c2d4..cb1250d08f0f8a 100644 --- a/Include/moduleobject.h +++ b/Include/moduleobject.h @@ -124,7 +124,7 @@ PyAPI_FUNC(int) PyUnstable_Module_SetGIL(PyObject *module, void *gil); #endif #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= _Py_PACK_VERSION(3, 15) -PyAPI_FUNC(PyObject *) PyModule_FromSlotsAndSpec(PyModuleDef_Slot *, +PyAPI_FUNC(PyObject *) PyModule_FromSlotsAndSpec(const PyModuleDef_Slot *, PyObject *spec); PyAPI_FUNC(int) PyModule_Exec(PyObject *mod); PyAPI_FUNC(int) PyModule_GetStateSize(PyObject *mod, Py_ssize_t *size_p); diff --git a/Include/object.h b/Include/object.h index 12a978229a2846..1b04ebaac3fc0f 100644 --- a/Include/object.h +++ b/Include/object.h @@ -835,7 +835,7 @@ PyAPI_FUNC(int) PyType_Freeze(PyTypeObject *type); #endif #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= _Py_PACK_VERSION(3, 15) -PyAPI_FUNC(PyObject *) PyType_GetModuleByToken(PyTypeObject *, void *); +PyAPI_FUNC(PyObject *) PyType_GetModuleByToken(PyTypeObject *, const void *); #endif #ifdef __cplusplus diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index c919d40cc09039..651191891bac35 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -639,7 +639,7 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio } PyObject * -PyModule_FromSlotsAndSpec(PyModuleDef_Slot *slots, PyObject *spec) +PyModule_FromSlotsAndSpec(const PyModuleDef_Slot *slots, PyObject *spec) { if (!slots) { PyErr_Format( @@ -648,7 +648,7 @@ PyModule_FromSlotsAndSpec(PyModuleDef_Slot *slots, PyObject *spec) return NULL; } // Fill in enough of a PyModuleDef to pass to common machinery - PyModuleDef def_like = {.m_slots = slots}; + PyModuleDef def_like = {.m_slots = (PyModuleDef_Slot *)slots}; return module_from_def_and_spec(&def_like, spec, PYTHON_API_VERSION, NULL); diff --git a/Objects/typeobject.c b/Objects/typeobject.c index bc21fe5e8ff1f6..dc20846264271b 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5763,7 +5763,7 @@ PyType_GetModuleState(PyTypeObject *type) * has the given token. */ PyObject * -borrow_module_by_token(PyTypeObject *type, void *token) +borrow_module_by_token(PyTypeObject *type, const void *token) { assert(PyType_Check(type)); @@ -5828,7 +5828,7 @@ PyType_GetModuleByDef(PyTypeObject *type, PyModuleDef *def) } PyObject * -PyType_GetModuleByToken(PyTypeObject *type, void *token) +PyType_GetModuleByToken(PyTypeObject *type, const void *token) { return Py_XNewRef(borrow_module_by_token(type, token)); }