Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

bpo-33331: Clean modules in the reversed order in PyImport_Cleanup(). #6565

Merged
merged 7 commits into from Oct 29, 2018
118 changes: 62 additions & 56 deletions Lib/importlib/_bootstrap.py
Expand Up @@ -302,33 +302,6 @@ def _module_repr(module):
return '<module {!r} from {!r}>'.format(name, filename)


class _installed_safely:

def __init__(self, module):
self._module = module
self._spec = module.__spec__

def __enter__(self):
# This must be done before putting the module in sys.modules
# (otherwise an optimization shortcut in import.c becomes
# wrong)
self._spec._initializing = True
sys.modules[self._spec.name] = self._module

def __exit__(self, *args):
try:
spec = self._spec
if any(arg is not None for arg in args):
try:
del sys.modules[spec.name]
except KeyError:
pass
else:
_verbose_message('import {!r} # {!r}', spec.name, spec.loader)
finally:
self._spec._initializing = False


class ModuleSpec:
"""The specification for a module, used for loading.

Expand Down Expand Up @@ -614,30 +587,44 @@ def _exec(spec, module):
if sys.modules.get(name) is not module:
msg = 'module {!r} not in sys.modules'.format(name)
raise ImportError(msg, name=name)
if spec.loader is None:
if spec.submodule_search_locations is None:
raise ImportError('missing loader', name=spec.name)
# namespace package
_init_module_attrs(spec, module, override=True)
return module
_init_module_attrs(spec, module, override=True)
if not hasattr(spec.loader, 'exec_module'):
# (issue19713) Once BuiltinImporter and ExtensionFileLoader
# have exec_module() implemented, we can add a deprecation
# warning here.
spec.loader.load_module(name)
else:
spec.loader.exec_module(module)
return sys.modules[name]
try:
if spec.loader is None:
if spec.submodule_search_locations is None:
raise ImportError('missing loader', name=spec.name)
# Namespace package.
_init_module_attrs(spec, module, override=True)
else:
_init_module_attrs(spec, module, override=True)
if not hasattr(spec.loader, 'exec_module'):
# (issue19713) Once BuiltinImporter and ExtensionFileLoader
# have exec_module() implemented, we can add a deprecation
# warning here.
spec.loader.load_module(name)
else:
spec.loader.exec_module(module)
finally:
# Update the order of insertion into sys.modules for module
# clean-up at shutdown.
module = sys.modules.pop(spec.name)
serhiy-storchaka marked this conversation as resolved.
Show resolved Hide resolved
sys.modules[spec.name] = module
return module


def _load_backward_compatible(spec):
# (issue19713) Once BuiltinImporter and ExtensionFileLoader
# have exec_module() implemented, we can add a deprecation
# warning here.
spec.loader.load_module(spec.name)
try:
spec.loader.load_module(spec.name)
except:
if spec.name in sys.modules:
module = sys.modules.pop(spec.name)
brettcannon marked this conversation as resolved.
Show resolved Hide resolved
sys.modules[spec.name] = module
raise
# The module must be in sys.modules at this point!
module = sys.modules[spec.name]
# Move it to the end of sys.modules.
module = sys.modules.pop(spec.name)
sys.modules[spec.name] = module
if getattr(module, '__loader__', None) is None:
try:
module.__loader__ = spec.loader
Expand All @@ -663,23 +650,42 @@ def _load_backward_compatible(spec):
def _load_unlocked(spec):
# A helper for direct use by the import system.
if spec.loader is not None:
# not a namespace package
# Not a namespace package.
if not hasattr(spec.loader, 'exec_module'):
return _load_backward_compatible(spec)

module = module_from_spec(spec)
with _installed_safely(module):
if spec.loader is None:
if spec.submodule_search_locations is None:
raise ImportError('missing loader', name=spec.name)
# A namespace package so do nothing.
else:
spec.loader.exec_module(module)

# We don't ensure that the import-related module attributes get
# set in the sys.modules replacement case. Such modules are on
# their own.
return sys.modules[spec.name]
# This must be done before putting the module in sys.modules
# (otherwise an optimization shortcut in import.c becomes
# wrong).
spec._initializing = True
try:
sys.modules[spec.name] = module
try:
if spec.loader is None:
if spec.submodule_search_locations is None:
raise ImportError('missing loader', name=spec.name)
# A namespace package so do nothing.
else:
spec.loader.exec_module(module)
except:
try:
del sys.modules[spec.name]
except KeyError:
pass
raise
# Move the module to the end of sys.modules.
# We don't ensure that the import-related module attributes get
# set in the sys.modules replacement case. Such modules are on
# their own.
module = sys.modules.pop(spec.name)
sys.modules[spec.name] = module
_verbose_message('import {!r} # {!r}', spec.name, spec.loader)
finally:
spec._initializing = False

return module

# A method used during testing of _load_unlocked() and by
# _load_module_shim().
Expand Down
@@ -0,0 +1 @@
Modules imported last are now cleared first at interpreter shutdown.
7 changes: 4 additions & 3 deletions Python/import.c
Expand Up @@ -543,9 +543,10 @@ PyImport_Cleanup(void)
module last. Likewise, we don't delete sys until the very
end because it is implicitly referenced (e.g. by print). */
if (weaklist != NULL) {
Py_ssize_t i, n;
n = PyList_GET_SIZE(weaklist);
for (i = 0; i < n; i++) {
Py_ssize_t i;
/* Since dict is ordered in CPython 3.6+, modules are saved in
importing order. First clear modules imported later. */
for (i = PyList_GET_SIZE(weaklist) - 1; i >= 0; i--) {
PyObject *tup = PyList_GET_ITEM(weaklist, i);
PyObject *name = PyTuple_GET_ITEM(tup, 0);
PyObject *mod = PyWeakref_GET_OBJECT(PyTuple_GET_ITEM(tup, 1));
Expand Down