-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
bpo-28411: Support other mappings in PyInterpreterState.modules. #3593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
ericsnowcurrently
merged 30 commits into
python:master
from
ericsnowcurrently:sys-modules-any-mapping
Sep 15, 2017
Merged
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
8f3ecfc
Add _PyImport_GetModule*.
ericsnowcurrently 9dc9ded
Allow sys.modules to be any mapping.
ericsnowcurrently f2a54b1
Add _PyImport_SetModule*.
ericsnowcurrently 502e138
Add PyImport_GetModule().
ericsnowcurrently ec8c504
Decref the module when done.
ericsnowcurrently a1546f6
Fix ref counts.
ericsnowcurrently 5a5fad1
Look up "new" modules in the given modules dict.
ericsnowcurrently db0a247
Fix error checking.
ericsnowcurrently 7082b47
Use PyImport_GetModuleDict() in PyImport_GetModule().
ericsnowcurrently 5082c47
Add a missing incref.
ericsnowcurrently bb03872
Add a Misc/NEWS entry.
ericsnowcurrently 2e07296
Make the docs for PyImport_GetModule() more clear.
ericsnowcurrently 4631163
Fix style (bracket placement).
ericsnowcurrently 5b154b3
Drop _PyImport_GetModuleString().
ericsnowcurrently 33174f3
Use PyDict_CheckExact() for sys.modules in fast case.
ericsnowcurrently e50e258
Do not use PyMapping_HasKey() with an error set.
ericsnowcurrently 23cf493
Revert a code order change.
ericsnowcurrently eed5e24
Switch the code order back.
ericsnowcurrently bcc72a5
Only use PyDict_* for dicts.
ericsnowcurrently 981dbd2
Add a reference while cleaning up.
ericsnowcurrently 0fe1aff
Fix _pickle.
ericsnowcurrently d4d9219
Fix the NEWS entry.
ericsnowcurrently 9c39295
Return a borrowed reference from _PyImport_GetModuleWithError().
ericsnowcurrently 446acd5
Fix some ref leaks.
ericsnowcurrently 540b99a
Add a fast path for pickle.
ericsnowcurrently 5f0735b
Ignore errors from PyObject_GetItem().
ericsnowcurrently 63beead
Factor out _checkmodule.
ericsnowcurrently 00f3f24
Drop an extra INCREF.
ericsnowcurrently 5a7bba2
Drop _PyImport_GetModule() and _PyImport_GetModuleWithError().
ericsnowcurrently 3a73b52
tabs to spaces
ericsnowcurrently File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Drop _PyImport_GetModuleString().
- Loading branch information
commit 5b154b3a71998e89c525a6a89186fea9a094f9fe
There are no files selected for viewing
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is missing the Py_XDECREF to return a borrowed reference instead of a new one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gah! The docs for PyDict_GetItemWithError() before 3.7 don't mention it returning a borrowed reference. Thanks for catching that. I was looking for that refleak!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Looks like that wasn't the refleak I was looking for. :/ Back I go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just pattern matching with the code above based on the assumption that
PyDict_GetItem
andPyDict_GetItemWithError
exposed the same refcount semantics :)However, it now occurs to me that these access patterns are thoroughly dubious if you genuinely want to allow dict subclasses and arbitrary mappings, as those may return dynamic references (e.g. from
__missing__
methods) , in which case thePy_XDECREF()
line may invalidate the reference entirely.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I'll fix that.