8000 bpo-28411: Support other mappings in PyInterpreterState.modules. by ericsnowcurrently · Pull Request #3593 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

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.

Sign up for GitHub

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
merged 30 commits into from
Sep 15, 2017
Merged

bpo-28411: Support other mappings in PyInterpreterState.modules. #3593

Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
8f3ecfc
Add _PyImport_GetModule*.
ericsnowcurrently May 18, 2017
9dc9ded
Allow sys.modules to be any mapping.
ericsnowcurrently May 18, 2017
f2a54b1
Add _PyImport_SetModule*.
ericsnowcurrently May 18, 2017
502e138
Add PyImport_GetModule().
ericsnowcurrently May 18, 2017
ec8c504
Decref the module when done.
ericsnowcurrently May 25, 2017
a1546f6
Fix ref counts.
ericsnowcurrently May 18, 2017
5a5fad1
Look up "new" modules in the given modules dict.
ericsnowcurrently May 18, 2017
db0a247
Fix error checking.
ericsnowcurrently May 20, 2017
7082b47
Use PyImport_GetModuleDict() in PyImport_GetModule().
ericsnowcurrently May 23, 2017
5082c47
Add a missing incref.
ericsnowcurrently May 25, 2017
bb03872
Add a Misc/NEWS entry.
ericsnowcurrently Sep 4, 2017
2e07296
Make the docs for PyImport_GetModule() more clear.
ericsnowcurrently Sep 4, 2017
4631163
Fix style (bracket placement).
ericsnowcurrently Sep 4, 2017
5b154b3
Drop _PyImport_GetModuleString().
ericsnowcurrently Sep 4, 2017
33174f3
Use PyDict_CheckExact() for sys.modules in fast case.
ericsnowcurrently Sep 4, 2017
e50e258
Do not use PyMapping_HasKey() with an error set.
ericsnowcurrently Sep 4, 2017
23cf493
Revert a code order change.
ericsnowcurrently Sep 4, 2017
eed5e24
Switch the code order back.
ericsnowcurrently Sep 4, 2017
bcc72a5 8000
Only use PyDict_* for dicts.
ericsnowcurrently Sep 12, 2017
981dbd2
Add a reference while cleaning up.
ericsnowcurrently Sep 12, 2017
0fe1aff
Fix _pickle.
ericsnowcurrently Sep 14, 2017
d4d9219
Fix the NEWS entry.
ericsnowcurrently Sep 14, 2017
9c39295
Return a borrowed reference from _PyImport_GetModuleWithError().
ericsnowcurrently Sep 15, 2017
446acd5
Fix some ref leaks.
ericsnowcurrently Sep 15, 2017
540b99a
Add a fast path for pickle.
ericsnowcurrently Sep 15, 2017
5f0735b
Ignore errors from PyObject_GetItem().
ericsnowcurrently Sep 15, 2017
63beead
Factor out _checkmodule.
ericsnowcurrently Sep 15, 2017
00f3f24
Drop an extra INCREF.
ericsnowcurrently Sep 15, 2017
5a7bba2
Drop _PyImport_GetModule() and _PyImport_GetModuleWithError().
ericsnowcurrently Sep 15, 2017
3a73b52
tabs to spaces
ericsnowcurrently Sep 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8000
Prev Previous commit
Next Next commit
Return a borrowed reference from _PyImport_GetModuleWithError().
  • Loading branch information
@ericsnowcurrently
ericsnowcurrently committed Sep 15, 2017
commit 9c3929574c70f349c0d1029700a8e704064bd762
3 changes: 3 additions & 0 deletions Python/import.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ _PyImport_GetModule(PyObject *name)
if (PyErr_Occurred()) {
PyErr_Clear();
}
/* Return a borrowed reference instead of a new one. */
Py_XDECREF(mod);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could do with a second comment saying "Return a borrowed reference instead of a new one"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

return mod;
}
Expand All @@ -340,6 +341,8 @@ _PyImport_GetModuleWithError(PyObject *name)
if (PyErr_ExceptionMatches(PyExc_KeyError)) {
PyErr_Clear();
}
/* Return a borrowed reference instead of a new one. */
Py_XDECREF(mod);
return mod;
Copy link
Contributor

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.

Copy link
Member Author

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!

Copy link
Member Author

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.

Copy link
Contributor

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 and PyDict_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 the Py_XDECREF() line may invalidate the reference entirely.

Copy link
Member Author

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.

}

Expand Down
0