Repository navigation
GH-83065: Fix import deadlock by implementing hierarchical module locking - #137196
Merged
Merged
Conversation
…le locking Fix a deadlock in the import system that occurs when concurrent threads import modules at different levels of the same package hierarchy while `__init__.py` files have circular imports. The deadlock scenario (correctly analyzed by @emmatyping): - Thread 1 imports `package.subpackage`, which imports `package.subpackage.module` in its `__init__.py` - Thread 2 imports `package.subpackage.module`, which needs to ensure `package.subpackage` is loaded first - Each thread holds a lock the other needs, causing deadlock The fix introduces _HierarchicalLockManager that acquires all necessary module locks in a consistent order (parent before child) for nested modules. This ensures all threads acquire locks in the same order, preventing circular wait conditions. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
gpshead
commented
Jul 29, 2025
Member
|
I don't feel qualified to say this PR is correct, as import logic has a lot of nasty reentrant behavior I haven't dove into, but the PR does look right, based on what I imagined an implementation of a solution might be. |
|
This PR is stale because it has been open for 30 days with no activity. |
Resolved conflict in Lib/test/test_importlib/test_threaded_import.py: both branches added a new test method at the same location. Kept both test_import_failure_race_condition (from upstream) and test_hierarchical_import_deadlock (from this PR).
…partial parent on cycle If lock.acquire() raises (e.g. _DeadlockError or KeyboardInterrupt) after some chain locks are already held, __exit__ is never called and the held locks leak permanently. Wrap the loop so any exception releases what was acquired before re-raising. When acquiring a parent's lock would deadlock with another thread that is loading that parent, skip the parent and proceed - the same accept-a- partial-parent policy that _lock_unlock_module() applies. Without this, concurrent cross-package circular imports that previously succeeded via that path now raise _DeadlockError to user code. The leaf's lock is still required; a deadlock there propagates as before. Also lift _get_module_chain to a module-level function as it does not use self.
…rcular test test_hierarchical_import_deadlock: synchronise thread start with a Barrier; assert errors == [] (the prior isinstance(error, ImportError) check never matched _DeadlockError, a RuntimeError); record exception type and message rather than the bare object so failures self-describe. test_cross_package_circular_import: new. Two packages whose __init__.py each import a submodule of the other; concurrent imports of further submodules must not raise _DeadlockError.
gpshead
marked this pull request as ready for review
April 28, 2026 06:16
gpshead
requested review from
AA-Turner,
FFY00,
brettcannon,
ericsnowcurrently,
ncoghlan and
warsaw
as code owners
April 28, 2026 06:16
gpshead
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @gpshead for commit b3d4028 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F137196%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
3 tasks
|
Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
|
Sorry, @gpshead, I could not cleanly backport this to Please backport manually with cherry_picker, see the devguide for more information. |
Member
Author
|
okay so it'll require more effort to backport.. had to try; this is a large change. not sure we'd actually want to put it in a patch release. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When one thread imports
pkg.sub.modand another thread importspkg.suband
pkg/sub/__init__.pyitself importspkg.sub.mod, the two threads canacquire the per-module locks in opposite orders and trip the import system's
deadlock detector, raising
_DeadlockErrorto user code.This change makes
_find_and_load()acquire the module locks for the fulldotted-name chain (parent before child) when loading a nested module, so
both threads contend on the same first lock and serialise instead of
deadlocking.
When acquiring a parent's lock would itself deadlock with another thread
that is loading that parent (cross-package circular imports), the parent's
lock is skipped and the partially-initialised parent is accepted -- the
same policy
_lock_unlock_module()already applies on the existing codepath -- so concurrent circular imports that worked before continue to work.
A second new test covers that cross-package case.
The hot path (the module is already fully loaded) does not reach the new
locking code at all.
Fixes #83065