380 lines
14 KiB
Diff
380 lines
14 KiB
Diff
From 932ccedc35b14b2f520f1c0f449f575e1239cf48 Mon Sep 17 00:00:00 2001
|
|
From: "Miss Islington (bot)"
|
|
<31488909+miss-islington@users.noreply.github.com>
|
|
Date: Thu, 28 May 2020 08:12:23 -0700
|
|
Subject: [PATCH] 00349: Ensure Py_VISIT(Py_TYPE(self)) is always called for
|
|
PyType_FromSpec types
|
|
|
|
Heap types now always visit the type in tp_traverse. See added docs for details.
|
|
|
|
Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
|
|
---
|
|
Doc/c-api/typeobj.rst | 16 ++-
|
|
Doc/whatsnew/3.9.rst | 49 +++++++++
|
|
.../2020-05-23-01-15-51.bpo-40217.jZsHTc.rst | 4 +
|
|
Modules/_abc.c | 1 +
|
|
Modules/_curses_panel.c | 1 +
|
|
Modules/_json.c | 2 +
|
|
Modules/_struct.c | 1 +
|
|
Modules/xxlimited.c | 1 +
|
|
Objects/structseq.c | 3 +
|
|
Objects/typeobject.c | 101 ++----------------
|
|
Parser/asdl_c.py | 1 +
|
|
Python/Python-ast.c | 1 +
|
|
12 files changed, 87 insertions(+), 94 deletions(-)
|
|
create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-05-23-01-15-51.bpo-40217.jZsHTc.rst
|
|
|
|
diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst
|
|
index ce4e8c926b..385c7f94c6 100644
|
|
--- a/Doc/c-api/typeobj.rst
|
|
+++ b/Doc/c-api/typeobj.rst
|
|
@@ -1223,11 +1223,25 @@ and :c:type:`PyType_Type` effectively act as defaults.)
|
|
but the instance has no strong reference to the elements inside it, as they
|
|
are allowed to be removed even if the instance is still alive).
|
|
|
|
-
|
|
Note that :c:func:`Py_VISIT` requires the *visit* and *arg* parameters to
|
|
:c:func:`local_traverse` to have these specific names; don't name them just
|
|
anything.
|
|
|
|
+ Heap-allocated types (:const:`Py_TPFLAGS_HEAPTYPE`, such as those created
|
|
+ with :c:func:`PyType_FromSpec` and similar APIs) hold a reference to their
|
|
+ type. Their traversal function must therefore either visit
|
|
+ :c:func:`Py_TYPE(self) <Py_TYPE>`, or delegate this responsibility by
|
|
+ calling ``tp_traverse`` of another heap-allocated type (such as a
|
|
+ heap-allocated superclass).
|
|
+ If they do not, the type object may not be garbage-collected.
|
|
+
|
|
+ .. versionchanged:: 3.9
|
|
+
|
|
+ Heap-allocated types are expected to visit ``Py_TYPE(self)`` in
|
|
+ ``tp_traverse``. In earlier versions of Python, due to
|
|
+ `bug 40217 <https://bugs.python.org/issue40217>`_, doing this
|
|
+ may lead to crashes in subclasses.
|
|
+
|
|
**Inheritance:**
|
|
|
|
Group: :const:`Py_TPFLAGS_HAVE_GC`, :attr:`tp_traverse`, :attr:`tp_clear`
|
|
diff --git a/Doc/whatsnew/3.9.rst b/Doc/whatsnew/3.9.rst
|
|
index 593f523828..2d095dcae9 100644
|
|
--- a/Doc/whatsnew/3.9.rst
|
|
+++ b/Doc/whatsnew/3.9.rst
|
|
@@ -862,6 +862,55 @@ Changes in the Python API
|
|
(Contributed by Inada Naoki in :issue:`34538`.)
|
|
|
|
|
|
+Changes in the C API
|
|
+--------------------
|
|
+
|
|
+* Instances of heap-allocated types (such as those created with
|
|
+ :c:func:`PyType_FromSpec` and similar APIs) hold a reference to their type
|
|
+ object since Python 3.8. As indicated in the "Changes in the C API" of Python
|
|
+ 3.8, for the vast majority of cases, there should be no side effect but for
|
|
+ types that have a custom :c:member:`~PyTypeObject.tp_traverse` function,
|
|
+ ensure that all custom ``tp_traverse`` functions of heap-allocated types
|
|
+ visit the object's type.
|
|
+
|
|
+ Example:
|
|
+
|
|
+ .. code-block:: c
|
|
+
|
|
+ int
|
|
+ foo_traverse(foo_struct *self, visitproc visit, void *arg) {
|
|
+ // Rest of the traverse function
|
|
+ #if PY_VERSION_HEX >= 0x03090000
|
|
+ // This was not needed before Python 3.9 (Python issue 35810 and 40217)
|
|
+ Py_VISIT(Py_TYPE(self));
|
|
+ #endif
|
|
+ }
|
|
+
|
|
+ If your traverse function delegates to ``tp_traverse`` of its base class
|
|
+ (or another type), ensure that ``Py_TYPE(self)`` is visited only once.
|
|
+ Note that only heap types are expected to visit the type in ``tp_traverse``.
|
|
+
|
|
+ For example, if your ``tp_traverse`` function includes:
|
|
+
|
|
+ .. code-block:: c
|
|
+
|
|
+ base->tp_traverse(self, visit, arg)
|
|
+
|
|
+ then add:
|
|
+
|
|
+ .. code-block:: c
|
|
+
|
|
+ #if PY_VERSION_HEX >= 0x03090000
|
|
+ // This was not needed before Python 3.9 (Python issue 35810 and 40217)
|
|
+ if (base->tp_flags & Py_TPFLAGS_HEAPTYPE) {
|
|
+ // a heap type's tp_traverse already visited Py_TYPE(self)
|
|
+ } else {
|
|
+ Py_VISIT(Py_TYPE(self));
|
|
+ }
|
|
+ #else
|
|
+
|
|
+ (See :issue:`35810` and :issue:`40217` for more information.)
|
|
+
|
|
CPython bytecode changes
|
|
------------------------
|
|
|
|
diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-05-23-01-15-51.bpo-40217.jZsHTc.rst b/Misc/NEWS.d/next/Core and Builtins/2020-05-23-01-15-51.bpo-40217.jZsHTc.rst
|
|
new file mode 100644
|
|
index 0000000000..b13e8eeb06
|
|
--- /dev/null
|
|
+++ b/Misc/NEWS.d/next/Core and Builtins/2020-05-23-01-15-51.bpo-40217.jZsHTc.rst
|
|
@@ -0,0 +1,4 @@
|
|
+Instances of types created with :c:func:`PyType_FromSpecWithBases` will no
|
|
+longer automatically visit their class object when traversing references in
|
|
+the garbage collector. The user is expected to manually visit the object's
|
|
+class. Patch by Pablo Galindo.
|
|
diff --git a/Modules/_abc.c b/Modules/_abc.c
|
|
index 434bc45417..709b52ff96 100644
|
|
--- a/Modules/_abc.c
|
|
+++ b/Modules/_abc.c
|
|
@@ -46,6 +46,7 @@ typedef struct {
|
|
static int
|
|
abc_data_traverse(_abc_data *self, visitproc visit, void *arg)
|
|
{
|
|
+ Py_VISIT(Py_TYPE(self));
|
|
Py_VISIT(self->_abc_registry);
|
|
Py_VISIT(self->_abc_cache);
|
|
Py_VISIT(self->_abc_negative_cache);
|
|
diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c
|
|
index 7ca91f6416..f124803493 100644
|
|
--- a/Modules/_curses_panel.c
|
|
+++ b/Modules/_curses_panel.c
|
|
@@ -39,6 +39,7 @@ _curses_panel_clear(PyObject *m)
|
|
static int
|
|
_curses_panel_traverse(PyObject *m, visitproc visit, void *arg)
|
|
{
|
|
+ Py_VISIT(Py_TYPE(m));
|
|
Py_VISIT(get_curses_panelstate(m)->PyCursesError);
|
|
return 0;
|
|
}
|
|
diff --git a/Modules/_json.c b/Modules/_json.c
|
|
index 075aa3d2f4..faa3944eed 100644
|
|
--- a/Modules/_json.c
|
|
+++ b/Modules/_json.c
|
|
@@ -647,6 +647,7 @@ scanner_dealloc(PyObject *self)
|
|
static int
|
|
scanner_traverse(PyScannerObject *self, visitproc visit, void *arg)
|
|
{
|
|
+ Py_VISIT(Py_TYPE(self));
|
|
Py_VISIT(self->object_hook);
|
|
Py_VISIT(self->object_pairs_hook);
|
|
Py_VISIT(self->parse_float);
|
|
@@ -1745,6 +1746,7 @@ encoder_dealloc(PyObject *self)
|
|
static int
|
|
encoder_traverse(PyEncoderObject *self, visitproc visit, void *arg)
|
|
{
|
|
+ Py_VISIT(Py_TYPE(self));
|
|
Py_VISIT(self->markers);
|
|
Py_VISIT(self->defaultfn);
|
|
Py_VISIT(self->encoder);
|
|
diff --git a/Modules/_struct.c b/Modules/_struct.c
|
|
index 13d8072f61..3cb3ccd782 100644
|
|
--- a/Modules/_struct.c
|
|
+++ b/Modules/_struct.c
|
|
@@ -1641,6 +1641,7 @@ unpackiter_dealloc(unpackiterobject *self)
|
|
static int
|
|
unpackiter_traverse(unpackiterobject *self, visitproc visit, void *arg)
|
|
{
|
|
+ Py_VISIT(Py_TYPE(self));
|
|
Py_VISIT(self->so);
|
|
Py_VISIT(self->buf.obj);
|
|
return 0;
|
|
diff --git a/Modules/xxlimited.c b/Modules/xxlimited.c
|
|
index 7ce0b6ec88..5b05a9454a 100644
|
|
--- a/Modules/xxlimited.c
|
|
+++ b/Modules/xxlimited.c
|
|
@@ -43,6 +43,7 @@ newXxoObject(PyObject *arg)
|
|
static int
|
|
Xxo_traverse(XxoObject *self, visitproc visit, void *arg)
|
|
{
|
|
+ Py_VISIT(Py_TYPE(self));
|
|
Py_VISIT(self->x_attr);
|
|
return 0;
|
|
}
|
|
diff --git a/Objects/structseq.c b/Objects/structseq.c
|
|
index 9bdda87ae0..b17b1f99a5 100644
|
|
--- a/Objects/structseq.c
|
|
+++ b/Objects/structseq.c
|
|
@@ -70,6 +70,9 @@ PyStructSequence_GetItem(PyObject* op, Py_ssize_t i)
|
|
static int
|
|
structseq_traverse(PyStructSequence *obj, visitproc visit, void *arg)
|
|
{
|
|
+ if (Py_TYPE(obj)->tp_flags & Py_TPFLAGS_HEAPTYPE) {
|
|
+ Py_VISIT(Py_TYPE(obj));
|
|
+ }
|
|
Py_ssize_t i, size;
|
|
size = REAL_SIZE(obj);
|
|
for (i = 0; i < size; ++i) {
|
|
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
|
|
index 243f8811b6..bd1acd5108 100644
|
|
--- a/Objects/typeobject.c
|
|
+++ b/Objects/typeobject.c
|
|
@@ -1039,42 +1039,6 @@ type_call(PyTypeObject *type, PyObject *args, PyObject *kwds)
|
|
return obj;
|
|
}
|
|
|
|
-PyObject *
|
|
-PyType_FromSpec_Alloc(PyTypeObject *type, Py_ssize_t nitems)
|
|
-{
|
|
- PyObject *obj;
|
|
- const size_t size = _Py_SIZE_ROUND_UP(
|
|
- _PyObject_VAR_SIZE(type, nitems+1) + sizeof(traverseproc),
|
|
- SIZEOF_VOID_P);
|
|
- /* note that we need to add one, for the sentinel and space for the
|
|
- provided tp-traverse: See bpo-40217 for more details */
|
|
-
|
|
- if (PyType_IS_GC(type)) {
|
|
- obj = _PyObject_GC_Malloc(size);
|
|
- }
|
|
- else {
|
|
- obj = (PyObject *)PyObject_MALLOC(size);
|
|
- }
|
|
-
|
|
- if (obj == NULL) {
|
|
- return PyErr_NoMemory();
|
|
- }
|
|
-
|
|
- memset(obj, '\0', size);
|
|
-
|
|
- if (type->tp_itemsize == 0) {
|
|
- (void)PyObject_INIT(obj, type);
|
|
- }
|
|
- else {
|
|
- (void) PyObject_INIT_VAR((PyVarObject *)obj, type, nitems);
|
|
- }
|
|
-
|
|
- if (PyType_IS_GC(type)) {
|
|
- _PyObject_GC_TRACK(obj);
|
|
- }
|
|
- return obj;
|
|
-}
|
|
-
|
|
PyObject *
|
|
PyType_GenericAlloc(PyTypeObject *type, Py_ssize_t nitems)
|
|
{
|
|
@@ -1164,11 +1128,16 @@ subtype_traverse(PyObject *self, visitproc visit, void *arg)
|
|
Py_VISIT(*dictptr);
|
|
}
|
|
|
|
- if (type->tp_flags & Py_TPFLAGS_HEAPTYPE)
|
|
+ if (type->tp_flags & Py_TPFLAGS_HEAPTYPE
|
|
+ && (!basetraverse || !(base->tp_flags & Py_TPFLAGS_HEAPTYPE))) {
|
|
/* For a heaptype, the instances count as references
|
|
to the type. Traverse the type so the collector
|
|
- can find cycles involving this link. */
|
|
+ can find cycles involving this link.
|
|
+ Skip this visit if basetraverse belongs to a heap type: in that
|
|
+ case, basetraverse will visit the type when we call it later.
|
|
+ */
|
|
Py_VISIT(type);
|
|
+ }
|
|
|
|
if (basetraverse)
|
|
return basetraverse(self, visit, arg);
|
|
@@ -2910,36 +2879,6 @@ static const short slotoffsets[] = {
|
|
#include "typeslots.inc"
|
|
};
|
|
|
|
-static int
|
|
-PyType_FromSpec_tp_traverse(PyObject *self, visitproc visit, void *arg)
|
|
-{
|
|
- PyTypeObject *parent = Py_TYPE(self);
|
|
-
|
|
- // Only a instance of a type that is directly created by
|
|
- // PyType_FromSpec (not subclasses) must visit its parent.
|
|
- if (parent->tp_traverse == PyType_FromSpec_tp_traverse) {
|
|
- Py_VISIT(parent);
|
|
- }
|
|
-
|
|
- // Search for the original type that was created using PyType_FromSpec
|
|
- PyTypeObject *base;
|
|
- base = parent;
|
|
- while (base->tp_traverse != PyType_FromSpec_tp_traverse) {
|
|
- base = base->tp_base;
|
|
- assert(base);
|
|
- }
|
|
-
|
|
- // Extract the user defined traverse function that we placed at the end
|
|
- // of the type and call it.
|
|
- size_t size = Py_SIZE(base);
|
|
- size_t _offset = _PyObject_VAR_SIZE(&PyType_Type, size+1);
|
|
- traverseproc fun = *(traverseproc*)((char*)base + _offset);
|
|
- if (fun == NULL) {
|
|
- return 0;
|
|
- }
|
|
- return fun(self, visit, arg);
|
|
-}
|
|
-
|
|
PyObject *
|
|
PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases)
|
|
{
|
|
@@ -2985,7 +2924,7 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases)
|
|
}
|
|
}
|
|
|
|
- res = (PyHeapTypeObject*)PyType_FromSpec_Alloc(&PyType_Type, nmembers);
|
|
+ res = (PyHeapTypeObject*)PyType_GenericAlloc(&PyType_Type, nmembers);
|
|
if (res == NULL)
|
|
return NULL;
|
|
res_start = (char*)res;
|
|
@@ -3093,30 +3032,6 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases)
|
|
memcpy(PyHeapType_GET_MEMBERS(res), slot->pfunc, len);
|
|
type->tp_members = PyHeapType_GET_MEMBERS(res);
|
|
}
|
|
- else if (slot->slot == Py_tp_traverse) {
|
|
-
|
|
- /* Types created by PyType_FromSpec own a strong reference to their
|
|
- * type, but this was added in Python 3.8. The tp_traverse function
|
|
- * needs to call Py_VISIT on the type but all existing traverse
|
|
- * functions cannot be updated (especially the ones from existing user
|
|
- * functions) so we need to provide a tp_traverse that manually calls
|
|
- * Py_VISIT(Py_TYPE(self)) and then call the provided tp_traverse. In
|
|
- * this way, user functions do not need to be updated, preserve
|
|
- * backwards compatibility.
|
|
- *
|
|
- * We store the user-provided traverse function at the end of the type
|
|
- * (we have allocated space for it) so we can call it from our
|
|
- * PyType_FromSpec_tp_traverse wrapper.
|
|
- *
|
|
- * Check bpo-40217 for more information and rationale about this issue.
|
|
- *
|
|
- * */
|
|
-
|
|
- type->tp_traverse = PyType_FromSpec_tp_traverse;
|
|
- size_t _offset = _PyObject_VAR_SIZE(&PyType_Type, nmembers+1);
|
|
- traverseproc *user_traverse = (traverseproc*)((char*)type + _offset);
|
|
- *user_traverse = slot->pfunc;
|
|
- }
|
|
else {
|
|
/* Copy other slots directly */
|
|
*(void**)(res_start + slotoffsets[slot->slot]) = slot->pfunc;
|
|
diff --git a/Parser/asdl_c.py b/Parser/asdl_c.py
|
|
index 6d572755e6..4ceeb0b85a 100755
|
|
--- a/Parser/asdl_c.py
|
|
+++ b/Parser/asdl_c.py
|
|
@@ -673,6 +673,7 @@ ast_dealloc(AST_object *self)
|
|
static int
|
|
ast_traverse(AST_object *self, visitproc visit, void *arg)
|
|
{
|
|
+ Py_VISIT(Py_TYPE(self));
|
|
Py_VISIT(self->dict);
|
|
return 0;
|
|
}
|
|
diff --git a/Python/Python-ast.c b/Python/Python-ast.c
|
|
index f34b1450c6..aba879e485 100644
|
|
--- a/Python/Python-ast.c
|
|
+++ b/Python/Python-ast.c
|
|
@@ -1109,6 +1109,7 @@ ast_dealloc(AST_object *self)
|
|
static int
|
|
ast_traverse(AST_object *self, visitproc visit, void *arg)
|
|
{
|
|
+ Py_VISIT(Py_TYPE(self));
|
|
Py_VISIT(self->dict);
|
|
return 0;
|
|
}
|
|
--
|
|
2.26.2
|
|
|