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-91351: Fix some bugs in importlib handling of re-entrant imports by exarkun · Pull Request #94504 · python/cpython Conversation
Contributor
See #91351 for details about the problem. Re-entrancy is always tricky and given the requirements of _bootstrap.py (to operate with re-entrancy and multi-threading and to do so without exposing any of the details to application code doing an import) I think this goes double. This PR does a few things to achieve better safety in the face of re-entrancy:
I'm not quite sure I believe this new version of the code is 100% correct with respect to re-entrancy but it does fixes mishandling of two specific cases:
This PR also does not include any new unit tests. I have a small stand-alone program which can reproduce both of these but only with the assistance of some additional instrumentation inside Compared to the previous PR, this one is a bit simpler because it uses For reference, here is a stand-alone reproducer. This one very reliably reproduces the
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactionsThis might make it a little bit easier for a new reader/maintainer to understand what this code is doing. Also, link to a couple application-level bug reports about probable misbehaviors related to the deadlock-detection code. This more clearly separates the logic for that management from the application code that runs in this context. It will also make subsequent changes to improve that logic more clear. Also update the deadlock detection to work with the new _blocking_on Switch to a recursive implementation so it can easily follow the branching path through the "blocking on" graph that is now possible thanks to re-entrancy.
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
brettcannon
requested changes
Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page.
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. Co-authored-by: Brett Cannon <[email protected]> Also rename it as suggested by review Co-authored-by: Brett Cannon <[email protected]>
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
brettcannon
approved these changes
Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page.
brettcannon
requested changes
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would have approved and made the commits myself, but the branch is still blocked from me making changes. 😅 Can you just make the last couple and then I can merge this!
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. Co-authored-by: Brett Cannon <[email protected]>
Contributor
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
brettcannon
approved these changes
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
This was referenced
facebook-github-bot
pushed a commit
to facebookincubator/MetaPython
that referenced
this pull request
…imports Summary: upstream issue: python/cpython#91351 upstream PR: python/cpython#94504 upstream merge commit: python/cpython@3325f05 symptom: ``` File "<frozen importlib._bootstrap>", line 1004, in _find_and_load(name='oe.gpg_sign', import_=<built-in function __import__>) File "<frozen importlib._bootstrap>", line 158, in _ModuleLockManager.__enter__() File "<frozen importlib._bootstrap>", line 110, in _ModuleLock.acquire() KeyError: 139622474778432 and File "<frozen importlib._bootstrap>", line 1004, in _find_and_load(name='oe.path', import_=<built-in function __import__>) File "<frozen importlib._bootstrap>", line 158, in _ModuleLockManager.__enter__() File "<frozen importlib._bootstrap>", line 110, in _ModuleLock.acquire() KeyError: 140438942700992 ``` Reviewed By: carljm Differential Revision: D53641441 fbshipit-source-id: e142eb17442da370861cd3a3398b0eef9930d041
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ReviewersUh oh!
There was an error while loading. Please reload this page. AssigneesNo one assignedLabels
None yet
ProjectsMilestoneNo milestoneDevelopmentSuccessfully merging this pull request may close these issues. importlib lock race issue in deadlock handling codeUh oh!
There was an error while loading. Please reload this page. 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. | ||||||||
| You are visitor 01402520 — GeezerVison does not store the pages you look up. Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required. |