-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
bpo-35810: Incref heap-allocated types in PyObject_Init #11661
Changes from all commits
ff4cc4c
fd7eba4
88616bb
9b5ecd6
569c8e8
938ebbf
d404aee
6d55b0a
c8b773a
1649502
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Modify ``PyObject_Init`` to correctly increase the refcount of heap- | ||
allocated Type objects. Also fix the refcounts of the heap-allocated types | ||
that were either doing this manually or not decreasing the type's refcount | ||
in tp_dealloc |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -229,6 +229,9 @@ PyObject_Init(PyObject *op, PyTypeObject *tp) | |
return PyErr_NoMemory(); | ||
/* Any changes should be reflected in PyObject_INIT (objimpl.h) */ | ||
Py_TYPE(op) = tp; | ||
if (PyType_GetFlags(tp) & Py_TPFLAGS_HEAPTYPE) { | ||
Py_INCREF(tp); | ||
} | ||
_Py_NewReference(op); | ||
return op; | ||
} | ||
|
@@ -239,9 +242,8 @@ PyObject_InitVar(PyVarObject *op, PyTypeObject *tp, Py_ssize_t size) | |
if (op == NULL) | ||
return (PyVarObject *) PyErr_NoMemory(); | ||
/* Any changes should be reflected in PyObject_INIT_VAR */ | ||
op->ob_size = size; | ||
Py_TYPE(op) = tp; | ||
_Py_NewReference((PyObject *)op); | ||
Py_SIZE(op) = size; | ||
PyObject_Init((PyObject *)op, tp); | ||
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. Shouldn't this just call 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. Or, actually, shouldn't 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.
As for code duplication, that's a valid point, but I'd call that out of scope for this PR. Let's keep orthogonal issues revertable separately. |
||
return op; | ||
} | ||
|
||
|
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.
In Cython, we normally use the exact alpha/beta version where the change was introduced (to support testing during the CPython release cycle), but we obviously don't know that yet.
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.
The example is not really written for projects with Cython's level of involvement. Most projects will only test against a nightly build or the latest alpha/beta (if even that). The few others should know to adjust the example. We don't explicitly tell you to rename
foo_new
, either :)