From ff4cc4c338dcfc7f99bdafd3a14af961a317967c Mon Sep 17 00:00:00 2001 From: eduardo-elizondo Date: Wed, 23 Jan 2019 12:34:32 -0800 Subject: [PATCH 1/9] Add incref to heap-allocated type objects --- Include/objimpl.h | 2 ++ Modules/_curses_panel.c | 2 ++ Modules/_tkinter.c | 3 --- Objects/object.c | 6 ++++-- Objects/typeobject.c | 3 --- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Include/objimpl.h b/Include/objimpl.h index f475ed001d02f8..fb559e1bf1fac5 100644 --- a/Include/objimpl.h +++ b/Include/objimpl.h @@ -138,6 +138,8 @@ _PyObject_INIT(PyObject *op, PyTypeObject *typeobj) { assert(op != NULL); Py_TYPE(op) = typeobj; + if (PyType_GetFlags(typeobj) & Py_TPFLAGS_HEAPTYPE) + Py_INCREF(typeobj); _Py_NewReference(op); return op; } diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 609718f65f15ab..1c0628edbf9cf8 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -250,6 +250,7 @@ PyCursesPanel_New(PANEL *pan, PyCursesWindowObject *wo) static void PyCursesPanel_Dealloc(PyCursesPanelObject *po) { + PyObject *tp = (PyObject *) Py_TYPE(po); PyObject *obj = (PyObject *) panel_userptr(po->pan); if (obj) { (void)set_panel_userptr(po->pan, NULL); @@ -261,6 +262,7 @@ PyCursesPanel_Dealloc(PyCursesPanelObject *po) remove_lop(po); } PyObject_DEL(po); + Py_DECREF(tp); } /* panel_above(NULL) returns the bottom panel in the stack. To get diff --git a/Modules/_tkinter.c b/Modules/_tkinter.c index a96924c9c6e71d..613a95b0897255 100644 --- a/Modules/_tkinter.c +++ b/Modules/_tkinter.c @@ -617,7 +617,6 @@ Tkapp_New(const char *screenName, const char *className, v = PyObject_New(TkappObject, (PyTypeObject *) Tkapp_Type); if (v == NULL) return NULL; - Py_INCREF(Tkapp_Type); v->interp = Tcl_CreateInterp(); v->wantobjects = wantobjects; @@ -802,7 +801,6 @@ newPyTclObject(Tcl_Obj *arg) self = PyObject_New(PyTclObject, (PyTypeObject *) PyTclObject_Type); if (self == NULL) return NULL; - Py_INCREF(PyTclObject_Type); Tcl_IncrRefCount(arg); self->value = arg; self->string = NULL; @@ -2722,7 +2720,6 @@ Tktt_New(PyObject *func) v = PyObject_New(TkttObject, (PyTypeObject *) Tktt_Type); if (v == NULL) return NULL; - Py_INCREF(Tktt_Type); Py_INCREF(func); v->token = NULL; diff --git a/Objects/object.c b/Objects/object.c index 6c2bd7717c01ce..7416d06d850d50 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -229,6 +229,8 @@ 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,8 +241,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_SIZE(op) = size; + PyObject_Init((PyObject *)op, tp); _Py_NewReference((PyObject *)op); return op; } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ace45ba5794ba4..abe92a4fb6e760 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -981,9 +981,6 @@ PyType_GenericAlloc(PyTypeObject *type, Py_ssize_t nitems) memset(obj, '\0', size); - if (type->tp_flags & Py_TPFLAGS_HEAPTYPE) - Py_INCREF(type); - if (type->tp_itemsize == 0) (void)PyObject_INIT(obj, type); else From fd7eba4c768c0aa9537e531c52ad2f8f675f13ba Mon Sep 17 00:00:00 2001 From: eduardo-elizondo Date: Wed, 23 Jan 2019 12:38:20 -0800 Subject: [PATCH 2/9] Added NEWS --- .../next/C API/2019-01-23-12-38-11.bpo-35810.wpbWeb.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/C API/2019-01-23-12-38-11.bpo-35810.wpbWeb.rst diff --git a/Misc/NEWS.d/next/C API/2019-01-23-12-38-11.bpo-35810.wpbWeb.rst b/Misc/NEWS.d/next/C API/2019-01-23-12-38-11.bpo-35810.wpbWeb.rst new file mode 100644 index 00000000000000..720a6e475333d0 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2019-01-23-12-38-11.bpo-35810.wpbWeb.rst @@ -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_decref From 88616bbb3d4cc82213d812771643a443dfd7fe70 Mon Sep 17 00:00:00 2001 From: Eddie Elizondo Date: Wed, 23 Jan 2019 13:06:04 -0800 Subject: [PATCH 3/9] Nits and fix wrong copypasta --- Include/objimpl.h | 3 ++- Objects/object.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Include/objimpl.h b/Include/objimpl.h index fb559e1bf1fac5..2337d8a56c774e 100644 --- a/Include/objimpl.h +++ b/Include/objimpl.h @@ -138,8 +138,9 @@ _PyObject_INIT(PyObject *op, PyTypeObject *typeobj) { assert(op != NULL); Py_TYPE(op) = typeobj; - if (PyType_GetFlags(typeobj) & Py_TPFLAGS_HEAPTYPE) + if (PyType_GetFlags(typeobj) & Py_TPFLAGS_HEAPTYPE) { Py_INCREF(typeobj); + } _Py_NewReference(op); return op; } diff --git a/Objects/object.c b/Objects/object.c index 7416d06d850d50..fbb137080121cf 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -229,8 +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) + if (PyType_GetFlags(tp) & Py_TPFLAGS_HEAPTYPE) { Py_INCREF(tp); + } _Py_NewReference(op); return op; } @@ -243,7 +244,6 @@ PyObject_InitVar(PyVarObject *op, PyTypeObject *tp, Py_ssize_t size) /* Any changes should be reflected in PyObject_INIT_VAR */ Py_SIZE(op) = size; PyObject_Init((PyObject *)op, tp); - _Py_NewReference((PyObject *)op); return op; } From 9b5ecd64f3a5d767a3a7a3e015ad561ab910bd76 Mon Sep 17 00:00:00 2001 From: Eddie Elizondo Date: Tue, 5 Feb 2019 16:19:42 -0800 Subject: [PATCH 4/9] Update structseq and whatsnew --- Doc/whatsnew/3.8.rst | 14 ++++++++++++++ Modules/_curses_panel.c | 6 ++++-- Objects/structseq.c | 5 +++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 0360be604f5699..54f4a7e6e32e27 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -347,6 +347,20 @@ Build and C API Changes (Contributed by Antoine Pitrou in :issue:`32430`.) +* Heap-allocated Types will now incref through :c:func:`PyObject_Init` (and + its parallel macro PyObject_INIT) instead of :c:func:`PyType_GenericAlloc`. + + All instance allocation C APIs that call + :c:func:`PyObject_Init` will incref the Type accordingly. This + includes: :c:func:`PyObject_New`, :c:func:`PyObject_NewVar`, + :c:func:`PyObject_GC_New`, :c:func:`PyObject_GC_NewVar`, and all their + corresponding macros. + + To prevent leaks tp_dealloc has be updated to decref the Type if it's not + already doing so. + + (Contributed by Eddie Elizondo in :issue:`35810`.) + Deprecated ========== diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 1c0628edbf9cf8..70c41d79982eed 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -250,8 +250,10 @@ PyCursesPanel_New(PANEL *pan, PyCursesWindowObject *wo) static void PyCursesPanel_Dealloc(PyCursesPanelObject *po) { - PyObject *tp = (PyObject *) Py_TYPE(po); - PyObject *obj = (PyObject *) panel_userptr(po->pan); + PyObject *tp, *obj; + + tp = (PyObject *) Py_TYPE(po); + obj = (PyObject *) panel_userptr(po->pan); if (obj) { (void)set_panel_userptr(po->pan, NULL); Py_DECREF(obj); diff --git a/Objects/structseq.c b/Objects/structseq.c index cf94155f18f833..a83e9c013be5dc 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -62,12 +62,17 @@ static void structseq_dealloc(PyStructSequence *obj) { Py_ssize_t i, size; + PyTypeObject *tp; + tp = (PyTypeObject *) Py_TYPE(obj); size = REAL_SIZE(obj); for (i = 0; i < size; ++i) { Py_XDECREF(obj->ob_item[i]); } PyObject_GC_Del(obj); + if (PyType_GetFlags(tp) & Py_TPFLAGS_HEAPTYPE) { + Py_DECREF(tp); + } } /*[clinic input] From 569c8e8cb7cf1f32a4686900313c9becbbc7eec8 Mon Sep 17 00:00:00 2001 From: Eddie Elizondo Date: Tue, 5 Feb 2019 16:42:16 -0800 Subject: [PATCH 5/9] Improve whatsnew wording --- Doc/whatsnew/3.8.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 54f4a7e6e32e27..2746527e4625f1 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -356,8 +356,8 @@ Build and C API Changes :c:func:`PyObject_GC_New`, :c:func:`PyObject_GC_NewVar`, and all their corresponding macros. - To prevent leaks tp_dealloc has be updated to decref the Type if it's not - already doing so. + If the Type has a custom tp_dealloc, it has to be updated to decref the Type + if it's not already doing so. (Contributed by Eddie Elizondo in :issue:`35810`.) From 938ebbf7b3ca06404412ef17b6af8027b3471a3e Mon Sep 17 00:00:00 2001 From: Eddie Elizondo Date: Thu, 14 Feb 2019 10:25:23 -0800 Subject: [PATCH 6/9] Add a Changes in the C API section --- Doc/whatsnew/3.8.rst | 57 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 2746527e4625f1..65cb8fca0c9407 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -350,15 +350,6 @@ Build and C API Changes * Heap-allocated Types will now incref through :c:func:`PyObject_Init` (and its parallel macro PyObject_INIT) instead of :c:func:`PyType_GenericAlloc`. - All instance allocation C APIs that call - :c:func:`PyObject_Init` will incref the Type accordingly. This - includes: :c:func:`PyObject_New`, :c:func:`PyObject_NewVar`, - :c:func:`PyObject_GC_New`, :c:func:`PyObject_GC_NewVar`, and all their - corresponding macros. - - If the Type has a custom tp_dealloc, it has to be updated to decref the Type - if it's not already doing so. - (Contributed by Eddie Elizondo in :issue:`35810`.) @@ -537,6 +528,54 @@ Changes in the Python API (Contributed by Xiang Zhang in :issue:`33106`.) +Changes in the C API +-------------------------- + +The incref to a heap-allocated type has been moved from +:c:func:`PyType_GenericAlloc` to :c:func:`PyObject_Init` and +:c:func:`PyObject_INIT`. This now makes types created through +:c:func:`PyType_FromSpec` behave like any other class in managed code. + +For the vast mojority of cases, there should be no side effect. However, this +might cause some types to become immortal. To correctly port these types into +3.8, please apply the following changes: + +* Remove any manual incref on the itself after allocating an instance. This + usually happens after calling :c:func:`PyObject_New`, + :c:func:`PyObject_NewVar`, :c:func:`PyObject_GC_New`, + :c:func:`PyObject_GC_NewVar`, or any other custom allocator that uses + :c:func:`PyObject_Init` or :c:func:`PyObject_INIT`. + +Example:: + + static foo_struct * + foo_new(PyObject *type) { + foo_struct *foo = PyObject_GC_New(foo_struct, (PyTypeObject *) type); + if (foo == NULL) + return NULL; + #if PY_VERSION_HEX < 0x03080000 + PY_INCREF(type) + #endif + return foo; + } + +* Add a decref to the type in any custom tp_dealloc. The type should always + decref at deallocation time. + +Example:: + + static void + foo_dealloc(foo_struct *instance) { + PyObject *type = Py_TYPE(instance); + PyObject_GC_Del(instance); + #if PY_VERSION_HEX >= 0x03080000 + Py_DECREF(type); + #endif + } + +(Contributed by Eddie Elizondo in :issue:`35810`.) + + CPython bytecode changes ------------------------ From d404aeea4548ecdefcb2247412352d4e0de8a6bf Mon Sep 17 00:00:00 2001 From: Eddie Elizondo Date: Thu, 14 Feb 2019 10:28:31 -0800 Subject: [PATCH 7/9] Nits Fix Typo --- .../next/C API/2019-01-23-12-38-11.bpo-35810.wpbWeb.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/C API/2019-01-23-12-38-11.bpo-35810.wpbWeb.rst b/Misc/NEWS.d/next/C API/2019-01-23-12-38-11.bpo-35810.wpbWeb.rst index 720a6e475333d0..47d25a5924c700 100644 --- a/Misc/NEWS.d/next/C API/2019-01-23-12-38-11.bpo-35810.wpbWeb.rst +++ b/Misc/NEWS.d/next/C API/2019-01-23-12-38-11.bpo-35810.wpbWeb.rst @@ -1,4 +1,4 @@ Modify ``PyObject_Init`` to correctly increase the refcount of heap- -allocated Type objects. Also fix the refcounts of the heap-allocated Types +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_decref +in tp_dealloc From 6d55b0aae35241c4d9a579875311faada2fd2ea1 Mon Sep 17 00:00:00 2001 From: Eddie Elizondo Date: Wed, 20 Feb 2019 01:41:48 -0800 Subject: [PATCH 8/9] Improve wording --- Doc/whatsnew/3.8.rst | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 65cb8fca0c9407..325315def97038 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -536,15 +536,17 @@ The incref to a heap-allocated type has been moved from :c:func:`PyObject_INIT`. This now makes types created through :c:func:`PyType_FromSpec` behave like any other class in managed code. -For the vast mojority of cases, there should be no side effect. However, this -might cause some types to become immortal. To correctly port these types into -3.8, please apply the following changes: - -* Remove any manual incref on the itself after allocating an instance. This - usually happens after calling :c:func:`PyObject_New`, - :c:func:`PyObject_NewVar`, :c:func:`PyObject_GC_New`, - :c:func:`PyObject_GC_NewVar`, or any other custom allocator that uses - :c:func:`PyObject_Init` or :c:func:`PyObject_INIT`. +For the vast majority of cases, there should be no side effect. However, this +causes types that are manually incref after allocating an instance to become +immortal. Furthermore, the incref guarantee should now be reflected by having +a decref during instance deallocation. + +To correctly port these types into 3.8, please apply the following changes: + +* Remove any manual incref on the itself after allocating an instance - if any. + This happens after calling :c:func:`PyObject_New`, :c:func:`PyObject_NewVar`, + :c:func:`PyObject_GC_New`, :c:func:`PyObject_GC_NewVar`, or any other custom + allocator that uses :c:func:`PyObject_Init` or :c:func:`PyObject_INIT`. Example:: From 1649502fa84cb71582a74cc57260fe0b156d0374 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 6 Mar 2019 18:01:47 +0100 Subject: [PATCH 9/9] Reword the documentation --- Doc/whatsnew/3.8.rst | 107 ++++++++++++++++++++++++------------------- 1 file changed, 60 insertions(+), 47 deletions(-) diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 67d4f745a561b6..3834f2f47076c6 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -469,8 +469,10 @@ Build and C API Changes ``1`` for objects implementing ``__index__()``. (Contributed by Serhiy Storchaka in :issue:`36048`.) -* Heap-allocated Types will now incref through :c:func:`PyObject_Init` (and - its parallel macro PyObject_INIT) instead of :c:func:`PyType_GenericAlloc`. +* Heap-allocated type objects will now increase their reference count + in :c:func:`PyObject_Init` (and its parallel macro ``PyObject_INIT``) + instead of in :c:func:`PyType_GenericAlloc`. Types that modify instance + allocation or deallocation may need to be adjusted. (Contributed by Eddie Elizondo in :issue:`35810`.) @@ -668,51 +670,62 @@ Changes in the Python API Changes in the C API -------------------------- -The incref to a heap-allocated type has been moved from -:c:func:`PyType_GenericAlloc` to :c:func:`PyObject_Init` and -:c:func:`PyObject_INIT`. This now makes types created through -:c:func:`PyType_FromSpec` behave like any other class in managed code. - -For the vast majority of cases, there should be no side effect. However, this -causes types that are manually incref after allocating an instance to become -immortal. Furthermore, the incref guarantee should now be reflected by having -a decref during instance deallocation. - -To correctly port these types into 3.8, please apply the following changes: - -* Remove any manual incref on the itself after allocating an instance - if any. - This happens after calling :c:func:`PyObject_New`, :c:func:`PyObject_NewVar`, - :c:func:`PyObject_GC_New`, :c:func:`PyObject_GC_NewVar`, or any other custom - allocator that uses :c:func:`PyObject_Init` or :c:func:`PyObject_INIT`. - -Example:: - - static foo_struct * - foo_new(PyObject *type) { - foo_struct *foo = PyObject_GC_New(foo_struct, (PyTypeObject *) type); - if (foo == NULL) - return NULL; - #if PY_VERSION_HEX < 0x03080000 - PY_INCREF(type) - #endif - return foo; - } - -* Add a decref to the type in any custom tp_dealloc. The type should always - decref at deallocation time. - -Example:: - - static void - foo_dealloc(foo_struct *instance) { - PyObject *type = Py_TYPE(instance); - PyObject_GC_Del(instance); - #if PY_VERSION_HEX >= 0x03080000 - Py_DECREF(type); - #endif - } - -(Contributed by Eddie Elizondo in :issue:`35810`.) +* Instances of heap-allocated types (such as those created with + :c:func:`PyType_FromSpec`) hold a reference to their type object. + Increasing the reference count of these type objects has been moved from + :c:func:`PyType_GenericAlloc` to the more low-level functions, + :c:func:`PyObject_Init` and :c:func:`PyObject_INIT`. + This makes types created through :c:func:`PyType_FromSpec` behave like + other classes in managed code. + + Statically allocated types are not affected. + + For the vast majority of cases, there should be no side effect. + However, types that manually increase the reference count after allocating + an instance (perhaps to work around the bug) may now become immortal. + To avoid this, these classes need to call Py_DECREF on the type object + during instance deallocation. + + To correctly port these types into 3.8, please apply the following + changes: + + * Remove :c:macro:`Py_INCREF` on the type object after allocating an + instance - if any. + This may happen after calling :c:func:`PyObject_New`, + :c:func:`PyObject_NewVar`, :c:func:`PyObject_GC_New`, + :c:func:`PyObject_GC_NewVar`, or any other custom allocator that uses + :c:func:`PyObject_Init` or :c:func:`PyObject_INIT`. + + Example:: + + static foo_struct * + foo_new(PyObject *type) { + foo_struct *foo = PyObject_GC_New(foo_struct, (PyTypeObject *) type); + if (foo == NULL) + return NULL; + #if PY_VERSION_HEX < 0x03080000 + // Workaround for Python issue 35810; no longer necessary in Python 3.8 + PY_INCREF(type) + #endif + return foo; + } + + * Ensure that all custom ``tp_dealloc`` functions of heap-allocated types + decrease the type's reference count. + + Example:: + + static void + foo_dealloc(foo_struct *instance) { + PyObject *type = Py_TYPE(instance); + PyObject_GC_Del(instance); + #if PY_VERSION_HEX >= 0x03080000 + // This was not needed before Python 3.8 (Python issue 35810) + Py_DECREF(type); + #endif + } + + (Contributed by Eddie Elizondo in :issue:`35810`.) CPython bytecode changes