bpo-1635741: Add PyModule_AddObjectRef() function by vstinner · Pull Request #23122 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation9 Commits2 Checks0 Files changed
Conversation
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
[ Show hidden characters]({{ revealButtonHref }})
Added PyModule_AddObjectRef() function: similar to PyModule_AddObjectRef() but don't steal a reference to the value on success.
@shihai1991: I updated my PR, would you mind to review it again? I completed the documentation (added examples without checking explicitly if the value is NULL) and I clarified the case when value is NULL.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, perferct.
Ubuntu failed with:
test_heading_callback (tkinter.test.test_ttk.test_widgets.TreeviewTest) ... Timeout (0:20:00)!
I re-run the job.
| } |
|---|
| return 0; |
| return PyModule_AddObjectRef(module, name, (PyObject *)type); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for not decref'ing here, as opposed to PyModule_AddStringConstant and PyModule_AddIntConstant? PyModule_AddType used to steal a reference, but now it doesn't, no?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The caller holds a strong reference to type.
Reference counting is hard to get right. That's why I added a new function. Previously, Py_INCREF(type) was needed before calling PyModule_AddObject(). I let you count +1 and -1 on the ref count in all code paths and check manually if my PR doesn't anything (hint: it should not ;-)).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The caller holds a strong reference to type.
Reference counting is hard to get right. That's why I added a new function. Previously, Py_INCREF(type) was needed before calling PyModule_AddObject(). I let you count +1 and -1 on the ref count in all code paths and check manually if my PR doesn't anything (hint: it should not ;-)).
Argh, I though I had it right :) Thanks for the explanation! 🙏🏻
shihai1991 added a commit to shihai1991/cpython that referenced this pull request
This was referenced
Nov 10, 2020
adorilson pushed a commit to adorilson/cpython that referenced this pull request
Added PyModule_AddObjectRef() function: similar to PyModule_AddObjectRef() but don't steal a reference to the value on success.
