GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||
GeezerVison is showing github.com — GH-83065: Fix import deadlock by implementing hierarchical module locking by gpshead · Pull Request #137196 · python/cpython Conversation
|
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This PR is stale because it has been open for 30 days with no activity. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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.
|
🤖 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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sorry, @gpshead, I could not cleanly backport this to Please backport manually with cherry_picker, see the devguide for more information. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
__import__ is not thread-safe on Python 3
There was an error while loading. Please reload this page.