-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
Description
Commit 096d009 (for gh-101758) resulted in refleak failures. It's unclear if the leak is new or if the test simply exposed it. In 984f8ab, I skipped the leaking tests until we can fix the leak.
UPDATE: I've updates the tests to only skip when checking for refleaks. I've also added a few more tests.
Context
(expand)
(Also see gh-101758.)
Loading an extension module involves meaningfully different code paths depending on the content of the PyModuleDef. There's the difference between legacy (single-phase-init, PEP 3121) and modern (multi-phase-init, PEP 489) modules. For single-phase init, there are those that support being loaded more than once (m_size >= 0) and those that can't (m_size == -1). I've added several long comments in import.c explaining about single-phase init modules in detail. I also added some tests to verify the behavior of the single-phase-init cases more thoroughly. Those are the leaking tests.
Relevant state:
PyModuleDef.m_size- the size of the module's per-interpreter state (PyModuleObject.md_state)- single-phase init and multi-phase init modules can have a value of 0 or greater
- such modules must not have any process-global state
- only single-phase init modules can have a value of -1, which means the module does not support being loaded more than once
- such modules may have process-global state
PyModuleDef.m_base.m_index- the index intoPyInterpreterState.imports.modules_by_index(same index used for every interpreter)- set by
PyModuleDef_Init()(e.g. when the module is created)
- set by
PyModuleDef.m_base.m_copy- a copy of the__dict__of the last time a module was loaded using this def (only single-phase init wherem_sizeis -1)- set exclusively by
fix_up_extension() - used exclusively by
import_find_extension() - cleared by (replaced in)
fix_up_extension()ifm_copywas already set (e.g. multiple interpreters, multiple modules in same file using same def) - cleared by
_PyImport_ClearModulesByIndex()during interpreter finalization
- set exclusively by
_PyRuntime.imports.extensions- a dict mapping(filename, name)toPyModuleDef- entry set exclusively by
fix_up_extension()(only for single-phase init modules) - entry used exclusively by
import_find_extension() - entry cleared by
_PyImport_Fini()at runtime finalization
- entry set exclusively by
interp->imports.modules_by_index- a list of single-phase-init modules loaded in this interpreter; lookups (e.g.PyState_FindModule()) usePyModuleDef.m_base.m_index- entry set normally only by
fix_up_extension()andimport_find_extension()(only single-phase init modules) - (entry also set by
PyState_AddModule()) - used exclusively by
PyState_FindModule() - entry cleared normally by
_PyImport_ClearModulesByIndex()during interpreter finalization - (entry also cleared by
PyState_RemoveModule())
- entry set normally only by
Code path when loading a single-phase-init module for the first time (in import.c, unless otherwise noted):
imp.load_dynamic()importlib._boostrap._load()(using a spec withExtensionFileLoader)ExtensionFileLoader.create_module()(in _boostrap_external.py)_imp.create_dynamic()(_imp_create_dynamic_impl())import_find_extension()(not found in_PyRuntime.imports.extensions)_PyImport_LoadDynamicModuleWithSpec()(importdl.c)_PyImport_FindSharedFuncptr()<module init func from loaded binary>()- sets process-global state (only where
m_size == -1) PyModule_Create()- populates per-interpreter module state (only where
m_size > 0) - sets module attrs (on
__dict__)
- sets process-global state (only where
- sets
def->m_base.m_init(only needed for single-phase-init wherem_size >=0) _PyImport_FixupExtensionObject()- sets
sys.modules[spec.name] fix_up_extension()- set
interp->imports.modules_by_index[def->m_base.m_index] - clear
def->m_base.m_copy(only if set and only ifm_size == -1) - set
def->m_base.m_copyto a copy of the module's__dict__(only ifm_size == -1) - set
_PyRuntime.imports.extensions[(filename, name)]
- set
- sets
- sets missing module attrs (e.g.
__file__)
During testing we use a helper to erase (nearly) any evidence of the module having been imported before. That means clearing the state described above.
Here's the code path:
_testinternalcapi.clear_extension()(Modules/_testinternalcapi.c)_PyImport_ClearExtension()clear_singlephase_extension()- (only if there's a
_PyRuntime.imports.extensionsentry)- clear the module def's
m_copy - replace the
interp->imports.modules_by_indexentry withPy_None - delete the
_PyRuntime.imports.extensionsentry
- clear the module def's
- (only if there's a
Linked PRs
- gh-102251: Updates to test_imp Toward Fixing Some Refleaks #102254
- gh-102251: add missing cleanups for
test_import#104796 - gh-102251: Fix reference leak in _testsinglephase initialization #105082
- [3.12] gh-102251: Fix reference leak in _testsinglephase initialization (GH-105082) #105083
- gh-102251: Explicitly free state for test modules with state in test_import #105085
- [3.12] gh-102251: Explicitly free state for test modules with state in test_import (GH-105085) #105170
- gh-102251: Disable non-rerunnable test in test_import #106013
- [3.12] gh-102251: Disable non-rerunnable test in test_import (GH-106013) #109540