From fea385a6e9ca69417a49c3318c7308155bef208d Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Tue, 20 Oct 2026 22:04:30 +0000 Subject: [PATCH 1/7] gh-159963: Don't hold parent package import locks while a submodule executes Since gh-83065, the first import of pkg.mod kept pkg's module lock until pkg/mod.py had finished executing. Threads needing only the finished pkg were blocked for that long, and concurrent first imports could raise _DeadlockError or a spurious "partially initialized module" ImportError, or hang, with no import cycle involved. Parent packages are now imported, or waited for, before the submodule's lock is taken, and only that lock is held while it executes. The original gh-83065 deadlock stays fixed. importlib.import_module() now returns the partially initialized module in a cross-thread circular import, as the import statement already did, instead of raising _DeadlockError. --- Doc/whatsnew/3.15.rst | 26 + Lib/importlib/_bootstrap.py | 163 +++-- .../test_importlib/import_/test_packages.py | 16 + .../test_importlib/test_threaded_import.py | 557 ++++++++++++++++++ ...-10-20-22-00-00.gh-issue-159963.pLkHld.rst | 12 + 5 files changed, 690 insertions(+), 84 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-10-20-22-00-00.gh-issue-159963.pLkHld.rst diff --git a/Doc/whatsnew/3.15.rst b/Doc/whatsnew/3.15.rst index 7c4996c5603c6c5..e182fab96fc4e91 100644 --- a/Doc/whatsnew/3.15.rst +++ b/Doc/whatsnew/3.15.rst @@ -2917,3 +2917,29 @@ that may require changes to your code. Use warning filters to silence them or additional :meth:`!assertWarns*` to catch and check them. (Contributed by Serhiy Storchaka in :gh:`143231`.) + + +Notable changes in 3.15.2 +========================= + +Import system +------------- + +* The :gh:`83065` fix shipped in 3.15.0 made the first ``import pkg.mod`` + in a process keep ``pkg``'s import lock held until ``pkg/mod.py`` had + finished executing. Other threads importing anything from ``pkg`` in that + window could block for the whole of ``mod``'s execution, raise + ``_DeadlockError``, or get a spurious "partially initialized module" + :exc:`ImportError`, even without any circular import. Parent packages are + now imported, or waited for if another thread is still initializing them, + *before* the submodule's own lock is taken, and a package's lock is released + as soon as its ``__init__`` returns; the original deadlock remains fixed. + As in 3.15.0, the first import of anything beneath a package waits for that + package's ``__init__`` to finish in whichever thread is running it, so a + package ``__init__`` should not block on another thread that performs such + an import. + :func:`importlib.import_module` of a module caught in a genuine cross-thread + circular import now returns the partially initialized module, as the + :keyword:`import` statement already did, instead of raising + ``_DeadlockError``. + (Contributed by Gregory P. Smith in :gh:`159963`.) diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py index 081bb98ec2a9723..7d5f118ce10fefa 100644 --- a/Lib/importlib/_bootstrap.py +++ b/Lib/importlib/_bootstrap.py @@ -418,64 +418,6 @@ def __exit__(self, *args, **kwargs): self._lock.release() -def _get_module_chain(name): - """Return the chain of dotted-name prefixes from root to leaf. - - For example: 'a.b.c' -> ['a', 'a.b', 'a.b.c'] - """ - parts = name.split('.') - return ['.'.join(parts[:i+1]) for i in range(len(parts))] - - -class _HierarchicalLockManager: - """Manages acquisition of multiple module locks in hierarchical order. - - This prevents deadlocks by ensuring all threads acquire locks in the - same order (parent modules before child modules). - """ - - def __init__(self, name): - self._name = name - self._module_chain = _get_module_chain(name) - self._locks = [] - - def __enter__(self): - try: - for module_name in self._module_chain: - # Only acquire lock if module is not already fully loaded - module = sys.modules.get(module_name) - if (module is None or - getattr(getattr(module, "__spec__", None), - "_initializing", False)): - lock = _get_module_lock(module_name) - try: - lock.acquire() - except _DeadlockError: - if module_name == self._name: - raise - # The parent is being initialised by a thread that - # is (transitively) waiting on a lock we hold. - # Apply the same policy as _lock_unlock_module(): - # accept a partially-initialised parent for circular - # imports rather than failing the whole chain. - continue - self._locks.append((module_name, lock)) - except: - # __exit__ is not called when __enter__ raises (e.g. _DeadlockError - # on the leaf lock, or KeyboardInterrupt), so release whatever we - # already hold to avoid permanently leaking held module locks. - for module_name, lock in reversed(self._locks): - lock.release() - self._locks.clear() - raise - return self - - def __exit__(self, *args, **kwargs): - for module_name, lock in reversed(self._locks): - lock.release() - self._locks.clear() - - # The following two functions are for consumption by Python/import.c. def _get_module_lock(name): @@ -1256,17 +1198,17 @@ def _sanity_check(name, package, level): def _find_and_load_unlocked(name, import_, *, lazy_submodule=False): path = None - sys.audit( - "import", - name, - path, - getattr(sys, "path", None), - getattr(sys, "meta_path", None), - getattr(sys, "path_hooks", None) - ) parent = name.rpartition('.')[0] parent_spec = None if parent: + # _resolve_parents() has settled the parent, but nothing pins it in + # sys.modules while this thread waits for name's lock. If it was + # deleted meanwhile, the first import_(parent) below imports it again + # (else sys.modules[parent] raises KeyError) and the second waits for + # another thread doing so, in case its __init__ makes this submodule + # importable (gh-130094). Otherwise the second call returns at once, + # for this thread's own partly initialized parent or a partial parent + # accepted in a circular import split across threads. if parent not in sys.modules: _call_with_frames_removed(import_, parent) # Crazy side-effects! @@ -1324,31 +1266,84 @@ def _find_and_load_unlocked(name, import_, *, lazy_submodule=False): _NEEDS_LOADING = object() +def _is_initializing(module): + """Return whether *module* (a sys.modules value) is still executing.""" + return getattr(getattr(module, '__spec__', None), '_initializing', False) + + +def _resolve_parents(name, import_): + """Import, or wait for, the parent packages of *name*. + + _find_and_load() calls this before taking name's module lock, so that no + thread holds a submodule's lock while waiting for its parent: ``import + pkg.mod`` and a pkg/__init__.py importing pkg.mod would then take the two + locks in opposite orders (gh-83065). Each level's lock is released as + soon as that level is settled. Holding a parent's lock through the + submodule's body blocks threads which need only the finished parent, and + causes _DeadlockError and "partially initialized module" errors with no + import cycle (gh-159963). + """ + # Only the immediate parent has to exist, so ancestors above it are not + # imported here. Wait for those another thread is still executing, + # outermost first: otherwise this thread could enter a chain of imports + # within a package from the far end while that package's __init__ is + # working through it, and one of the two would get a partially initialized + # module. Hence an __init__ must not block on another thread importing + # something beneath it. + parts = name.split('.') + for i in range(1, len(parts) - 1): + ancestor = '.'.join(parts[:i]) + if _is_initializing(sys.modules.get(ancestor)): + _lock_unlock_module(ancestor) + parent = name.rpartition('.')[0] + if parent: + module = sys.modules.get(parent, _NEEDS_LOADING) + if module is _NEEDS_LOADING or _is_initializing(module): + _call_with_frames_removed(import_, parent) + + def _find_and_load(name, import_, *, lazy_submodule=False): """Find and load the module.""" # Optimization: we avoid unneeded module locking if the module # already exists in sys.modules and is fully initialized. module = sys.modules.get(name, _NEEDS_LOADING) - if (module is _NEEDS_LOADING or - getattr(getattr(module, "__spec__", None), "_initializing", False)): - - if '.' in name: - lock_manager = _HierarchicalLockManager(name) - else: - lock_manager = _ModuleLockManager(name) - - with lock_manager: + if module is _NEEDS_LOADING or _is_initializing(module): + if module is not _NEEDS_LOADING: + # NOTE: this relies on __spec__._initializing being set *before* + # the module is put in sys.modules. + _lock_unlock_module(name) + # The module is gone if the thread loading it failed. module = sys.modules.get(name, _NEEDS_LOADING) - if module is _NEEDS_LOADING: - return _find_and_load_unlocked( - name, import_, lazy_submodule=lazy_submodule) - - # Optimization: only call _bootstrap._lock_unlock_module() if - # module.__spec__._initializing is True. - # NOTE: because of this, initializing must be set *before* - # putting the new module in sys.modules. - _lock_unlock_module(name) + + if module is _NEEDS_LOADING: + sys.audit( + "import", + name, + None, + getattr(sys, "path", None), + getattr(sys, "meta_path", None), + getattr(sys, "path_hooks", None) + ) + _resolve_parents(name, import_) + lock = _get_module_lock(name) + try: + lock.acquire() + except _DeadlockError: + # A circular import split across threads. As in + # _lock_unlock_module(), accept a partially initialized + # module if there is one. + module = sys.modules.get(name, _NEEDS_LOADING) + if module is _NEEDS_LOADING: + raise + else: + try: + module = sys.modules.get(name, _NEEDS_LOADING) + if module is _NEEDS_LOADING: + return _find_and_load_unlocked( + name, import_, lazy_submodule=lazy_submodule) + finally: + lock.release() else: # Verify the module is still in sys.modules. Another thread may have # removed it (due to import failure) between our sys.modules.get() diff --git a/Lib/test/test_importlib/import_/test_packages.py b/Lib/test/test_importlib/import_/test_packages.py index 0c29d6083265fa1..10ca5ce2d5a0e1d 100644 --- a/Lib/test/test_importlib/import_/test_packages.py +++ b/Lib/test/test_importlib/import_/test_packages.py @@ -1,5 +1,6 @@ from test.test_importlib import util import sys +import types import unittest from test.support import import_helper @@ -100,6 +101,21 @@ def module_injection(): finally: import_helper.unload(subname) + def test_missing_grandparent_not_imported(self): + # Only the immediate parent has to be present. With a hand-made + # 'pkg.sub' package in sys.modules and no 'pkg' at all, importing + # pkg.sub.mod must neither fail nor try to import 'pkg'. + parent = types.ModuleType('pkg.sub') + parent.__path__ = [] + with util.mock_spec('pkg.sub.mod') as mock: + with util.import_state(meta_path=[mock]): + with util.uncache('pkg', 'pkg.sub', 'pkg.sub.mod'): + sys.modules['pkg.sub'] = parent + module = self.__import__('pkg.sub.mod', fromlist=['*']) + self.assertEqual(module.__name__, 'pkg.sub.mod') + self.assertIs(parent.mod, module) + self.assertNotIn('pkg', sys.modules) + (Frozen_ParentTests, Source_ParentTests diff --git a/Lib/test/test_importlib/test_threaded_import.py b/Lib/test/test_importlib/test_threaded_import.py index 6875fdca9c8528d..c7829205bd6113c 100644 --- a/Lib/test/test_importlib/test_threaded_import.py +++ b/Lib/test/test_importlib/test_threaded_import.py @@ -6,12 +6,16 @@ # randrange, and then Python hangs. import _imp as imp +import collections +import faulthandler import os import importlib +import importlib.machinery import sys import time import shutil import threading +import types import unittest from test import support from test.support import verbose @@ -461,6 +465,559 @@ def do_import(name): errors, [], f"Import(s) failed on iteration {i}: {errors}") + # Helpers for the deterministic two-thread tests below. Rather than + # relying on sleeps, a package __init__ calls back into the test and is + # held there until the second thread is observed waiting on a specific + # module lock, so the problematic interleaving happens on every run. + + def _setup_package_tree(self, files): + # These tests are about what happens when one module's body imports + # another. Under -X lazy_imports=all those imports would never run. + self.addCleanup(sys.set_lazy_imports, sys.get_lazy_imports()) + sys.set_lazy_imports("normal") + os.makedirs(TESTFN, exist_ok=True) + self.addCleanup(rmtree, TESTFN) + for relpath, source in files.items(): + path = os.path.join(TESTFN, *relpath.split("/")) + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, "w", encoding="utf-8") as f: + f.write(source) + sys.path.insert(0, TESTFN) + self.addCleanup(sys.path.remove, TESTFN) + # Forget by top-level name, not by file name: a submodule may come + # from a directory added to __path__ or have no file at all. + tops = {relpath.partition("/")[0].removesuffix(".py") + for relpath in files} + self.addCleanup(self._forget_trees, tops) + importlib.invalidate_caches() + + @staticmethod + def _forget_trees(tops): + for name in list(sys.modules): + if name.partition(".")[0] in tops: + forget(name) + + def _wait_until_blocked_on(self, thread, modname): + # Return once *thread* is waiting to acquire *modname*'s module lock + # (any module lock if *modname* is None) and another thread owns it. + # The owner check matters: a lock is listed in _blocking_on for a + # moment during every acquire(), contended or not. + blocking_on = importlib._bootstrap._blocking_on + for _ in support.sleeping_retry( + support.SHORT_TIMEOUT, + f"thread never waited for the module lock of {modname!r}"): + for lock in list(blocking_on.get(thread.ident, ())): + if (modname in (None, lock.name) + and lock.owner not in (None, thread.ident)): + return + if not thread.is_alive(): + self.fail("thread finished without ever waiting for the " + f"module lock of {modname!r}") + + def _join_import_threads(self, threads): + # Every wait made by the test's own code inside these threads is + # bounded by SHORT_TIMEOUT and reports what it was waiting for, so + # wait longer than their sum here: only a real import deadlock gets + # this far. The threads are daemons so that one cannot also block + # interpreter shutdown. + deadline = time.monotonic() + support.LONG_TIMEOUT + for t in threads: + if t.ident is not None: + t.join(max(deadline - time.monotonic(), 0)) + hung = [t for t in threads if t.is_alive()] + if hung: + faulthandler.dump_traceback(sys.__stderr__) + self.fail(f"import deadlock: unable to join {hung}") + + def _run_two_first_imports(self, files, stmt1, stmt2, *, + sync_name, t2_waits_for): + """Thread 1 runs *stmt1*. Something it executes calls + ``.hold()``, which starts thread 2 on *stmt2* and returns + only once thread 2 is blocked on the module lock of *t2_waits_for* + (any module lock if None). Module code may also call + ``.join_t2()`` to wait for thread 2 to finish. + Returns a list of (what, problem) pairs; empty means both imports + succeeded and no module body ran more than once. + """ + # list.append() is atomic, on free-threaded builds too. + count = f"import {sync_name}; {sync_name}.executed.append(__name__)\n" + self._setup_package_tree({relpath: count + source + for relpath, source in files.items()}) + sync = types.ModuleType(sync_name) + sync.executed = [] + sys.modules[sync_name] = sync + self.addCleanup(sys.modules.pop, sync_name, None) + errors = [] + + def run(stmt): + try: + exec(stmt, {}) + except Exception as e: + errors.append((stmt, f"{type(e).__name__}: {e}")) + + t1 = threading.Thread(target=run, args=(stmt1,), daemon=True) + t2 = threading.Thread(target=run, args=(stmt2,), daemon=True) + + def hold(): + # One-shot: if a broken interpreter makes thread 1's import fail, + # thread 2 may re-run the same code; don't wait on ourselves. + if t2.ident is not None: + return + t2.start() + self._wait_until_blocked_on(t2, t2_waits_for) + sync.hold = hold + + def join_t2(): + t2.join(support.SHORT_TIMEOUT) + if t2.is_alive(): + errors.append((stmt2, "still blocked while thread 1's module " + "code waited for it")) + sync.join_t2 = join_t2 + + t1.start() + self._join_import_threads([t1, t2]) + if t2.ident is None: + errors.append((stmt2, "was never started")) + for name, n in collections.Counter(sync.executed).items(): + if n != 1: + errors.append((name, f"executed {n} times")) + return errors + + def _hold_in_create_module(self, modname, sync_name): + # Have the import of *modname* call .hold() from its + # loader's create_module(): the importing thread then owns modname's + # module lock but modname is not in sys.modules yet. (A finder's + # find_spec() would be too early a place to stop: it runs under the + # global import lock, which every other import needs briefly.) + class Loader(importlib.machinery.SourceFileLoader): + def create_module(self, spec): + sys.modules[sync_name].hold() + return super().create_module(spec) + + class Finder: + def find_spec(self, name, path=None, target=None): + if name != modname: + return None + spec = importlib.machinery.PathFinder.find_spec( + name, path, target) + spec.loader = Loader(spec.loader.name, spec.loader.path) + return spec + + finder = Finder() + sys.meta_path.insert(0, finder) + self.addCleanup(sys.meta_path.remove, finder) + + # A first import of {pkg}.mod. The statements call back into C + # (import_ensure_initialized()) for a parent that is in sys.modules; + # import_module() stays in _find_and_load(). + SUBMODULE_IMPORTS = ( + "from {pkg}.mod import X", + "import {pkg}.mod", + "import importlib; importlib.import_module('{pkg}.mod')", + ) + + def test_parent_lock_not_held_while_submodule_executes(self): + # gh-159963: the first `import pkg.mod` in a process also imports pkg. + # Once pkg/__init__.py has finished, pkg's module lock must not stay + # held while mod's body runs; otherwise every thread that began + # waiting for pkg is blocked for the whole of mod's body as well + # (the threaded tests below show what that leads to). + self._setup_package_tree({ + "unheld_pkg/__init__.py": "", + "unheld_pkg/mod.py": ( + "import sys, importlib._bootstrap as _b\n" + "def held(name):\n" + " ref = _b._module_locks.get(name)\n" + " lock = ref() if ref is not None else None\n" + " return lock is not None and lock.locked()\n" + "own_lock_held = held(__name__)\n" + "parent_lock_held = held('unheld_pkg')\n" + "parent_initializing = " + "sys.modules['unheld_pkg'].__spec__._initializing\n"), + }) + import unheld_pkg.mod + self.assertTrue(unheld_pkg.mod.own_lock_held) + self.assertFalse(unheld_pkg.mod.parent_initializing) + self.assertFalse(unheld_pkg.mod.parent_lock_held) + + def test_package_init_imports_submodule_deterministic(self): + # gh-83065 (bpo-38884) with the losing interleaving forced: thread 1 + # is inside pkg/__init__.py, about to import pkg.mod, at the moment + # thread 2's first import of pkg.mod has got as far as waiting for + # pkg. Thread 2 must be waiting *without* already owning pkg.mod's + # lock, or thread 1's import of pkg.mod deadlocks against it. + for i, stmt in enumerate(self.SUBMODULE_IMPORTS): + pkg = f"detinit{i}_pkg" + stmt = stmt.format(pkg=pkg) + with self.subTest(stmt=stmt): + errors = self._run_two_first_imports( + { + f"{pkg}/__init__.py": ( + "import detinit_sync; detinit_sync.hold()\n" + f"import {pkg}.mod\n"), + f"{pkg}/mod.py": "X = 1\n", + }, + f"import {pkg}", + stmt, + sync_name="detinit_sync", + t2_waits_for=pkg, + ) + self.assertEqual(errors, []) + self.assertEqual(sys.modules[f"{pkg}.mod"].X, 1) + + def test_submodule_import_while_package_not_yet_in_sys_modules(self): + # gh-83065, the earlier window: thread 1 owns pkg's lock but has not + # put pkg in sys.modules yet when thread 2 first imports pkg.mod. + for i, stmt in enumerate(self.SUBMODULE_IMPORTS): + pkg = f"early{i}_pkg" + stmt = stmt.format(pkg=pkg) + with self.subTest(stmt=stmt): + self._hold_in_create_module(pkg, "early_sync") + errors = self._run_two_first_imports( + { + f"{pkg}/__init__.py": "from . import mod\n", + f"{pkg}/mod.py": "X = 1\n", + }, + f"import {pkg}", + stmt, + sync_name="early_sync", + t2_waits_for=pkg, + ) + self.assertEqual(errors, []) + self.assertEqual(sys.modules[f"{pkg}.mod"].X, 1) + + def test_submodule_found_via_path_set_late_in_package_init(self): + # pkg.mod can only be found once pkg/__init__.py has replaced + # __path__, pkgutil.extend_path() style. Thread 2 must look at + # pkg.__path__ after it has waited for pkg, not before. + extra = os.path.abspath(os.path.join(TESTFN, "latepath_extra")) + errors = self._run_two_first_imports( + { + "latepath_pkg/__init__.py": ( + "import latepath_sync; latepath_sync.hold()\n" + f"__path__ = [*__path__, {extra!r}]\n"), + "latepath_extra/mod.py": "X = 1\n", + }, + "import latepath_pkg", + "from latepath_pkg.mod import X", + sync_name="latepath_sync", + t2_waits_for="latepath_pkg", + ) + self.assertEqual(errors, []) + self.assertEqual(sys.modules["latepath_pkg.mod"].X, 1) + + def test_submodule_put_in_sys_modules_late_in_package_init(self): + # gh-130094: there is no pkg/mod.py; pkg/__init__.py puts pkg.mod in + # sys.modules itself, as collections does for collections.abc. + errors = self._run_two_first_imports( + { + "inject_pkg/__init__.py": ( + "import inject_sync; inject_sync.hold()\n" + "import sys, types\n" + "mod = types.ModuleType('inject_pkg.mod'); mod.X = 1\n" + "sys.modules['inject_pkg.mod'] = mod\n"), + }, + "import inject_pkg", + "from inject_pkg.mod import X", + sync_name="inject_sync", + t2_waits_for="inject_pkg", + ) + self.assertEqual(errors, []) + + def test_concurrent_first_imports_of_sibling_packages(self): + # gh-159963. Thread 1: `import a.leaf`, where a/leaf.py needs b.leaf. + # Thread 2: `import b.leaf`, where b/__init__.py needs a, started + # while thread 1 is still inside a/__init__.py. + # Dependencies are a.leaf -> b.leaf -> b -> a; `a` needs nothing, so + # there is no cycle and either order works in one thread. This + # failed with _DeadlockError when the importer of a.leaf kept a's + # lock for the duration of a/leaf.py. + errors = self._run_two_first_imports( + { + "sib_a/__init__.py": "import sib_sync; sib_sync.hold()\n", + "sib_a/leaf.py": "from sib_b.leaf import X\n", + "sib_b/__init__.py": "import sib_a\n", + "sib_b/leaf.py": "X = 1\n", + }, + "import sib_a.leaf", + "import sib_b.leaf", + sync_name="sib_sync", + t2_waits_for="sib_a", + ) + self.assertEqual(errors, []) + self.assertEqual(sys.modules["sib_a.leaf"].X, 1) + self.assertEqual(sys.modules["sib_b.leaf"].X, 1) + + def test_concurrent_first_imports_no_partial_module(self): + # Dependencies are a.leaf -> b -> a, again with no cycle. Thread 1 + # finds b in sys.modules and waits for it, so what it got when it + # kept a's lock during a/leaf.py was not _DeadlockError but "cannot + # import name 'X' from partially initialized module 'b'". + errors = self._run_two_first_imports( + { + "part_a/__init__.py": "import part_sync; part_sync.hold()\n", + "part_a/leaf.py": "from part_b import X\n", + "part_b.py": "import part_a\nX = 1\n", + }, + "import part_a.leaf", + "import part_b", + sync_name="part_sync", + t2_waits_for="part_a", + ) + self.assertEqual(errors, []) + self.assertEqual(sys.modules["part_a.leaf"].X, 1) + + def test_finished_package_import_not_blocked_by_submodule_body(self): + # Thread 2 starts waiting for pkg while pkg/__init__.py runs. It + # needs only the finished pkg, so it must be released when that + # __init__ returns and not when thread 1's pkg/main.py does. Here + # main.py waits for thread 2, which no deadlock detector can see. + for i, stmt in enumerate(( + "import {pkg}", + "import {pkg}.other", + "from {pkg}.other import X")): + pkg = f"body{i}_pkg" + stmt = stmt.format(pkg=pkg) + with self.subTest(stmt=stmt): + errors = self._run_two_first_imports( + { + f"{pkg}/__init__.py": ( + "import body_sync; body_sync.hold()\n"), + f"{pkg}/main.py": ( + "import body_sync; body_sync.join_t2()\n"), + f"{pkg}/other.py": "X = 1\n", + }, + f"import {pkg}.main", + stmt, + sync_name="body_sync", + t2_waits_for=pkg, + ) + self.assertEqual(errors, []) + + def test_concurrent_first_imports_of_nested_packages(self): + # As above but one level down: thread 1 does `import g.sub.leaf`; + # g/sub/__init__.py needs module c; c needs g. Thread 2 imports c + # while thread 1 is still inside g/__init__.py. No cycle. This + # failed with "cannot import name 'X' from partially initialized + # module" when g's lock was still held by thread 1 while it ran + # g/sub/__init__.py, because thread 2 (holding c, waiting for g) + # then looked deadlocked to thread 1. Releasing ancestor locks only + # just before the *leaf* body would not be enough to fix this one. + errors = self._run_two_first_imports( + { + "nest_g/__init__.py": "import nest_sync; nest_sync.hold()\n", + "nest_g/sub/__init__.py": "from nest_c import X\n", + "nest_g/sub/leaf.py": "Y = 2\n", + "nest_c.py": "import nest_g\nX = 1\n", + }, + "import nest_g.sub.leaf", + "import nest_c", + sync_name="nest_sync", + t2_waits_for="nest_g", + ) + self.assertEqual(errors, []) + self.assertEqual(sys.modules["nest_g.sub"].X, 1) + self.assertEqual(sys.modules["nest_g.sub.leaf"].Y, 2) + + def test_submodule_import_waits_for_initializing_top_package(self): + # Thread 1 runs g/__init__.py, which has imported subpackage g.a and + # will next need g.a.other. Thread 2's first import of g.a.other + # starts at that point; other.py needs an attribute of g that is + # already set. Thread 2 must wait for g *before* owning g.a.other: + # if it loaded g.a.other first and only then blocked on g (inside + # other.py), thread 1 would find g.a.other held by a thread waiting + # on itself and be handed the half-executed module ("cannot import + # name 'X'"). Nothing here can fail in a single thread. + errors = self._run_two_first_imports( + { + "topw_g/__init__.py": ( + "Z = 1\n" + "import topw_g.a\n" + "import topw_sync; topw_sync.hold()\n" + "from topw_g.a.other import X\n"), + "topw_g/a/__init__.py": "", + "topw_g/a/other.py": "from topw_g import Z\nX = 1\n", + }, + "import topw_g", + "from topw_g.a.other import X", + sync_name="topw_sync", + t2_waits_for="topw_g", + ) + self.assertEqual(errors, []) + self.assertEqual(sys.modules["topw_g"].X, 1) + + def test_import_module_of_initializing_submodule_waits_for_it_only(self): + # The flip side of the test above: pkg/__init__.py has *already* + # imported pkg.leaf and then blocks on thread 2 (any non-import + # wait). Thread 2, started while pkg/leaf.py is still executing, + # asks for pkg.leaf via importlib. It must get it as soon as leaf.py + # finishes, exactly as the import statement would, and not be made + # to wait for pkg/__init__.py -- which is waiting for thread 2. + errors = self._run_two_first_imports( + { + "lfo_pkg/__init__.py": ( + "import lfo_pkg.leaf\n" + "import lfo_sync; lfo_sync.join_t2()\n"), + "lfo_pkg/leaf.py": "import lfo_sync; lfo_sync.hold()\nX = 1\n", + }, + "import lfo_pkg", + "import importlib; importlib.import_module('lfo_pkg.leaf')", + sync_name="lfo_sync", + t2_waits_for=None, + ) + self.assertEqual(errors, []) + + def test_import_module_in_cross_thread_cycle_returns_partial_module(self): + # ax/x.py sets X, then imports by.y; by/y.py does `from ax.x import + # X`. Thread 1 is inside x.py (holding ax.x's lock) when thread 2's + # `import by.y` reaches y.py and blocks on ax.x. Thread 1 then asks + # for by.y and is the one to detect the cycle. The import statement + # has always resolved this by handing thread 1 the partially + # initialized by.y (import.c -> _lock_unlock_module()); + # importlib.import_module() must do the same rather than raise + # _DeadlockError into x.py. + for i, form in enumerate(( + "import by{i}_cyc.y", + "import importlib; importlib.import_module('by{i}_cyc.y')")): + form = form.format(i=i) + with self.subTest(form=form): + errors = self._run_two_first_imports( + { + f"ax{i}_cyc/__init__.py": "", + f"ax{i}_cyc/x.py": ( + "X = 1\n" + "import cyc_sync; cyc_sync.hold()\n" + f"{form}\n"), + f"by{i}_cyc/__init__.py": "", + f"by{i}_cyc/y.py": ( + f"from ax{i}_cyc.x import X\nassert X == 1\n"), + }, + f"import ax{i}_cyc.x", + f"import by{i}_cyc.y", + sync_name="cyc_sync", + t2_waits_for=f"ax{i}_cyc.x", + ) + self.assertEqual(errors, []) + + def test_submodule_enters_cross_thread_cycle_after_parents_resolved(self): + # Thread 1 is inside m.py and has found pkg.leaf missing, but only + # asks for its lock once thread 2 is inside leaf.py, waiting for m. + # Thread 1 is the one to detect the cycle, and pkg.leaf is in + # sys.modules by now, so it must get the partial module, as if it had + # found it there in the first place. + resolve_parents = importlib._bootstrap._resolve_parents + + def stalling_resolve_parents(name, import_): + resolve_parents(name, import_) + if name == "stall_pkg.leaf": + sys.modules["stall_sync"].hold() + + with support.swap_attr(importlib._bootstrap, "_resolve_parents", + stalling_resolve_parents): + errors = self._run_two_first_imports( + { + "stall_pkg/__init__.py": "", + "stall_pkg/leaf.py": "import stall_m\nX = 1\n", + "stall_m.py": "import stall_pkg.leaf\n", + }, + "import stall_m", + "import stall_pkg.leaf", + sync_name="stall_sync", + t2_waits_for="stall_m", + ) + self.assertEqual(errors, []) + self.assertEqual(sys.modules["stall_pkg.leaf"].X, 1) + + def test_waiter_retries_after_failed_submodule_import(self): + # Thread 2 waits for thread 1's import of pkg.mod, whose first + # execution then fails. Thread 2 must load pkg.mod itself and not + # return the discarded module. + for i, stmt in enumerate(self.SUBMODULE_IMPORTS): + pkg = f"retry{i}_pkg" + stmt = stmt.format(pkg=pkg) + with self.subTest(stmt=stmt): + errors = self._run_two_first_imports( + { + f"{pkg}/__init__.py": "", + f"{pkg}/mod.py": ( + "import retry_sync; retry_sync.hold()\n" + "if retry_sync.executed.count(__name__) == 1:\n" + " raise RuntimeError('first attempt fails')\n" + "X = 1\n"), + }, + f"import {pkg}.mod", + stmt, + sync_name="retry_sync", + t2_waits_for=f"{pkg}.mod", + ) + self.assertEqual(errors, [ + (f"import {pkg}.mod", + "RuntimeError: first attempt fails"), + (f"{pkg}.mod", "executed 2 times"), + ]) + self.assertEqual(sys.modules[f"{pkg}.mod"].X, 1) + + def test_waiters_recover_after_failed_package_init(self): + # pkg/__init__.py imports pkg.a and pkg.b, but its first execution + # (thread 0) raises once two other threads -- importing pkg.a and + # pkg.b -- are already waiting for pkg. One of the waiters then + # re-runs pkg/__init__.py successfully; both waiters must succeed, + # and neither may end up importing the parent while holding its + # submodule's lock (the gh-83065 lock order) on that retry path. + self._setup_package_tree({ + "flaky_pkg/__init__.py": ( + "import flaky_sync; flaky_sync.executed.append(__name__)\n" + "flaky_sync.gate()\n" + "import flaky_pkg.a\n" + "import flaky_pkg.b\n"), + "flaky_pkg/a.py": ( + "import flaky_sync; flaky_sync.executed.append(__name__)\n" + "X = 1\n"), + "flaky_pkg/b.py": ( + "import flaky_sync; flaky_sync.executed.append(__name__)\n" + "import flaky_pkg.a\nX = 1\n"), + }) + sync = types.ModuleType("flaky_sync") + sync.executed = [] + sys.modules["flaky_sync"] = sync + self.addCleanup(sys.modules.pop, "flaky_sync", None) + errors = [] + + def run(stmt): + try: + exec(stmt, {}) + except Exception as e: + errors.append((stmt, f"{type(e).__name__}: {e}")) + + waiters = [threading.Thread(target=run, daemon=True, + args=(f"import flaky_pkg.{m}",)) + for m in ("a", "b")] + first = True + + def gate(): + nonlocal first + if not first: + return + first = False + for t in waiters: + t.start() + for t in waiters: + self._wait_until_blocked_on(t, "flaky_pkg") + raise RuntimeError("first attempt fails") + sync.gate = gate + + t0 = threading.Thread(target=run, args=("import flaky_pkg",), + daemon=True) + t0.start() + self._join_import_threads([t0, *waiters]) + self.assertEqual( + errors, + [("import flaky_pkg", "RuntimeError: first attempt fails")]) + self.assertEqual(sys.modules["flaky_pkg.b"].X, 1) + self.assertEqual( + sync.executed, + ["flaky_pkg", "flaky_pkg", "flaky_pkg.a", "flaky_pkg.b"]) + def setUpModule(): thread_info = threading_helper.threading_setup() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-20-22-00-00.gh-issue-159963.pLkHld.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-20-22-00-00.gh-issue-159963.pLkHld.rst new file mode 100644 index 000000000000000..ae06db342603a77 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-20-22-00-00.gh-issue-159963.pLkHld.rst @@ -0,0 +1,12 @@ +Fix a regression introduced in Python 3.15.0 by the :gh:`83065` import +deadlock fix: the first ``import pkg.mod`` in a process kept ``pkg``'s import +lock held until ``pkg/mod.py`` had finished executing, long after ``pkg`` +itself was initialized. Two threads importing submodules of the same packages +for the first time could therefore raise ``_DeadlockError`` or a spurious +"partially initialized module" :exc:`ImportError`, or block one another for +the duration of a submodule's execution, without any circular import being +involved. Parent packages are now imported before the submodule's lock is +taken, and a package's lock is no longer held while its submodules execute. +:func:`importlib.import_module` of a module caught in a genuine cross-thread +circular import now returns the partially initialized module, as the +:keyword:`import` statement already did, instead of raising ``_DeadlockError``. From 74497369597a8119bb77f85b31a36bb5ff7c9741 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Wed, 21 Oct 2026 21:22:50 +0000 Subject: [PATCH 2/7] Simplify the NEWS, What's New and comment wording Address review: shorten the NEWS entry per the updated devguide guidance, reword the What's New entry, and trim the _resolve_parents() docstring and the parent fallback comment in _find_and_load_unlocked(). --- Doc/whatsnew/3.15.rst | 31 ++++++++-------- Lib/importlib/_bootstrap.py | 36 ++++++++----------- .../test_importlib/test_threaded_import.py | 36 +++++++++---------- ...-10-20-22-00-00.gh-issue-159963.pLkHld.rst | 18 ++++------ 4 files changed, 52 insertions(+), 69 deletions(-) diff --git a/Doc/whatsnew/3.15.rst b/Doc/whatsnew/3.15.rst index e182fab96fc4e91..e9e836207ad5ef7 100644 --- a/Doc/whatsnew/3.15.rst +++ b/Doc/whatsnew/3.15.rst @@ -2925,21 +2925,18 @@ Notable changes in 3.15.2 Import system ------------- -* The :gh:`83065` fix shipped in 3.15.0 made the first ``import pkg.mod`` - in a process keep ``pkg``'s import lock held until ``pkg/mod.py`` had - finished executing. Other threads importing anything from ``pkg`` in that - window could block for the whole of ``mod``'s execution, raise - ``_DeadlockError``, or get a spurious "partially initialized module" - :exc:`ImportError`, even without any circular import. Parent packages are - now imported, or waited for if another thread is still initializing them, - *before* the submodule's own lock is taken, and a package's lock is released - as soon as its ``__init__`` returns; the original deadlock remains fixed. - As in 3.15.0, the first import of anything beneath a package waits for that - package's ``__init__`` to finish in whichever thread is running it, so a - package ``__init__`` should not block on another thread that performs such - an import. - :func:`importlib.import_module` of a module caught in a genuine cross-thread - circular import now returns the partially initialized module, as the - :keyword:`import` statement already did, instead of raising - ``_DeadlockError``. +* Parent packages are now imported, or waited for if another thread is still + initializing them, *before* the lock needed to import a submodule is taken. + A package's lock is released as soon as its ``__init__`` returns. + This fixes a potential for deadlock or import errors when two threads are + simultaneously executing nested ``import package.submodule`` imports. + This issue was introduced in 3.15.0 while fixing a *different* import + parallelism lock ordering deadlock in :gh:`83065`. + The first import of anything beneath a package waits for that package's + ``__init__`` to finish in whichever thread is running it, so a package + ``__init__`` should not block on another thread that performs such an + import. + :func:`importlib.import_module` now returns the partially initialized + module in a circular import split across threads, as the :keyword:`import` + statement already did, instead of raising ``_DeadlockError``. (Contributed by Gregory P. Smith in :gh:`159963`.) diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py index 7d5f118ce10fefa..780b367a428ba1b 100644 --- a/Lib/importlib/_bootstrap.py +++ b/Lib/importlib/_bootstrap.py @@ -1201,14 +1201,12 @@ def _find_and_load_unlocked(name, import_, *, lazy_submodule=False): parent = name.rpartition('.')[0] parent_spec = None if parent: - # _resolve_parents() has settled the parent, but nothing pins it in - # sys.modules while this thread waits for name's lock. If it was - # deleted meanwhile, the first import_(parent) below imports it again - # (else sys.modules[parent] raises KeyError) and the second waits for - # another thread doing so, in case its __init__ makes this submodule - # importable (gh-130094). Otherwise the second call returns at once, - # for this thread's own partly initialized parent or a partial parent - # accepted in a circular import split across threads. + # _resolve_parents() has normally imported the parent already. It can + # be missing again by now if its import failed in another thread or + # something removed it from sys.modules; it is then imported afresh, + # like any other module that is not in sys.modules. That import runs + # while holding name's lock, the lock order _resolve_parents() exists + # to avoid; it is tolerated here because the case is so rare. if parent not in sys.modules: _call_with_frames_removed(import_, parent) # Crazy side-effects! @@ -1275,26 +1273,20 @@ def _resolve_parents(name, import_): """Import, or wait for, the parent packages of *name*. _find_and_load() calls this before taking name's module lock, so that no - thread holds a submodule's lock while waiting for its parent: ``import - pkg.mod`` and a pkg/__init__.py importing pkg.mod would then take the two - locks in opposite orders (gh-83065). Each level's lock is released as - soon as that level is settled. Holding a parent's lock through the - submodule's body blocks threads which need only the finished parent, and - causes _DeadlockError and "partially initialized module" errors with no - import cycle (gh-159963). + thread holds a submodule's lock while waiting for its parent. This avoids + the deadlocks and import errors of gh-83065 and gh-159963. """ - # Only the immediate parent has to exist, so ancestors above it are not - # imported here. Wait for those another thread is still executing, - # outermost first: otherwise this thread could enter a chain of imports - # within a package from the far end while that package's __init__ is - # working through it, and one of the two would get a partially initialized - # module. Hence an __init__ must not block on another thread importing - # something beneath it. + # Wait for each ancestor package that another thread is still + # initializing, outermost first. Importing beneath a package while its + # __init__ is part way through its own imports could give one of the two + # threads a partially initialized module. Missing ancestors are not + # imported: only the immediate parent has to exist. parts = name.split('.') for i in range(1, len(parts) - 1): ancestor = '.'.join(parts[:i]) if _is_initializing(sys.modules.get(ancestor)): _lock_unlock_module(ancestor) + # Import the immediate parent, or wait for it likewise. parent = name.rpartition('.')[0] if parent: module = sys.modules.get(parent, _NEEDS_LOADING) diff --git a/Lib/test/test_importlib/test_threaded_import.py b/Lib/test/test_importlib/test_threaded_import.py index c7829205bd6113c..22ba0e18ddff633 100644 --- a/Lib/test/test_importlib/test_threaded_import.py +++ b/Lib/test/test_importlib/test_threaded_import.py @@ -559,8 +559,8 @@ def run(stmt): t2 = threading.Thread(target=run, args=(stmt2,), daemon=True) def hold(): - # One-shot: if a broken interpreter makes thread 1's import fail, - # thread 2 may re-run the same code; don't wait on ourselves. + # One-shot: if thread 1's import fails, thread 2 may re-run the + # same code; it must not wait on itself. if t2.ident is not None: return t2.start() @@ -620,8 +620,7 @@ def test_parent_lock_not_held_while_submodule_executes(self): # gh-159963: the first `import pkg.mod` in a process also imports pkg. # Once pkg/__init__.py has finished, pkg's module lock must not stay # held while mod's body runs; otherwise every thread that began - # waiting for pkg is blocked for the whole of mod's body as well - # (the threaded tests below show what that leads to). + # waiting for pkg is blocked for the whole of mod's body as well. self._setup_package_tree({ "unheld_pkg/__init__.py": "", "unheld_pkg/mod.py": ( @@ -729,9 +728,9 @@ def test_concurrent_first_imports_of_sibling_packages(self): # Thread 2: `import b.leaf`, where b/__init__.py needs a, started # while thread 1 is still inside a/__init__.py. # Dependencies are a.leaf -> b.leaf -> b -> a; `a` needs nothing, so - # there is no cycle and either order works in one thread. This - # failed with _DeadlockError when the importer of a.leaf kept a's - # lock for the duration of a/leaf.py. + # there is no cycle and either order works in one thread. If the + # importer of a.leaf kept a's lock for the duration of a/leaf.py, + # this would raise _DeadlockError. errors = self._run_two_first_imports( { "sib_a/__init__.py": "import sib_sync; sib_sync.hold()\n", @@ -750,8 +749,8 @@ def test_concurrent_first_imports_of_sibling_packages(self): def test_concurrent_first_imports_no_partial_module(self): # Dependencies are a.leaf -> b -> a, again with no cycle. Thread 1 - # finds b in sys.modules and waits for it, so what it got when it - # kept a's lock during a/leaf.py was not _DeadlockError but "cannot + # finds b in sys.modules and waits for it, so if it kept a's lock + # during a/leaf.py it would get not _DeadlockError but "cannot # import name 'X' from partially initialized module 'b'". errors = self._run_two_first_imports( { @@ -795,14 +794,14 @@ def test_finished_package_import_not_blocked_by_submodule_body(self): self.assertEqual(errors, []) def test_concurrent_first_imports_of_nested_packages(self): - # As above but one level down: thread 1 does `import g.sub.leaf`; + # The same one level down: thread 1 does `import g.sub.leaf`; # g/sub/__init__.py needs module c; c needs g. Thread 2 imports c - # while thread 1 is still inside g/__init__.py. No cycle. This - # failed with "cannot import name 'X' from partially initialized - # module" when g's lock was still held by thread 1 while it ran - # g/sub/__init__.py, because thread 2 (holding c, waiting for g) - # then looked deadlocked to thread 1. Releasing ancestor locks only - # just before the *leaf* body would not be enough to fix this one. + # while thread 1 is still inside g/__init__.py. No cycle. If + # thread 1 still held g's lock while it ran g/sub/__init__.py, + # thread 2 (holding c, waiting for g) would look deadlocked to it + # and it would get "cannot import name 'X' from partially + # initialized module". So g's lock must be released before any + # package beneath it executes, not only before the leaf's body. errors = self._run_two_first_imports( { "nest_g/__init__.py": "import nest_sync; nest_sync.hold()\n", @@ -847,7 +846,8 @@ def test_submodule_import_waits_for_initializing_top_package(self): self.assertEqual(sys.modules["topw_g"].X, 1) def test_import_module_of_initializing_submodule_waits_for_it_only(self): - # The flip side of the test above: pkg/__init__.py has *already* + # The converse of waiting for an initializing ancestor before + # loading a submodule: pkg/__init__.py has *already* # imported pkg.leaf and then blocks on thread 2 (any non-import # wait). Thread 2, started while pkg/leaf.py is still executing, # asks for pkg.leaf via importlib. It must get it as soon as leaf.py @@ -872,7 +872,7 @@ def test_import_module_in_cross_thread_cycle_returns_partial_module(self): # X`. Thread 1 is inside x.py (holding ax.x's lock) when thread 2's # `import by.y` reaches y.py and blocks on ax.x. Thread 1 then asks # for by.y and is the one to detect the cycle. The import statement - # has always resolved this by handing thread 1 the partially + # resolves this by handing thread 1 the partially # initialized by.y (import.c -> _lock_unlock_module()); # importlib.import_module() must do the same rather than raise # _DeadlockError into x.py. diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-20-22-00-00.gh-issue-159963.pLkHld.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-20-22-00-00.gh-issue-159963.pLkHld.rst index ae06db342603a77..5c3bd8f63f1d4fb 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-20-22-00-00.gh-issue-159963.pLkHld.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-20-22-00-00.gh-issue-159963.pLkHld.rst @@ -1,12 +1,6 @@ -Fix a regression introduced in Python 3.15.0 by the :gh:`83065` import -deadlock fix: the first ``import pkg.mod`` in a process kept ``pkg``'s import -lock held until ``pkg/mod.py`` had finished executing, long after ``pkg`` -itself was initialized. Two threads importing submodules of the same packages -for the first time could therefore raise ``_DeadlockError`` or a spurious -"partially initialized module" :exc:`ImportError`, or block one another for -the duration of a submodule's execution, without any circular import being -involved. Parent packages are now imported before the submodule's lock is -taken, and a package's lock is no longer held while its submodules execute. -:func:`importlib.import_module` of a module caught in a genuine cross-thread -circular import now returns the partially initialized module, as the -:keyword:`import` statement already did, instead of raising ``_DeadlockError``. +Fix a regression in Python 3.15.0 where threads importing submodules of +packages for the first time could raise ``_DeadlockError`` or a spurious +"partially initialized module" :exc:`ImportError`, or block one another, with +no circular import involved. :func:`importlib.import_module` now returns the +partially initialized module in a cross-thread circular import, as the +:keyword:`import` statement does, instead of raising ``_DeadlockError``. From f4070d175c54ad1120dc5f18863682c5a4a3b22c Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Wed, 21 Oct 2026 22:01:59 +0000 Subject: [PATCH 3/7] drop the What's New entry --- Doc/whatsnew/3.15.rst | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/Doc/whatsnew/3.15.rst b/Doc/whatsnew/3.15.rst index e9e836207ad5ef7..7c4996c5603c6c5 100644 --- a/Doc/whatsnew/3.15.rst +++ b/Doc/whatsnew/3.15.rst @@ -2917,26 +2917,3 @@ that may require changes to your code. Use warning filters to silence them or additional :meth:`!assertWarns*` to catch and check them. (Contributed by Serhiy Storchaka in :gh:`143231`.) - - -Notable changes in 3.15.2 -========================= - -Import system -------------- - -* Parent packages are now imported, or waited for if another thread is still - initializing them, *before* the lock needed to import a submodule is taken. - A package's lock is released as soon as its ``__init__`` returns. - This fixes a potential for deadlock or import errors when two threads are - simultaneously executing nested ``import package.submodule`` imports. - This issue was introduced in 3.15.0 while fixing a *different* import - parallelism lock ordering deadlock in :gh:`83065`. - The first import of anything beneath a package waits for that package's - ``__init__`` to finish in whichever thread is running it, so a package - ``__init__`` should not block on another thread that performs such an - import. - :func:`importlib.import_module` now returns the partially initialized - module in a circular import split across threads, as the :keyword:`import` - statement already did, instead of raising ``_DeadlockError``. - (Contributed by Gregory P. Smith in :gh:`159963`.) From a1269ba83705c877e37e93b79085725c9fa72229 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 22 Oct 2026 21:09:19 +0000 Subject: [PATCH 4/7] Remove two tests that duplicate coverage of others test_concurrent_first_imports_no_partial_module is the depth-one form of test_concurrent_first_imports_of_nested_packages, and test_waiters_recover_after_failed_package_init failed only where test_package_init_imports_submodule_deterministic already does. --- .../test_importlib/test_threaded_import.py | 84 +------------------ 1 file changed, 2 insertions(+), 82 deletions(-) diff --git a/Lib/test/test_importlib/test_threaded_import.py b/Lib/test/test_importlib/test_threaded_import.py index 22ba0e18ddff633..e454d8a8d2f894c 100644 --- a/Lib/test/test_importlib/test_threaded_import.py +++ b/Lib/test/test_importlib/test_threaded_import.py @@ -747,25 +747,6 @@ def test_concurrent_first_imports_of_sibling_packages(self): self.assertEqual(sys.modules["sib_a.leaf"].X, 1) self.assertEqual(sys.modules["sib_b.leaf"].X, 1) - def test_concurrent_first_imports_no_partial_module(self): - # Dependencies are a.leaf -> b -> a, again with no cycle. Thread 1 - # finds b in sys.modules and waits for it, so if it kept a's lock - # during a/leaf.py it would get not _DeadlockError but "cannot - # import name 'X' from partially initialized module 'b'". - errors = self._run_two_first_imports( - { - "part_a/__init__.py": "import part_sync; part_sync.hold()\n", - "part_a/leaf.py": "from part_b import X\n", - "part_b.py": "import part_a\nX = 1\n", - }, - "import part_a.leaf", - "import part_b", - sync_name="part_sync", - t2_waits_for="part_a", - ) - self.assertEqual(errors, []) - self.assertEqual(sys.modules["part_a.leaf"].X, 1) - def test_finished_package_import_not_blocked_by_submodule_body(self): # Thread 2 starts waiting for pkg while pkg/__init__.py runs. It # needs only the finished pkg, so it must be released when that @@ -794,8 +775,8 @@ def test_finished_package_import_not_blocked_by_submodule_body(self): self.assertEqual(errors, []) def test_concurrent_first_imports_of_nested_packages(self): - # The same one level down: thread 1 does `import g.sub.leaf`; - # g/sub/__init__.py needs module c; c needs g. Thread 2 imports c + # Thread 1 does `import g.sub.leaf`; g/sub/__init__.py needs + # module c; c needs g. Thread 2 imports c # while thread 1 is still inside g/__init__.py. No cycle. If # thread 1 still held g's lock while it ran g/sub/__init__.py, # thread 2 (holding c, waiting for g) would look deadlocked to it @@ -957,67 +938,6 @@ def test_waiter_retries_after_failed_submodule_import(self): ]) self.assertEqual(sys.modules[f"{pkg}.mod"].X, 1) - def test_waiters_recover_after_failed_package_init(self): - # pkg/__init__.py imports pkg.a and pkg.b, but its first execution - # (thread 0) raises once two other threads -- importing pkg.a and - # pkg.b -- are already waiting for pkg. One of the waiters then - # re-runs pkg/__init__.py successfully; both waiters must succeed, - # and neither may end up importing the parent while holding its - # submodule's lock (the gh-83065 lock order) on that retry path. - self._setup_package_tree({ - "flaky_pkg/__init__.py": ( - "import flaky_sync; flaky_sync.executed.append(__name__)\n" - "flaky_sync.gate()\n" - "import flaky_pkg.a\n" - "import flaky_pkg.b\n"), - "flaky_pkg/a.py": ( - "import flaky_sync; flaky_sync.executed.append(__name__)\n" - "X = 1\n"), - "flaky_pkg/b.py": ( - "import flaky_sync; flaky_sync.executed.append(__name__)\n" - "import flaky_pkg.a\nX = 1\n"), - }) - sync = types.ModuleType("flaky_sync") - sync.executed = [] - sys.modules["flaky_sync"] = sync - self.addCleanup(sys.modules.pop, "flaky_sync", None) - errors = [] - - def run(stmt): - try: - exec(stmt, {}) - except Exception as e: - errors.append((stmt, f"{type(e).__name__}: {e}")) - - waiters = [threading.Thread(target=run, daemon=True, - args=(f"import flaky_pkg.{m}",)) - for m in ("a", "b")] - first = True - - def gate(): - nonlocal first - if not first: - return - first = False - for t in waiters: - t.start() - for t in waiters: - self._wait_until_blocked_on(t, "flaky_pkg") - raise RuntimeError("first attempt fails") - sync.gate = gate - - t0 = threading.Thread(target=run, args=("import flaky_pkg",), - daemon=True) - t0.start() - self._join_import_threads([t0, *waiters]) - self.assertEqual( - errors, - [("import flaky_pkg", "RuntimeError: first attempt fails")]) - self.assertEqual(sys.modules["flaky_pkg.b"].X, 1) - self.assertEqual( - sync.executed, - ["flaky_pkg", "flaky_pkg", "flaky_pkg.a", "flaky_pkg.b"]) - def setUpModule(): thread_info = threading_helper.threading_setup() From 48734dbc5ce63cb19671cbe48d1e20e30cd33b92 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 22 Oct 2026 21:11:52 +0000 Subject: [PATCH 5/7] Test the import of a parent package removed before its submodule loads --- .../test_importlib/test_threaded_import.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/Lib/test/test_importlib/test_threaded_import.py b/Lib/test/test_importlib/test_threaded_import.py index e454d8a8d2f894c..a33a40db0d522e2 100644 --- a/Lib/test/test_importlib/test_threaded_import.py +++ b/Lib/test/test_importlib/test_threaded_import.py @@ -909,6 +909,30 @@ def stalling_resolve_parents(name, import_): self.assertEqual(errors, []) self.assertEqual(sys.modules["stall_pkg.leaf"].X, 1) + def test_parent_removed_before_submodule_is_loaded(self): + # The parent package leaves sys.modules after it was imported but + # before the submodule is loaded, as when its import failed in + # another thread. The parent must be imported again rather than + # the submodule's import failing with KeyError. + self._setup_package_tree({ + "gone_pkg/__init__.py": "", + "gone_pkg/mod.py": "X = 1\n", + }) + resolve_parents = importlib._bootstrap._resolve_parents + removed = [] + + def removing_resolve_parents(name, import_): + resolve_parents(name, import_) + if name == "gone_pkg.mod": + removed.append(sys.modules.pop("gone_pkg")) + + with support.swap_attr(importlib._bootstrap, "_resolve_parents", + removing_resolve_parents): + import gone_pkg.mod + self.assertEqual(len(removed), 1) + self.assertIsNot(sys.modules["gone_pkg"], removed[0]) + self.assertEqual(sys.modules["gone_pkg"].mod.X, 1) + def test_waiter_retries_after_failed_submodule_import(self): # Thread 2 waits for thread 1's import of pkg.mod, whose first # execution then fails. Thread 2 must load pkg.mod itself and not From 8d8771643aa4a8b22841e95ffc80486b38a92f88 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 22 Oct 2026 23:16:21 +0000 Subject: [PATCH 6/7] Use comments rather than docstrings for internal helpers --- Lib/importlib/_bootstrap.py | 13 ++++++------- Lib/test/test_importlib/test_threaded_import.py | 15 +++++++-------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py index 780b367a428ba1b..b514a3383ec9f1d 100644 --- a/Lib/importlib/_bootstrap.py +++ b/Lib/importlib/_bootstrap.py @@ -1264,18 +1264,17 @@ def _find_and_load_unlocked(name, import_, *, lazy_submodule=False): _NEEDS_LOADING = object() +# Return whether module (a sys.modules value) is still executing. def _is_initializing(module): - """Return whether *module* (a sys.modules value) is still executing.""" return getattr(getattr(module, '__spec__', None), '_initializing', False) +# Import, or wait for, the parent packages of name. +# +# _find_and_load() calls this before taking name's module lock, so that no +# thread holds a submodule's lock while waiting for its parent. This avoids +# the deadlocks and import errors of gh-83065 and gh-159963. def _resolve_parents(name, import_): - """Import, or wait for, the parent packages of *name*. - - _find_and_load() calls this before taking name's module lock, so that no - thread holds a submodule's lock while waiting for its parent. This avoids - the deadlocks and import errors of gh-83065 and gh-159963. - """ # Wait for each ancestor package that another thread is still # initializing, outermost first. Importing beneath a package while its # __init__ is part way through its own imports could give one of the two diff --git a/Lib/test/test_importlib/test_threaded_import.py b/Lib/test/test_importlib/test_threaded_import.py index a33a40db0d522e2..1eff097dea2946d 100644 --- a/Lib/test/test_importlib/test_threaded_import.py +++ b/Lib/test/test_importlib/test_threaded_import.py @@ -531,14 +531,13 @@ def _join_import_threads(self, threads): def _run_two_first_imports(self, files, stmt1, stmt2, *, sync_name, t2_waits_for): - """Thread 1 runs *stmt1*. Something it executes calls - ``.hold()``, which starts thread 2 on *stmt2* and returns - only once thread 2 is blocked on the module lock of *t2_waits_for* - (any module lock if None). Module code may also call - ``.join_t2()`` to wait for thread 2 to finish. - Returns a list of (what, problem) pairs; empty means both imports - succeeded and no module body ran more than once. - """ + # Thread 1 runs stmt1. Something it executes calls + # .hold(), which starts thread 2 on stmt2 and returns + # only once thread 2 is blocked on the module lock of t2_waits_for + # (any module lock if None). Module code may also call + # .join_t2() to wait for thread 2 to finish. + # Returns a list of (what, problem) pairs; empty means both imports + # succeeded and no module body ran more than once. # list.append() is atomic, on free-threaded builds too. count = f"import {sync_name}; {sync_name}.executed.append(__name__)\n" self._setup_package_tree({relpath: count + source From e0982d9b185f8d5a6ed109fdc81cd47025ff14a2 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Fri, 23 Oct 2026 22:59:19 +0000 Subject: [PATCH 7/7] Don't return a module another thread has begun loading again After waiting for a module that another thread was executing, _find_and_load() looked in sys.modules again only after releasing the module's lock. If that import had failed and a thread had already begun retrying it, the waiter returned the new, still executing module. Look while still holding the lock. Reported by Emma Smith in review of GH-159965. --- Lib/importlib/_bootstrap.py | 19 +++++- .../test_importlib/test_threaded_import.py | 66 +++++++++++++++++++ 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py index b514a3383ec9f1d..cf0462f25556e6f 100644 --- a/Lib/importlib/_bootstrap.py +++ b/Lib/importlib/_bootstrap.py @@ -1301,11 +1301,24 @@ def _find_and_load(name, import_, *, lazy_submodule=False): module = sys.modules.get(name, _NEEDS_LOADING) if module is _NEEDS_LOADING or _is_initializing(module): if module is not _NEEDS_LOADING: + # Wait for the thread executing the module. # NOTE: this relies on __spec__._initializing being set *before* # the module is put in sys.modules. - _lock_unlock_module(name) - # The module is gone if the thread loading it failed. - module = sys.modules.get(name, _NEEDS_LOADING) + lock = _get_module_lock(name) + try: + lock.acquire() + except _DeadlockError: + # A circular import split across threads: accept the + # partially initialized module. + pass + else: + # The module is gone if the thread loading it failed. Look + # while still holding the lock, so that another thread cannot + # have begun loading it again. + try: + module = sys.modules.get(name, _NEEDS_LOADING) + finally: + lock.release() if module is _NEEDS_LOADING: sys.audit( diff --git a/Lib/test/test_importlib/test_threaded_import.py b/Lib/test/test_importlib/test_threaded_import.py index 1eff097dea2946d..99f1480c8d72122 100644 --- a/Lib/test/test_importlib/test_threaded_import.py +++ b/Lib/test/test_importlib/test_threaded_import.py @@ -961,6 +961,72 @@ def test_waiter_retries_after_failed_submodule_import(self): ]) self.assertEqual(sys.modules[f"{pkg}.mod"].X, 1) + def test_waiter_not_given_module_being_retried(self): + # Thread B waits for thread A's import of a module, which fails. + # A retries, so sys.modules may hold A's new, still executing + # module by the time B looks there again. B must wait for that + # attempt too rather than return a half initialized module. + self._setup_package_tree({ + "again_mod.py": "import again_sync; again_sync.body()\nX = 1\n", + }) + sync = types.ModuleType("again_sync") + sys.modules["again_sync"] = sync + self.addCleanup(sys.modules.pop, "again_sync", None) + released = threading.Event() # B stopped waiting for the 1st attempt + retrying = threading.Event() # the 2nd attempt is in sys.modules + errors = [] + results = [] + + def a(): + try: + try: + import again_mod + except RuntimeError: + pass + released.wait(support.SHORT_TIMEOUT) + import again_mod + except Exception as e: + errors.append(f"A: {type(e).__name__}: {e}") + + def b(): + try: + module = importlib.import_module("again_mod") + results.append(hasattr(module, "X")) + except Exception as e: + errors.append(f"B: {type(e).__name__}: {e}") + + ta = threading.Thread(target=a, daemon=True) + tb = threading.Thread(target=b, daemon=True) + + def body(): + if tb.ident is None: + tb.start() + self._wait_until_blocked_on(tb, "again_mod") + raise RuntimeError("first attempt fails") + retrying.set() + if threading.current_thread() is not tb: + # Finish only once B is waiting for this attempt. + self._wait_until_blocked_on(tb, "again_mod") + sync.body = body + + # Hold B back, once it has stopped waiting for the first attempt, + # until the second attempt is under way. + release = importlib._bootstrap._ModuleLock.release + + def stalling_release(lock): + release(lock) + if (lock.name == "again_mod" and not released.is_set() + and threading.current_thread() is tb): + released.set() + retrying.wait(support.SHORT_TIMEOUT) + + with support.swap_attr(importlib._bootstrap._ModuleLock, "release", + stalling_release): + ta.start() + self._join_import_threads([ta, tb]) + self.assertEqual(results, [True]) + self.assertEqual(errors, []) + def setUpModule(): thread_info = threading_helper.threading_setup()