From 50e5921adc827a286f813e1201deb0b0c8d8214c Mon Sep 17 00:00:00 2001 From: SwayamInSync Date: Mon, 7 Sep 2026 19:58:15 +0530 Subject: [PATCH] gh-149728: bind a submodule on its parent before it stops initializing --- Lib/importlib/_bootstrap.py | 46 ++++--- .../test_importlib/test_threaded_import.py | 114 ++++++++++++++++++ ...-09-07-16-40-00.gh-issue-149728.LzyImp.rst | 5 + 3 files changed, 149 insertions(+), 16 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-09-07-16-40-00.gh-issue-149728.LzyImp.rst diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py index 081bb98ec2a9723..bd38be1368043f2 100644 --- a/Lib/importlib/_bootstrap.py +++ b/Lib/importlib/_bootstrap.py @@ -890,8 +890,14 @@ def _exec(spec, module): sys.modules[spec.name] = module return module -def _load_unlocked(spec): +def _load_unlocked(spec, finish_load=None): # A helper for direct use by the import system. + # + # If given, finish_load() is called with the loaded module just before + # spec._initializing is cleared. Other threads may use a module whose + # spec is no longer initializing without taking the import lock, so + # anything that has to be in place by then (such as binding the module + # on its parent package) belongs in finish_load(). module = module_from_spec(spec) # This must be done before putting the module in sys.modules @@ -920,6 +926,8 @@ def _load_unlocked(spec): module = sys.modules.pop(spec.name) sys.modules[spec.name] = module _verbose_message('import {!r} # {!r}', spec.name, spec.loader) + if finish_load is not None: + finish_load(module) finally: spec._initializing = False @@ -1295,29 +1303,35 @@ def _find_and_load_unlocked(name, import_, *, lazy_submodule=False): return None raise ModuleNotFoundError(f'{_ERR_MSG_PREFIX}{name!r}', name=name) else: + def finish_load(module): + # Called by _load_unlocked() while spec._initializing is still + # true, so that no thread can see the module as fully imported + # before it is reachable as an attribute of its parent. + if parent: + # Set the module as an attribute on its parent. + parent_module = sys.modules[parent] + try: + setattr(parent_module, child, module) + except AttributeError: + msg = (f"Cannot set an attribute on {parent!r} " + f"for child module {child!r}") + _warnings.warn(msg, ImportWarning) + # Set attributes to lazy submodules on the module. + try: + _imp._set_lazy_attributes(module, name) + except Exception as e: + msg = f"Cannot set lazy attributes on {name!r}: {e!r}" + _warnings.warn(msg, ImportWarning) + if parent_spec: # Temporarily add child we are currently importing to parent's # _uninitialized_submodules for circular import tracking. parent_spec._uninitialized_submodules.append(child) try: - module = _load_unlocked(spec) + module = _load_unlocked(spec, finish_load) finally: if parent_spec: parent_spec._uninitialized_submodules.pop() - if parent: - # Set the module as an attribute on its parent. - parent_module = sys.modules[parent] - try: - setattr(parent_module, child, module) - except AttributeError: - msg = f"Cannot set an attribute on {parent!r} for child module {child!r}" - _warnings.warn(msg, ImportWarning) - # Set attributes to lazy submodules on the module. - try: - _imp._set_lazy_attributes(module, name) - except Exception as e: - msg = f"Cannot set lazy attributes on {name!r}: {e!r}" - _warnings.warn(msg, ImportWarning) return module diff --git a/Lib/test/test_importlib/test_threaded_import.py b/Lib/test/test_importlib/test_threaded_import.py index 6875fdca9c8528d..b6fedabd62ca2b5 100644 --- a/Lib/test/test_importlib/test_threaded_import.py +++ b/Lib/test/test_importlib/test_threaded_import.py @@ -461,6 +461,120 @@ def do_import(name): errors, [], f"Import(s) failed on iteration {i}: {errors}") + def test_lazy_submodule_not_visible_before_parent_setattr(self): + # gh-149728: a submodule must not be advertised as fully imported + # before it is bound as an attribute of its parent package. A + # thread seeing that window in a package that resolves its + # submodules from a module-level __getattr__ would re-enter + # __getattr__ for the same submodule over and over, until + # RecursionError. + os.makedirs(os.path.join(TESTFN, "lazypkg")) + self.addCleanup(shutil.rmtree, TESTFN) + with open(os.path.join(TESTFN, "lazypkg", "__init__.py"), "w") as f: + f.write("""if 1: + import sys + import threading + from types import ModuleType + + # Set when the import system is about to bind 'sub' on this + # package, i.e. at the start of the window under test. + setattr_reached = threading.Event() + # Set by the probing thread once it is out of that window. + probe_finished = threading.Event() + # __spec__._initializing of 'lazypkg.sub', as seen from + # inside the window. + observed_initializing = [] + # Whether the probing thread was still blocked when the + # window closed. + probe_blocked = [] + + _importing = threading.local() + + class _Package(ModuleType): + def __setattr__(self, name, value): + if name == "sub": + observed_initializing.append( + value.__spec__._initializing) + setattr_reached.set() + # Hold the window open long enough to see what + # the other thread does with it. This is a cap, + # not a synchronisation point: a thread wrongly + # handed the module returns at once and sets + # probe_finished, while a thread correctly made + # to block cannot set it at all, since it waits + # on the module lock held right here. + probe_blocked.append( + not probe_finished.wait(0.1)) + super().__setattr__(name, value) + + def __getattr__(name): + if name != "sub": + raise AttributeError(name) + if getattr(_importing, "sub", False): + raise AssertionError( + "lazypkg.__getattr__ re-entered for 'sub': the " + "submodule was handed out as fully imported " + "before being bound on its parent package") + _importing.sub = True + try: + import lazypkg.sub as sub + finally: + _importing.sub = False + return sub + + sys.modules[__name__].__class__ = _Package + """) + with open(os.path.join(TESTFN, "lazypkg", "sub.py"), "w") as f: + f.write("X = 42\n") + + sys.path.insert(0, TESTFN) + self.addCleanup(sys.path.remove, TESTFN) + for mod in ("lazypkg", "lazypkg.sub"): + self.addCleanup(forget, mod) + importlib.invalidate_caches() + + pkg = importlib.import_module("lazypkg") + results = {} + + def importer(): + try: + results["importer"] = pkg.sub + except BaseException as exc: + results["importer_error"] = exc + + def prober(): + pkg.setattr_reached.wait(support.SHORT_TIMEOUT) + try: + results["prober"] = pkg.sub + except BaseException as exc: + results["prober_error"] = exc + finally: + pkg.probe_finished.set() + + threads = [threading.Thread(target=prober), + threading.Thread(target=importer)] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=support.SHORT_TIMEOUT) + for thread in threads: + self.assertFalse(thread.is_alive(), "thread deadlocked") + + for key in ("importer_error", "prober_error"): + if key in results: + raise results[key] + sub = sys.modules["lazypkg.sub"] + self.assertIs(results["importer"], sub) + self.assertIs(results["prober"], sub) + # The submodule is bound on its parent while its spec still says it + # is initializing, so no thread can take the lock-free fast path + # for it before it is reachable through the parent package. + self.assertEqual(pkg.observed_initializing, [True]) + # And the other thread really was held off for as long as that + # window was open, rather than let through to a package that does + # not have 'sub' yet. + self.assertEqual(pkg.probe_blocked, [True]) + def setUpModule(): thread_info = threading_helper.threading_setup() diff --git a/Misc/NEWS.d/next/Library/2026-09-07-16-40-00.gh-issue-149728.LzyImp.rst b/Misc/NEWS.d/next/Library/2026-09-07-16-40-00.gh-issue-149728.LzyImp.rst new file mode 100644 index 000000000000000..c082860e5bd2a6c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-09-07-16-40-00.gh-issue-149728.LzyImp.rst @@ -0,0 +1,5 @@ +Fix a race in the import system where a submodule was briefly visible as +fully imported before being set as an attribute of its parent package. A +package importing its submodules from a module-level ``__getattr__`` could +raise :exc:`RecursionError` when several threads first used the same +submodule.