-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
bpo-39824: module_traverse() don't call m_traverse if md_state=NULL #18738
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18738 +/- ##
===========================================
+ Coverage 82.14% 83.22% +1.08%
===========================================
Files 1956 1571 -385
Lines 590140 415393 -174747
Branches 44488 44489 +1
===========================================
- Hits 484743 345715 -139028
+ Misses 95745 60024 -35721
- Partials 9652 9654 +2
Continue to review full report at Codecov.
|
shihai1991
left a comment
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.
Thanks, LGTM.
Doc/c-api/module.rst
Outdated
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.
IMO this needs clarification.
If a module has m_size of 0 or -1, so the state is not allocated, then m_traverse can be called before the Py_mod_exec function. Right?
| This function is not called before the module state is allocated (before | |
| the :c:member:`Py_mod_exec` function is executed): it is not called if | |
| :c:member:`m_size` is greater than 0 and module state | |
| (:c:func:`PyModule_GetState`) is ``NULL``. | |
| .. versionchanged:: 3.9 | |
| It is no longer called before the module state is allocated. | |
| This function is not called if module state was requested but is not allocated. | |
| (This is the case immediately after the module is created, before the | |
| :c:member:`Py_mod_exec` function is executed.) | |
| More precisely, this function is not called if :c:member:`m_size` is positive and | |
| module state (as returned by :c:func:`PyModule_GetState`) is ``NULL``. | |
| .. versionchanged:: 3.9 | |
| No longer called before the module state is allocated.``` |
|
@encukou: Thanks for your suggestion. I applied it with minor edits. Would you mind to review the updated PR? Sorry, I chose to amend the PR rather than add a second commit, to be able to edit the commit message as well (and the NEWS entry). |
|
cc @pablogsal @Dormouse759 |
Extension modules: m_traverse, m_clear and m_free functions of PyModuleDef are no longer called if the module state was requested but is not allocated yet. This is the case immediately after the module is created and before the module is executed (Py_mod_exec function). More precisely, this function is not called if m_size is greater than 0 and the module state (as returned by PyModule_GetState()) is NULL. Extension modules without module state (m_size <= 0) are not affected.
|
I added an entry to What's New In Python 3.9 to advice extension module maintainers to keep an eye on this corner case. I rebased the PR on master to fix conflicts. |
Misc/NEWS.d/next/C API/2020-03-02-11-29-45.bpo-39824.71_ZMn.rst
Outdated
Show resolved
Hide resolved
encukou
left a comment
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.
LGTM, except some grammar in the docs!
Co-Authored-By: Petr Viktorin <[email protected]>
Co-Authored-By: Petr Viktorin <[email protected]>
Extension modules: m_traverse, m_clear and m_free functions of
PyModuleDef are no longer called before the module state is
allocated. Extension modules with no module state (m_size <= 0) are
no affected.
https://bugs.python.org/issue39824