Skip to content

Commit 0afefeb

Browse files
committed
gh-92041: Preserve getmodule filename fallback
Restore filename-based module resolution when direct frame-globals lookup cannot prove ownership. Avoid repeated full sys.modules scans while its module names are unchanged.
1 parent 3fbfadc commit 0afefeb

5 files changed

Lines changed: 238 additions & 58 deletions

File tree

Lib/inspect.py

Lines changed: 57 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -925,6 +925,34 @@ def getabsfile(object, _filename=None):
925925

926926
modulesbyfile = {}
927927
_filesbymodname = {}
928+
_modulesbyfile_snapshot = None
929+
930+
931+
def _getframemodule(frame, _filename=None):
932+
"""Return a module owned by a frame's globals, or None if not found."""
933+
frame_globals = frame.f_globals
934+
module_name = frame_globals.get('__name__')
935+
if not isinstance(module_name, str):
936+
return None
937+
module = sys.modules.get(module_name)
938+
if not (ismodule(module) and module.__dict__ is frame_globals):
939+
return None
940+
module_file = getattr(module, '__file__', None)
941+
if module_file is None:
942+
return None
943+
try:
944+
file = getabsfile(frame, _filename)
945+
except (TypeError, FileNotFoundError):
946+
return None
947+
if frame.f_code.co_filename == module_file:
948+
return module
949+
try:
950+
module_file = getabsfile(module)
951+
except (TypeError, FileNotFoundError):
952+
return None
953+
if file == module_file or file == os.path.realpath(module_file):
954+
return module
955+
928956

929957
def getmodule(object, _filename=None):
930958
"""Return the module an object was defined in, or None if not found."""
@@ -938,29 +966,9 @@ def getmodule(object, _filename=None):
938966
# Frame globals identify the execution namespace directly. Preserve
939967
# the private filename override when it names a different file.
940968
if _filename is None or _filename == object.f_code.co_filename:
941-
object_globals = object.f_globals
942-
module_name = object_globals.get('__name__')
943-
if not isinstance(module_name, str):
944-
return None
945-
module = sys.modules.get(module_name)
946-
if not (ismodule(module) and module.__dict__ is object_globals):
947-
return None
948-
module_file = getattr(module, '__file__', None)
949-
if module_file is None:
950-
return None
951-
try:
952-
file = getabsfile(object, _filename)
953-
except (TypeError, FileNotFoundError):
954-
return None
955-
if object.f_code.co_filename == module_file:
956-
return module
957-
try:
958-
module_file = getabsfile(module)
959-
except (TypeError, FileNotFoundError):
960-
return None
961-
if file == module_file or file == os.path.realpath(module_file):
969+
module = _getframemodule(object, _filename)
970+
if module is not None:
962971
return module
963-
return None
964972

965973
# Try the filename to modulename cache
966974
if _filename is not None and _filename in modulesbyfile:
@@ -972,19 +980,33 @@ def getmodule(object, _filename=None):
972980
return None
973981
if file in modulesbyfile:
974982
return sys.modules.get(modulesbyfile[file])
975-
# Update the filename to module name cache and check yet again
976-
# Copy sys.modules in order to cope with changes while iterating
977-
for modname, module in sys.modules.copy().items():
978-
if ismodule(module) and hasattr(module, '__file__'):
979-
f = module.__file__
980-
if f == _filesbymodname.get(modname, None):
981-
# Have already mapped this module, so skip it
982-
continue
983-
_filesbymodname[modname] = f
984-
f = getabsfile(module)
985-
# Always map to the name the module knows itself by
986-
modulesbyfile[f] = modulesbyfile[
987-
os.path.realpath(f)] = module.__name__
983+
# Update the filename to module name cache only when sys.modules is
984+
# replaced or its module names have changed since the previous scan.
985+
# Retaining module values in the snapshot would keep removed modules alive.
986+
global _modulesbyfile_snapshot
987+
modules = sys.modules
988+
modules_id = id(modules)
989+
try:
990+
snapshot = (modules_id, tuple(modules))
991+
except RuntimeError:
992+
# The mapping changed while its keys were being copied.
993+
snapshot = None
994+
if snapshot is None or snapshot != _modulesbyfile_snapshot:
995+
# Copy sys.modules in order to cope with changes while iterating.
996+
modules = modules.copy()
997+
snapshot = (modules_id, tuple(modules))
998+
for modname, module in modules.items():
999+
if ismodule(module) and hasattr(module, '__file__'):
1000+
f = module.__file__
1001+
if f == _filesbymodname.get(modname, None):
1002+
# Have already mapped this module, so skip it
1003+
continue
1004+
_filesbymodname[modname] = f
1005+
f = getabsfile(module)
1006+
# Always map to the name the module knows itself by
1007+
modulesbyfile[f] = modulesbyfile[
1008+
os.path.realpath(f)] = module.__name__
1009+
_modulesbyfile_snapshot = snapshot
9881010
if file in modulesbyfile:
9891011
return sys.modules.get(modulesbyfile[file])
9901012
# Check the main module

Lib/test/libregrtest/utils.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ def clear_caches():
296296
inspect._shadowed_dict_from_weakref_mro_tuple.cache_clear()
297297
inspect._filesbymodname.clear()
298298
inspect.modulesbyfile.clear()
299+
inspect._modulesbyfile_snapshot = None
299300

300301
try:
301302
importlib_metadata = sys.modules['importlib.metadata']

Lib/test/test_inspect/test_inspect.py

Lines changed: 148 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -844,32 +844,142 @@ def test_getmodule(self):
844844
sys.modules[__name__])
845845

846846
def test_getmodule_unregistered_exec_frame(self):
847-
def exec_namespace(namespace):
847+
def exec_namespace(namespace, filename, expected):
848848
exec(compile(textwrap.dedent("""
849849
frame = inspect.currentframe()
850850
try:
851851
1 / 0
852852
except ZeroDivisionError as error:
853853
traceback = error.__traceback__
854-
"""), modfile, "exec"), namespace)
855-
self.assertIsNone(inspect.getmodule(namespace["frame"]))
856-
self.assertIsNone(inspect.getmodule(namespace["traceback"]))
857-
858-
# Missing and invalid module names identify no registered namespace.
859-
exec_namespace({"inspect": inspect})
860-
exec_namespace({"inspect": inspect, "__name__": []})
861-
862-
module_name = f"{__name__}.not_registered"
863-
for module in (None, object(), types.ModuleType(module_name)):
864-
with self.subTest(module=module):
865-
sys.modules[module_name] = module
866-
try:
867-
exec_namespace({
868-
"inspect": inspect,
869-
"__name__": module_name,
870-
})
871-
finally:
872-
del sys.modules[module_name]
854+
"""), filename, "exec"), namespace)
855+
self.assertIs(inspect.getmodule(namespace["frame"]), expected)
856+
self.assertIs(
857+
inspect.getmodule(namespace["frame"].f_code), expected)
858+
self.assertIs(inspect.getmodule(namespace["traceback"]), expected)
859+
860+
with (unittest.mock.patch.object(inspect, "modulesbyfile", {}),
861+
unittest.mock.patch.object(inspect, "_filesbymodname", {}),
862+
unittest.mock.patch.object(
863+
inspect, "_modulesbyfile_snapshot", None)):
864+
# Preserve filename-based resolution when the execution namespace
865+
# does not identify the module that supplied the source.
866+
exec_namespace({"inspect": inspect}, modfile, mod)
867+
exec_namespace({"inspect": inspect, "__name__": []},
868+
modfile, mod)
869+
870+
module_name = f"{__name__}.not_registered"
871+
for module in (None, object(), types.ModuleType(module_name)):
872+
with self.subTest(module=module):
873+
sys.modules[module_name] = module
874+
try:
875+
exec_namespace({
876+
"inspect": inspect,
877+
"__name__": module_name,
878+
}, modfile, mod)
879+
finally:
880+
del sys.modules[module_name]
881+
882+
# A namespace and filename with no registered module still
883+
# resolves to None after the filename fallback.
884+
with temp_cwd() as cwd:
885+
filename = os.path.join(cwd, "not_registered.py")
886+
exec_namespace({"inspect": inspect}, filename, None)
887+
888+
def test_getmodule_skips_unchanged_module_rescan(self):
889+
with temp_cwd() as cwd:
890+
filename = os.path.join(cwd, "not_registered.py")
891+
namespace = {"inspect": inspect}
892+
exec(compile("frame = inspect.currentframe()", filename, "exec"),
893+
namespace)
894+
frame = namespace["frame"]
895+
modules = sys.modules.copy()
896+
module_name = f"{__name__}.late_registered"
897+
marker_name = f"{__name__}.scan_marker"
898+
marker = types.ModuleType(marker_name)
899+
marker.__file__ = os.path.join(cwd, "scan_marker.py")
900+
with open(marker.__file__, "w"):
901+
pass
902+
modules[marker_name] = marker
903+
904+
original_ismodule = inspect.ismodule
905+
scan_count = 0
906+
907+
def ismodule(object):
908+
nonlocal scan_count
909+
if object is marker:
910+
scan_count += 1
911+
return original_ismodule(object)
912+
913+
with (unittest.mock.patch.object(sys, "modules", modules),
914+
unittest.mock.patch.object(inspect, "modulesbyfile", {}),
915+
unittest.mock.patch.object(inspect, "_filesbymodname", {}),
916+
unittest.mock.patch.object(inspect, "ismodule", ismodule),
917+
unittest.mock.patch.object(
918+
inspect, "_modulesbyfile_snapshot", None)):
919+
self.assertIsNone(inspect.getmodule(frame, filename))
920+
first_scan_count = scan_count
921+
self.assertGreater(first_scan_count, 0)
922+
self.assertIsNone(inspect.getmodule(frame, filename))
923+
self.assertEqual(scan_count, first_scan_count)
924+
925+
module = types.ModuleType(module_name)
926+
module.__file__ = filename
927+
modules[module_name] = module
928+
self.assertIs(inspect.getmodule(frame.f_code), module)
929+
self.assertGreater(scan_count, first_scan_count)
930+
second_scan_count = scan_count
931+
932+
# A replacement sys.modules object must invalidate the
933+
# snapshot even when it contains the same entries.
934+
replacement = modules.copy()
935+
unknown = os.path.join(cwd, "unknown.py")
936+
with unittest.mock.patch.object(sys, "modules", replacement):
937+
self.assertIsNone(inspect.getmodule(None, unknown))
938+
self.assertGreater(scan_count, second_scan_count)
939+
940+
def test_getmodule_sys_modules_changes_during_key_snapshot(self):
941+
class MutatingModules(dict):
942+
mutate = True
943+
944+
def __iter__(self):
945+
iterator = super().__iter__()
946+
yield next(iterator)
947+
if self.mutate:
948+
self.mutate = False
949+
self[f"{__name__}.added_during_iteration"] = None
950+
yield from iterator
951+
952+
modules = MutatingModules(sys.modules)
953+
with (temp_cwd() as cwd,
954+
unittest.mock.patch.object(sys, "modules", modules),
955+
unittest.mock.patch.object(inspect, "modulesbyfile", {}),
956+
unittest.mock.patch.object(inspect, "_filesbymodname", {}),
957+
unittest.mock.patch.object(
958+
inspect, "_modulesbyfile_snapshot", None)):
959+
filename = os.path.join(cwd, "not_registered.py")
960+
self.assertIsNone(inspect.getmodule(None, filename))
961+
962+
def test_getmodule_snapshot_does_not_retain_modules(self):
963+
with temp_cwd() as cwd:
964+
module_name = f"{__name__}.snapshot_module"
965+
module = types.ModuleType(module_name)
966+
module.__file__ = os.path.join(cwd, "snapshot_module.py")
967+
with open(module.__file__, "w"):
968+
pass
969+
module_ref = weakref.ref(module)
970+
modules = sys.modules.copy()
971+
modules[module_name] = module
972+
973+
with (unittest.mock.patch.object(sys, "modules", modules),
974+
unittest.mock.patch.object(inspect, "modulesbyfile", {}),
975+
unittest.mock.patch.object(inspect, "_filesbymodname", {}),
976+
unittest.mock.patch.object(
977+
inspect, "_modulesbyfile_snapshot", None)):
978+
self.assertIs(inspect.getmodule(None, module.__file__), module)
979+
del modules[module_name]
980+
del module
981+
support.gc_collect()
982+
self.assertIsNone(module_ref())
873983

874984
def test_getmodule_registered_exec_frame(self):
875985
def exec_module(module, filename):
@@ -902,8 +1012,24 @@ def exec_module(module, filename):
9021012
self.assertIsNone(inspect.getmodule(module.frame))
9031013
self.assertIsNone(inspect.getmodule(module.traceback))
9041014

905-
# Preserve the existing result for fileless modules while
906-
# avoiding a scan of sys.modules.
1015+
# If another module supplied the source, fall back to the
1016+
# filename-based lookup instead of trusting frame globals.
1017+
source_name = f"{module_name}.source"
1018+
source_module = types.ModuleType(source_name)
1019+
source_module.__file__ = filename + ".source"
1020+
with open(source_module.__file__, "w"):
1021+
pass
1022+
sys.modules[source_name] = source_module
1023+
try:
1024+
exec_module(module, source_module.__file__)
1025+
self.assertIs(inspect.getmodule(module.frame),
1026+
source_module)
1027+
self.assertIs(inspect.getmodule(module.traceback),
1028+
source_module)
1029+
finally:
1030+
del sys.modules[source_name]
1031+
1032+
# Preserve the existing result for fileless modules.
9071033
del module.__file__
9081034
exec_module(module, "<fileless>")
9091035
self.assertIsNone(inspect.getmodule(module.frame))

Lib/test/test_zipimport_support.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import zipfile
1111
import zipimport
1212
import doctest
13+
import importlib
1314
import inspect
1415
import linecache
1516
import unittest
@@ -96,6 +97,36 @@ def test_inspect_getsource_issue4223(self):
9697
finally:
9798
del sys.modules["zip_pkg"]
9899

100+
def test_inspect_fresh_namespace_uses_module_loader(self):
101+
test_src = textwrap.dedent("""\
102+
import inspect
103+
104+
def capture():
105+
return inspect.currentframe()
106+
107+
frame = capture()
108+
""")
109+
expected_source = textwrap.dedent("""\
110+
def capture():
111+
return inspect.currentframe()
112+
""")
113+
module_name = "test_zipped_inspect"
114+
with os_helper.temp_dir() as d:
115+
script_name = make_script(d, module_name, test_src)
116+
zip_name, _ = make_zip_script(d, "test_zip", script_name)
117+
os.remove(script_name)
118+
sys.path.insert(0, zip_name)
119+
module = importlib.import_module(module_name)
120+
try:
121+
namespace = {}
122+
exec(compile(test_src, module.__file__, "exec"), namespace)
123+
frame = namespace["frame"]
124+
125+
self.assertIs(inspect.getmodule(frame), module)
126+
self.assertEqual(inspect.getsource(frame), expected_source)
127+
finally:
128+
del sys.modules[module_name]
129+
99130
def test_doctest_issue4197(self):
100131
# To avoid having to keep two copies of the doctest module's
101132
# unit tests in sync, this test works by taking the source of
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Improve :func:`inspect.getmodule` performance for frame and traceback objects by resolving registered modules directly from frame globals. Frames executing in unregistered globals now return ``None`` instead of being associated with a module solely by filename.
1+
Improve :func:`inspect.getmodule` performance for frame and traceback objects by resolving registered modules directly from frame globals and avoiding repeated full scans when the names in :data:`sys.modules` are unchanged.

0 commit comments

Comments
 (0)