Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b6098ef

Browse filesBrowse files
ericsnowcurrentlyericsnowcurrently
committed
Drop m_init and m_copy from PyModuleDef_Base.
1 parent efa17fc commit b6098ef
Copy full SHA for b6098ef

File tree

Expand file treeCollapse file tree

2 files changed

+49
-55
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+49
-55
lines changed

‎Include/moduleobject.h

Copy file name to clipboardExpand all lines: Include/moduleobject.h
+6-15Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,30 +38,21 @@ PyAPI_DATA(PyTypeObject) PyModuleDef_Type;
3838

3939
typedef struct PyModuleDef_Base {
4040
PyObject_HEAD
41-
/* The function used to re-initialize the module.
42-
This is only set for legacy (single-phase init) extension modules
43-
and only used for those that support multiple initializations
44-
(m_size >= 0).
45-
It is set by _PyImport_LoadDynamicModuleWithSpec()
46-
and _imp.create_builtin(). */
47-
PyObject* (*m_init)(void);
41+
/* We keep the following fields, at the given size, for API compatibility. */
42+
Py_uintptr_t m_cache_has_m_init;
43+
Py_uintptr_t m_cache_has_m_dict;
4844
/* The module's index into its interpreter's modules_by_index cache.
4945
This is set for all extension modules but only used for legacy ones.
5046
(See PyInterpreterState.modules_by_index for more info.)
5147
It is set by PyModuleDef_Init(). */
5248
Py_ssize_t m_index;
53-
/* A copy of the module's __dict__ after the first time it was loaded.
54-
This is only set/used for legacy modules that do not support
55-
multiple initializations.
56-
It is set by fix_up_extension() in import.c. */
57-
PyObject* m_copy;
5849
} PyModuleDef_Base;
5950

6051
#define PyModuleDef_HEAD_INIT { \
6152
PyObject_HEAD_INIT(_Py_NULL) \
62-
_Py_NULL, /* m_init */ \
63-
0, /* m_index */ \
64-
_Py_NULL, /* m_copy */ \
53+
0, /* m_cache_has_m_init */ \
54+
0, /* m_cache_has_m_dict */ \
55+
0, /* m_index */ \
6556
}
6657

6758
#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03050000

‎Python/import.c

Copy file name to clipboardExpand all lines: Python/import.c
+43-40Lines changed: 43 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -580,19 +580,8 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp)
580580
return;
581581
}
582582

583-
Py_ssize_t i;
584-
for (i = 0; i < PyList_GET_SIZE(MODULES_BY_INDEX(interp)); i++) {
585-
PyObject *m = PyList_GET_ITEM(MODULES_BY_INDEX(interp), i);
586-
if (PyModule_Check(m)) {
587-
/* cleanup the saved copy of module dicts */
588-
PyModuleDef *md = PyModule_GetDef(m);
589-
if (md) {
590-
// XXX Do this more carefully. The dict might be owned
591-
// by another interpreter.
592-
Py_CLEAR(md->m_base.m_copy);
593-
}
594-
}
595-
}
583+
/* We clear out the cached module defs and copied module dicts
584+
* in _PyImport_Fini(). */
596585

597586
/* Setting modules_by_index to NULL could be dangerous, so we
598587
clear the list instead. */
@@ -652,7 +641,7 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp)
652641
O. import_run_extension(): set __file__
653642
P. import_run_extension() -> update_global_state_for_extension()
654643
Q. update_global_state_for_extension():
655-
copy __dict__ into def->m_base.m_copy
644+
copy __dict__ into the cached m_dict
656645
R. update_global_state_for_extension():
657646
add it to _PyRuntime.imports.extensions
658647
S. import_run_extension() -> finish_singlephase_extension()
@@ -672,7 +661,7 @@ _PyImport_ClearModulesByIndex(PyInterpreterState *interp)
672661
F. else:
673662
1. import_add_module() -> PyModule_NewObject()
674663
2. import_add_module(): set it on sys.modules
675-
G. reload_singlephase_extension(): copy the "m_copy" dict into __dict__
664+
G. reload_singlephase_extension(): copy the "m_dict" dict into __dict__
676665
H. reload_singlephase_extension(): add to modules_by_index
677666
678667
(10). (every time):
@@ -854,19 +843,19 @@ We support a number of kinds of single-phase init builtin/extension modules:
854843
855844
* "basic"
856845
* no module state (PyModuleDef.m_size == -1)
857-
* does not support repeated init (we use PyModuleDef.m_base.m_copy)
846+
* does not support repeated init (we use the cached m_dict)
858847
* may have process-global state
859848
* the module's def is cached in _PyRuntime.imports.extensions,
860849
by (name, filename)
861850
* "reinit"
862851
* no module state (PyModuleDef.m_size == 0)
863-
* supports repeated init (m_copy is never used)
852+
* supports repeated init (m_dict is never used)
864853
* should not have any process-global state
865854
* its def is never cached in _PyRuntime.imports.extensions
866855
(except, currently, under the main interpreter, for some reason)
867856
* "with state" (almost the same as reinit)
868857
* has module state (PyModuleDef.m_size > 0)
869-
* supports repeated init (m_copy is never used)
858+
* supports repeated init (m_dict is never used)
870859
* should not have any process-global state
871860
* its def is never cached in _PyRuntime.imports.extensions
872861
(except, currently, under the main interpreter, for some reason)
@@ -892,7 +881,7 @@ notable weirdness happens:
892881
* (non-basic-only) its init func is used when re-loading any of them
893882
(via the def's m_init)
894883
* (basic-only) the copy of its __dict__ is used when re-loading any of them
895-
(via the def's m_copy)
884+
(via the def's cached m_dict)
896885
897886
However, the following happens as expected:
898887
@@ -905,7 +894,7 @@ However, the following happens as expected:
905894
For "basic" modules there are other quirks:
906895
907896
* (whether sharing a def or not) when loaded the first time,
908-
m_copy is set before _init_module_attrs() is called
897+
m_dict is set before _init_module_attrs() is called
909898
in importlib._bootstrap.module_from_spec(),
910899
so when the module is re-loaded, the previous value
911900
for __wpec__ (and others) is reset, possibly unexpectedly.
@@ -948,7 +937,7 @@ typedef PyDictObject *cached_m_dict_t;
948937
struct extensions_cache_value {
949938
PyModuleDef *def;
950939

951-
/* m_init and m_copy were formerly part of PyModuleDef_Base. */
940+
/* m_init and m_dict were formerly part of PyModuleDef_Base. */
952941

953942
/* The function used to re-initialize the module.
954943
This is only set for legacy (single-phase init) extension modules
@@ -1008,7 +997,7 @@ set_cached_m_dict(struct extensions_cache_value *value, PyObject *m_dict)
1008997
* isolated interpreter, but there are ways around that.
1009998
* Hence, heere be dragons! Ideally we would instead do
1010999
* something like make a read-only, immortal copy of the
1011-
* dict using PyMem_RawMalloc() and store *that* in m_copy.
1000+
* dict using PyMem_RawMalloc() and store *that* in m_dict.
10121001
* Then we'd need to make sure to clear that when the
10131002
* runtime is finalized, rather than in
10141003
* PyImport_ClearModulesByIndex(). */
@@ -1051,7 +1040,7 @@ update_extensions_cache_value(struct extensions_cache_value *value,
10511040
/* We expect the same symbol to be used and the shared object file
10521041
* to have remained loaded, so it must be the same pointer. */
10531042
assert(value->m_init == NULL || value->m_init == m_init);
1054-
/* For now we don't worry about comparing value->m_copy. */
1043+
/* For now we don't worry about comparing value->m_dict. */
10551044
assert(value->m_dict == NULL || m_dict != NULL);
10561045

10571046
/* We assume that all module defs are statically allocated
@@ -1067,6 +1056,13 @@ update_extensions_cache_value(struct extensions_cache_value *value,
10671056
return -1;
10681057
}
10691058

1059+
if (m_init != NULL) {
1060+
def->m_base.m_cache_has_m_init = 1;
1061+
}
1062+
if (temp.m_dict != NULL) {
1063+
def->m_base.m_cache_has_m_dict = 1;
1064+
}
1065+
10701066
*value = temp;
10711067
return 0;
10721068
}
@@ -1396,7 +1392,7 @@ _get_extension_kind(PyModuleDef *def, bool check_size)
13961392
enum _Py_ext_module_kind kind;
13971393
if (def == NULL) {
13981394
/* It must be a module created by reload_singlephase_extension()
1399-
* from m_copy. Ideally we'd do away with this case. */
1395+
* from m_dict. Ideally we'd do away with this case. */
14001396
kind = _Py_ext_module_kind_SINGLEPHASE;
14011397
}
14021398
else if (def->m_slots != NULL) {
@@ -1405,7 +1401,10 @@ _get_extension_kind(PyModuleDef *def, bool check_size)
14051401
else if (check_size && def->m_size == -1) {
14061402
kind = _Py_ext_module_kind_SINGLEPHASE;
14071403
}
1408-
else if (def->m_base.m_init != NULL) {
1404+
else if (def->m_base.m_cache_has_m_init) {
1405+
kind = _Py_ext_module_kind_SINGLEPHASE;
1406+
}
1407+
else if (def->m_base.m_cache_has_m_dict) {
14091408
kind = _Py_ext_module_kind_SINGLEPHASE;
14101409
}
14111410
else {
@@ -1451,8 +1450,9 @@ update_global_state_for_extension(PyThreadState *tstate,
14511450
PyModInitFunction m_init = NULL;
14521451
PyObject *m_dict = NULL;
14531452

1454-
assert(def->m_base.m_init == NULL);
1455-
assert(def->m_base.m_copy == NULL);
1453+
/* It shouldn't have been set yet. */
1454+
assert(!def->m_base.m_cache_has_m_init);
1455+
assert(!def->m_base.m_cache_has_m_dict);
14561456

14571457
/* Set up for _extensions_cache_set(). */
14581458
if (singlephase != NULL) {
@@ -1473,7 +1473,7 @@ update_global_state_for_extension(PyThreadState *tstate,
14731473
else {
14741474
assert(singlephase->m_dict != NULL
14751475
&& PyDict_Check(singlephase->m_dict));
1476-
// gh-88216: Extensions and def->m_base.m_copy can be updated
1476+
// gh-88216: Extensions and the cached m_dict can be updated
14771477
// when the extension module doesn't support sub-interpreters.
14781478
assert(def->m_size == -1);
14791479
assert(!is_core_module(tstate->interp, name, path));
@@ -1496,6 +1496,8 @@ update_global_state_for_extension(PyThreadState *tstate,
14961496
// mark the module as not loadable.
14971497
return -1;
14981498
}
1499+
assert(def->m_base.m_cache_has_m_init
1500+
|| def->m_base.m_cache_has_m_dict);
14991501
}
15001502

15011503
return 0;
@@ -1547,14 +1549,15 @@ reload_singlephase_extension(PyThreadState *tstate,
15471549
if (def->m_size == -1) {
15481550
/* Module does not support repeated initialization */
15491551
assert(cached->m_init == NULL);
1550-
assert(def->m_base.m_init == NULL);
1552+
assert(!def->m_base.m_cache_has_m_init);
1553+
assert(def->m_base.m_cache_has_m_dict);
15511554
// XXX Copying the cached dict may break interpreter isolation.
15521555
// We could solve this by temporarily acquiring the original
15531556
// interpreter's GIL.
15541557
PyObject *m_copy = get_cached_m_dict(cached);
15551558
if (m_copy == NULL) {
15561559
/* It might be a core module (e.g. sys & builtins),
1557-
for which we don't set m_copy. */
1560+
for which we don't cache m_dict. */
15581561
m_copy = get_core_module_dict(
15591562
tstate->interp, info->name, info->path);
15601563
if (m_copy == NULL) {
@@ -1589,7 +1592,8 @@ reload_singlephase_extension(PyThreadState *tstate,
15891592
}
15901593
else {
15911594
assert(cached->m_dict == NULL);
1592-
assert(def->m_base.m_copy == NULL);
1595+
assert(def->m_base.m_cache_has_m_init);
1596+
assert(!def->m_base.m_cache_has_m_dict);
15931597
PyModInitFunction p0 = cached->m_init;
15941598
if (p0 == NULL) {
15951599
assert(!PyErr_Occurred());
@@ -1714,18 +1718,16 @@ import_run_extension(PyThreadState *tstate, PyModInitFunction p0,
17141718

17151719
/* Update global import state. */
17161720
struct singlephase_global_update singlephase = {0};
1717-
// gh-88216: Extensions and def->m_base.m_copy can be updated
1721+
// gh-88216: Extensions and def->m_base.m_dict can be updated
17181722
// when the extension module doesn't support sub-interpreters.
17191723
if (def->m_size == -1) {
1720-
/* We will reload from m_copy. */
1721-
assert(def->m_base.m_init == NULL);
1724+
/* We will reload from m_dict. */
17221725
singlephase.m_dict = PyModule_GetDict(mod);
17231726
assert(singlephase.m_dict != NULL);
17241727
}
17251728
else {
17261729
/* We will reload via the init function. */
17271730
assert(def->m_size >= 0);
1728-
assert(def->m_base.m_copy == NULL);
17291731
singlephase.m_init = p0;
17301732
}
17311733
if (update_global_state_for_extension(
@@ -1765,8 +1767,8 @@ clear_singlephase_extension(PyInterpreterState *interp,
17651767
PyModuleDef *def = value->def;
17661768

17671769
/* Clear data set when the module was initially loaded. */
1768-
def->m_base.m_init = NULL;
1769-
Py_CLEAR(def->m_base.m_copy);
1770+
def->m_base.m_cache_has_m_init = 0;
1771+
def->m_base.m_cache_has_m_dict = 0;
17701772
// We leave m_index alone since there's no reason to reset it.
17711773

17721774
/* Clear the PyState_*Module() cache entry. */
@@ -1808,22 +1810,23 @@ _PyImport_FixupBuiltin(PyThreadState *tstate, PyObject *mod, const char *name,
18081810

18091811
/* We only use _PyImport_FixupBuiltin() for the core builtin modules
18101812
* (sys and builtins). These modules are single-phase init with no
1811-
* module state, but we also don't populate def->m_base.m_copy
1813+
* module state, but we also don't populate the cached m_dict
18121814
* for them. */
18131815
assert(is_core_module(tstate->interp, nameobj, nameobj));
18141816
assert(check_singlephase(def));
18151817
assert(def->m_size == -1);
1816-
assert(def->m_base.m_copy == NULL);
18171818

18181819
struct singlephase_global_update singlephase = {
1819-
/* We don't want def->m_base.m_copy populated. */
1820+
/* We don't want the cached m_dict populated. */
18201821
.m_dict=NULL,
18211822
};
18221823
if (update_global_state_for_extension(
18231824
tstate, nameobj, nameobj, def, &singlephase) < 0)
18241825
{
18251826
goto finally;
18261827
}
1828+
assert(!def->m_base.m_cache_has_m_init);
1829+
assert(def->m_base.m_cache_has_m_dict);
18271830

18281831
if (finish_singlephase_extension(tstate, mod, def, nameobj, modules) < 0) {
18291832
goto finally;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.