[Python-checkins] bpo-31095: fix potential crash during GC (GH-3195)
INADA Naoki
webhook-mailer at python.org
Sun Sep 3 23:31:12 EDT 2017
https://github.com/python/cpython/commit/2eea952b1b9ebbc2d94fd3faca1536c6b4963725
commit: 2eea952b1b9ebbc2d94fd3faca1536c6b4963725
branch: 3.6
author: INADA Naoki <methane at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2017-09-04T12:31:09+09:00
summary:
bpo-31095: fix potential crash during GC (GH-3195)
(cherry picked from commit a6296d34a478b4f697ea9db798146195075d496c)
files:
A Misc/NEWS.d/next/Core and Builtins/2017-08-01-18-48-30.bpo-31095.bXWZDb.rst
M Doc/extending/newtypes.rst
M Doc/includes/noddy4.c
M Modules/_collectionsmodule.c
M Modules/_elementtree.c
M Modules/_functoolsmodule.c
M Modules/_io/bytesio.c
M Modules/_json.c
M Modules/_ssl.c
M Modules/_struct.c
M Objects/dictobject.c
M Objects/setobject.c
M Parser/asdl_c.py
M Python/Python-ast.c
diff --git a/Doc/extending/newtypes.rst b/Doc/extending/newtypes.rst
index 003b4e505d3..abd5da9db65 100644
--- a/Doc/extending/newtypes.rst
+++ b/Doc/extending/newtypes.rst
@@ -728,8 +728,9 @@ functions. With :c:func:`Py_VISIT`, :c:func:`Noddy_traverse` can be simplified:
uniformity across these boring implementations.
We also need to provide a method for clearing any subobjects that can
-participate in cycles. We implement the method and reimplement the deallocator
-to use it::
+participate in cycles.
+
+::
static int
Noddy_clear(Noddy *self)
@@ -747,13 +748,6 @@ to use it::
return 0;
}
- static void
- Noddy_dealloc(Noddy* self)
- {
- Noddy_clear(self);
- Py_TYPE(self)->tp_free((PyObject*)self);
- }
-
Notice the use of a temporary variable in :c:func:`Noddy_clear`. We use the
temporary variable so that we can set each member to *NULL* before decrementing
its reference count. We do this because, as was discussed earlier, if the
@@ -776,6 +770,23 @@ be simplified::
return 0;
}
+Note that :c:func:`Noddy_dealloc` may call arbitrary functions through
+``__del__`` method or weakref callback. It means circular GC can be
+triggered inside the function. Since GC assumes reference count is not zero,
+we need to untrack the object from GC by calling :c:func:`PyObject_GC_UnTrack`
+before clearing members. Here is reimplemented deallocator which uses
+:c:func:`PyObject_GC_UnTrack` and :c:func:`Noddy_clear`.
+
+::
+
+ static void
+ Noddy_dealloc(Noddy* self)
+ {
+ PyObject_GC_UnTrack(self);
+ Noddy_clear(self);
+ Py_TYPE(self)->tp_free((PyObject*)self);
+ }
+
Finally, we add the :const:`Py_TPFLAGS_HAVE_GC` flag to the class flags::
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, /* tp_flags */
diff --git a/Doc/includes/noddy4.c b/Doc/includes/noddy4.c
index eb9622a87d9..08ba4c3d91a 100644
--- a/Doc/includes/noddy4.c
+++ b/Doc/includes/noddy4.c
@@ -46,6 +46,7 @@ Noddy_clear(Noddy *self)
static void
Noddy_dealloc(Noddy* self)
{
+ PyObject_GC_UnTrack(self);
Noddy_clear(self);
Py_TYPE(self)->tp_free((PyObject*)self);
}
diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-08-01-18-48-30.bpo-31095.bXWZDb.rst b/Misc/NEWS.d/next/Core and Builtins/2017-08-01-18-48-30.bpo-31095.bXWZDb.rst
new file mode 100644
index 00000000000..ca1f8bafba6
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2017-08-01-18-48-30.bpo-31095.bXWZDb.rst
@@ -0,0 +1,2 @@
+Fix potential crash during GC caused by ``tp_dealloc`` which doesn't call
+``PyObject_GC_UnTrack()``.
diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c
index 30157701d70..e7a24f3f058 100644
--- a/Modules/_collectionsmodule.c
+++ b/Modules/_collectionsmodule.c
@@ -1706,6 +1706,8 @@ dequeiter_traverse(dequeiterobject *dio, visitproc visit, void *arg)
static void
dequeiter_dealloc(dequeiterobject *dio)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ PyObject_GC_UnTrack(dio);
Py_XDECREF(dio->deque);
PyObject_GC_Del(dio);
}
@@ -2086,6 +2088,8 @@ static PyMemberDef defdict_members[] = {
static void
defdict_dealloc(defdictobject *dd)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ PyObject_GC_UnTrack(dd);
Py_CLEAR(dd->default_factory);
PyDict_Type.tp_dealloc((PyObject *)dd);
}
diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c
index 599ca6a4cc0..ccf5e6a78fd 100644
--- a/Modules/_elementtree.c
+++ b/Modules/_elementtree.c
@@ -627,6 +627,7 @@ element_gc_clear(ElementObject *self)
static void
element_dealloc(ElementObject* self)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
PyObject_GC_UnTrack(self);
Py_TRASHCAN_SAFE_BEGIN(self)
@@ -2048,6 +2049,8 @@ elementiter_dealloc(ElementIterObject *it)
{
Py_ssize_t i = it->parent_stack_used;
it->parent_stack_used = 0;
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ PyObject_GC_UnTrack(it);
while (i--)
Py_XDECREF(it->parent_stack[i].parent);
PyMem_Free(it->parent_stack);
@@ -2055,7 +2058,6 @@ elementiter_dealloc(ElementIterObject *it)
Py_XDECREF(it->sought_tag);
Py_XDECREF(it->root_element);
- PyObject_GC_UnTrack(it);
PyObject_GC_Del(it);
}
diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 1bcf16a7e00..5622e2a6c3b 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -116,6 +116,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw)
static void
partial_dealloc(partialobject *pto)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
PyObject_GC_UnTrack(pto);
if (pto->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *) pto);
@@ -1038,7 +1039,11 @@ lru_cache_clear_list(lru_list_elem *link)
static void
lru_cache_dealloc(lru_cache_object *obj)
{
- lru_list_elem *list = lru_cache_unlink_list(obj);
+ lru_list_elem *list;
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ PyObject_GC_UnTrack(obj);
+
+ list = lru_cache_unlink_list(obj);
Py_XDECREF(obj->maxsize_O);
Py_XDECREF(obj->func);
Py_XDECREF(obj->cache);
diff --git a/Modules/_io/bytesio.c b/Modules/_io/bytesio.c
index a1ba121e262..6c54de733b9 100644
--- a/Modules/_io/bytesio.c
+++ b/Modules/_io/bytesio.c
@@ -1131,6 +1131,8 @@ bytesiobuf_traverse(bytesiobuf *self, visitproc visit, void *arg)
static void
bytesiobuf_dealloc(bytesiobuf *self)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ PyObject_GC_UnTrack(self);
Py_CLEAR(self->source);
Py_TYPE(self)->tp_free(self);
}
diff --git a/Modules/_json.c b/Modules/_json.c
index 1be4c17cf95..59376a7b0fb 100644
--- a/Modules/_json.c
+++ b/Modules/_json.c
@@ -655,7 +655,8 @@ py_encode_basestring(PyObject* self UNUSED, PyObject *pystr)
static void
scanner_dealloc(PyObject *self)
{
- /* Deallocate scanner object */
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ PyObject_GC_UnTrack(self);
scanner_clear(self);
Py_TYPE(self)->tp_free(self);
}
@@ -1793,7 +1794,8 @@ encoder_listencode_list(PyEncoderObject *s, _PyAccu *acc,
static void
encoder_dealloc(PyObject *self)
{
- /* Deallocate Encoder */
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ PyObject_GC_UnTrack(self);
encoder_clear(self);
Py_TYPE(self)->tp_free(self);
}
diff --git a/Modules/_ssl.c b/Modules/_ssl.c
index 0fffaaceb26..ae38386ca02 100644
--- a/Modules/_ssl.c
+++ b/Modules/_ssl.c
@@ -2776,6 +2776,8 @@ context_clear(PySSLContext *self)
static void
context_dealloc(PySSLContext *self)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ PyObject_GC_UnTrack(self);
context_clear(self);
SSL_CTX_free(self->ctx);
#ifdef OPENSSL_NPN_NEGOTIATED
@@ -4284,6 +4286,7 @@ static PyTypeObject PySSLMemoryBIO_Type = {
static void
PySSLSession_dealloc(PySSLSession *self)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
PyObject_GC_UnTrack(self);
Py_XDECREF(self->ctx);
if (self->session != NULL) {
diff --git a/Modules/_struct.c b/Modules/_struct.c
index 2635af9db69..e9af6efa2ea 100644
--- a/Modules/_struct.c
+++ b/Modules/_struct.c
@@ -1605,6 +1605,8 @@ typedef struct {
static void
unpackiter_dealloc(unpackiterobject *self)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ PyObject_GC_UnTrack(self);
Py_XDECREF(self->so);
PyBuffer_Release(&self->buf);
PyObject_GC_Del(self);
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index b0f583a067b..690ef3bd2b3 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -2006,6 +2006,8 @@ dict_dealloc(PyDictObject *mp)
PyObject **values = mp->ma_values;
PyDictKeysObject *keys = mp->ma_keys;
Py_ssize_t i, n;
+
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
PyObject_GC_UnTrack(mp);
Py_TRASHCAN_SAFE_BEGIN(mp)
if (values != NULL) {
@@ -3432,6 +3434,8 @@ dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
static void
dictiter_dealloc(dictiterobject *di)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ _PyObject_GC_UNTRACK(di);
Py_XDECREF(di->di_dict);
Py_XDECREF(di->di_result);
PyObject_GC_Del(di);
@@ -3800,6 +3804,8 @@ dictiter_reduce(dictiterobject *di)
static void
dictview_dealloc(_PyDictViewObject *dv)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ _PyObject_GC_UNTRACK(dv);
Py_XDECREF(dv->dv_dict);
PyObject_GC_Del(dv);
}
diff --git a/Objects/setobject.c b/Objects/setobject.c
index c1bc1e12347..24272b4d14a 100644
--- a/Objects/setobject.c
+++ b/Objects/setobject.c
@@ -556,6 +556,7 @@ set_dealloc(PySetObject *so)
setentry *entry;
Py_ssize_t used = so->used;
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
PyObject_GC_UnTrack(so);
Py_TRASHCAN_SAFE_BEGIN(so)
if (so->weakreflist != NULL)
@@ -812,6 +813,8 @@ typedef struct {
static void
setiter_dealloc(setiterobject *si)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ _PyObject_GC_UNTRACK(si);
Py_XDECREF(si->si_set);
PyObject_GC_Del(si);
}
diff --git a/Parser/asdl_c.py b/Parser/asdl_c.py
index 13124bbd3ad..1e5f4d9a2a5 100644
--- a/Parser/asdl_c.py
+++ b/Parser/asdl_c.py
@@ -630,6 +630,8 @@ def visitModule(self, mod):
static void
ast_dealloc(AST_object *self)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ PyObject_GC_UnTrack(self);
Py_CLEAR(self->dict);
Py_TYPE(self)->tp_free(self);
}
diff --git a/Python/Python-ast.c b/Python/Python-ast.c
index b78a0fc714e..212211c5f43 100644
--- a/Python/Python-ast.c
+++ b/Python/Python-ast.c
@@ -512,6 +512,8 @@ typedef struct {
static void
ast_dealloc(AST_object *self)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
+ PyObject_GC_UnTrack(self);
Py_CLEAR(self->dict);
Py_TYPE(self)->tp_free(self);
}
More information about the Python-checkins
mailing list