Repository navigation
gh-159963: Don't hold parent package import locks while a submodule executes - #159965
gpshead wants to merge 8 commits into
Conversation
Sorry, something went wrong.
…dule executes Since pythongh-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 pythongh-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.
Documentation build overview45 files changed · + 1 added · ± 44 modified + Added ± Modified |
Sorry, something went wrong.
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().
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.
|
@emmatyping volunteered as review tribute at the sprint 😅 |
Sorry, something went wrong.
| # 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) |
There was a problem hiding this comment.
I had Astra review and it found the following issue which I believe is valid. While you should absolutely not do this, we should probably ensure that import locking is safe.
_lock_unlock_module(name) releases the lock before the subsequent sys.modules.get(). This permits:
- Thread A initializes a module; thread B waits for it.
- A’s import fails and removes the module.
- B acquires and releases the lock.
- Thread C starts retrying the import, inserting a new, initializing module.
- B reads C’s module and returns it without waiting.
There is no circular import here. importlib.import_module() nevertheless returns an incomplete module, potentially causing missing-attribute errors.
Sorry, something went wrong.
There was a problem hiding this comment.
Good catch, this is valid, and it was introduced by this PR. main and 3.14 re-check sys.modules while still holding the module's lock; I had moved that look to after _lock_unlock_module() released it.
Fixed in e0982d9: the waiter now looks in sys.modules before releasing the lock, so it either sees a finished module or finds it gone and takes the normal load path, which re-checks under the lock.
test_waiter_not_given_module_being_retried forces the interleaving you describe by stalling the waiter right after it releases the lock. It fails on the previous commit, where import_module() returns the retry's half-initialized module, and passes with the fix. Unforced, I couldn't hit the window in 400 runs, so it's narrow, but agreed that import locking should be safe regardless.
Sorry, something went wrong.
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 pythonGH-159965.
Since gh-83065, the first import of pkg.mod kept pkg's module lock until pkg/mod.py had finished executing. This shipped in 3.15.0. 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. This part is not the 3.15.0 regression: import_module() has raised _DeadlockError there since at least 3.12 (3.12, 3.13, 3.14 and main all do), so it can be judged separately for the backport.
Two threads, step by step
The scenario from test_concurrent_first_imports_of_sibling_packages: nothing is imported yet, a/leaf.py does from b.leaf import X and b/__init__.py does import a. The dependencies are a.leaf → b.leaf → b → a, so there is no import cycle. Thread 2 starts while thread 1 is still inside a/__init__.py.
sequenceDiagram participant T1 as Thread 1<br/>import a.leaf participant T2 as Thread 2<br/>import b.leaf Note over T1: 1. acquire locks a and a.leaf<br/>run a/__init__.py Note over T2: 2. acquire locks b and b.leaf<br/>run b/__init__.py (import a) T2-->>T1: 3. wait for lock a Note over T1: 4. a/__init__.py returns<br/>lock a is NOT released<br/>run a/leaf.py (from b.leaf import X) T1--xT2: 5. needs lock b.leaf, but T2 is waiting on T1 Note over T1: 6. _DeadlockErrorsequenceDiagram participant T1 as Thread 1<br/>import a.leaf participant T2 as Thread 2<br/>import b.leaf Note over T1: 1. acquire lock a<br/>run a/__init__.py Note over T2: 2. acquire lock b<br/>run b/__init__.py (import a) T2-->>T1: 3. wait for lock a Note over T1: 4. a/__init__.py returns<br/>release lock a Note over T1: 5. acquire lock a.leaf<br/>run a/leaf.py (from b.leaf import X) Note over T2: 6. a is ready, b/__init__.py returns, release lock b<br/>acquire lock b.leaf, run b/leaf.py T1-->>T2: 7. wait for lock b.leaf Note over T2: 8. release lock b.leaf<br/>import b.leaf done Note over T1: 9. X imported, release lock a.leaf<br/>import a.leaf doneBefore, thread 1 keeps lock a after a/__init__.py has returned, so thread 2, which needs only the finished a, stays blocked while holding the locks thread 1 needs next. After, each thread holds one module lock at a time and both imports complete. Steps 5 to 8 of the right-hand side can interleave differently (thread 1 may load b.leaf itself); either way neither thread waits on a lock while holding one the other needs.
authored using Claude Fable