bpo-31095: Fix potential crash during GC (GH-3197)
(cherry picked from commit a6296d34a478b4f697ea9db798146195075d496c)
diff --git a/Doc/extending/newtypes.rst b/Doc/extending/newtypes.rst
index 5959e4f..7eadcae 100644
--- a/Doc/extending/newtypes.rst
+++ b/Doc/extending/newtypes.rst
@@ -748,8 +748,9 @@
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)
@@ -767,13 +768,6 @@
return 0;
}
- static void
- Noddy_dealloc(Noddy* self)
- {
- Noddy_clear(self);
- self->ob_type->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
@@ -796,6 +790,23 @@
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 9feb71a..0fd4dc3 100644
--- a/Doc/includes/noddy4.c
+++ b/Doc/includes/noddy4.c
@@ -46,6 +46,7 @@
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 0000000..ca1f8ba
--- /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 1dd4b99..2e4da4c 100644
--- a/Modules/_collectionsmodule.c
+++ b/Modules/_collectionsmodule.c
@@ -1271,6 +1271,8 @@
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);
}
@@ -1556,6 +1558,8 @@
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/_functoolsmodule.c b/Modules/_functoolsmodule.c
index e0781a9..e9e8933 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -143,6 +143,7 @@
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);
diff --git a/Modules/_io/bytesio.c b/Modules/_io/bytesio.c
index 1335ae8..0ee4b80 100644
--- a/Modules/_io/bytesio.c
+++ b/Modules/_io/bytesio.c
@@ -745,6 +745,7 @@
static void
bytesio_dealloc(bytesio *self)
{
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
_PyObject_GC_UNTRACK(self);
if (self->buf != NULL) {
PyMem_Free(self->buf);
diff --git a/Modules/_json.c b/Modules/_json.c
index 42c93ab..be1e079 100644
--- a/Modules/_json.c
+++ b/Modules/_json.c
@@ -840,7 +840,8 @@
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);
}
@@ -2298,7 +2299,8 @@
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 45a1d01..213c7d2 100644
--- a/Modules/_ssl.c
+++ b/Modules/_ssl.c
@@ -2214,6 +2214,8 @@
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
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 9506067..a792b2d 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -1076,6 +1076,7 @@
{
register PyDictEntry *ep;
Py_ssize_t fill = mp->ma_fill;
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
PyObject_GC_UnTrack(mp);
Py_TRASHCAN_SAFE_BEGIN(mp)
for (ep = mp->ma_table; fill > 0; ep++) {
@@ -2576,6 +2577,8 @@
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);
@@ -2855,6 +2858,8 @@
static void
dictview_dealloc(dictviewobject *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 b3ca643..0c69fac 100644
--- a/Objects/setobject.c
+++ b/Objects/setobject.c
@@ -549,6 +549,7 @@
{
register setentry *entry;
Py_ssize_t fill = so->fill;
+ /* bpo-31095: UnTrack is needed before calling any callbacks */
PyObject_GC_UnTrack(so);
Py_TRASHCAN_SAFE_BEGIN(so)
if (so->weakreflist != NULL)
@@ -811,6 +812,8 @@
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);
}