Skip to content

Commit 4cde4bd

Browse files
methanemethane
authored
bpo-31095: Fix potential crash during GC (GH-3197)
(cherry picked from commit a6296d3)
1 parent 990b2d0 commit 4cde4bd

File tree

10 files changed

+43
-11
lines changed

10 files changed

+43
-11
lines changed

‎Doc/extending/newtypes.rst‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -748,8 +748,9 @@ simplified::
748748
uniformity across these boring implementations.
749749

750750
We also need to provide a method for clearing any subobjects that can
751-
participate in cycles. We implement the method and reimplement the deallocator
752-
to use it::
751+
participate in cycles.
752+
753+
::
753754

754755
static int
755756
Noddy_clear(Noddy *self)
@@ -767,13 +768,6 @@ to use it::
767768
return 0;
768769
}
769770

770-
static void
771-
Noddy_dealloc(Noddy* self)
772-
{
773-
Noddy_clear(self);
774-
self->ob_type->tp_free((PyObject*)self);
775-
}
776-
777771
Notice the use of a temporary variable in :c:func:`Noddy_clear`. We use the
778772
temporary variable so that we can set each member to *NULL* before decrementing
779773
its reference count. We do this because, as was discussed earlier, if the
@@ -796,6 +790,23 @@ decrementing of reference counts. With :c:func:`Py_CLEAR`, the
796790
return 0;
797791
}
798792

793+
Note that :c:func:`Noddy_dealloc` may call arbitrary functions through
794+
``__del__`` method or weakref callback. It means circular GC can be
795+
triggered inside the function. Since GC assumes reference count is not zero,
796+
we need to untrack the object from GC by calling :c:func:`PyObject_GC_UnTrack`
797+
before clearing members. Here is reimplemented deallocator which uses
798+
:c:func:`PyObject_GC_UnTrack` and :c:func:`Noddy_clear`.
799+
800+
::
801+
802+
static void
803+
Noddy_dealloc(Noddy* self)
804+
{
805+
PyObject_GC_UnTrack(self);
806+
Noddy_clear(self);
807+
Py_TYPE(self)->tp_free((PyObject*)self);
808+
}
809+
799810
Finally, we add the :const:`Py_TPFLAGS_HAVE_GC` flag to the class flags::
800811

801812
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, /* tp_flags */

‎Doc/includes/noddy4.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ Noddy_clear(Noddy *self)
4646
static void
4747
Noddy_dealloc(Noddy* self)
4848
{
49+
PyObject_GC_UnTrack(self);
4950
Noddy_clear(self);
5051
Py_TYPE(self)->tp_free((PyObject*)self);
5152
}

‎Misc/NEWS.d/next/Core and Builtins/2017-08-01-18-48-30.bpo-31095.bXWZDb.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix potential crash during GC caused by ``tp_dealloc`` which doesn't call
2+
``PyObject_GC_UnTrack()``.

‎Modules/_collectionsmodule.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,6 +1271,8 @@ dequeiter_traverse(dequeiterobject *dio, visitproc visit, void *arg)
12711271
static void
12721272
dequeiter_dealloc(dequeiterobject *dio)
12731273
{
1274+
/* bpo-31095: UnTrack is needed before calling any callbacks */
1275+
PyObject_GC_UnTrack(dio);
12741276
Py_XDECREF(dio->deque);
12751277
PyObject_GC_Del(dio);
12761278
}
@@ -1556,6 +1558,8 @@ static PyMemberDef defdict_members[] = {
15561558
static void
15571559
defdict_dealloc(defdictobject *dd)
15581560
{
1561+
/* bpo-31095: UnTrack is needed before calling any callbacks */
1562+
PyObject_GC_UnTrack(dd);
15591563
Py_CLEAR(dd->default_factory);
15601564
PyDict_Type.tp_dealloc((PyObject *)dd);
15611565
}

‎Modules/_functoolsmodule.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw)
143143
static void
144144
partial_dealloc(partialobject *pto)
145145
{
146+
/* bpo-31095: UnTrack is needed before calling any callbacks */
146147
PyObject_GC_UnTrack(pto);
147148
if (pto->weakreflist != NULL)
148149
PyObject_ClearWeakRefs((PyObject *) pto);

‎Modules/_io/bytesio.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,7 @@ bytesio_setstate(bytesio *self, PyObject *state)
745745
static void
746746
bytesio_dealloc(bytesio *self)
747747
{
748+
/* bpo-31095: UnTrack is needed before calling any callbacks */
748749
_PyObject_GC_UNTRACK(self);
749750
if (self->buf != NULL) {
750751
PyMem_Free(self->buf);

‎Modules/_json.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -840,7 +840,8 @@ py_encode_basestring_ascii(PyObject* self UNUSED, PyObject *pystr)
840840
static void
841841
scanner_dealloc(PyObject *self)
842842
{
843-
/* Deallocate scanner object */
843+
/* bpo-31095: UnTrack is needed before calling any callbacks */
844+
PyObject_GC_UnTrack(self);
844845
scanner_clear(self);
845846
Py_TYPE(self)->tp_free(self);
846847
}
@@ -2298,7 +2299,8 @@ encoder_listencode_list(PyEncoderObject *s, PyObject *rval, PyObject *seq, Py_ss
22982299
static void
22992300
encoder_dealloc(PyObject *self)
23002301
{
2301-
/* Deallocate Encoder */
2302+
/* bpo-31095: UnTrack is needed before calling any callbacks */
2303+
PyObject_GC_UnTrack(self);
23022304
encoder_clear(self);
23032305
Py_TYPE(self)->tp_free(self);
23042306
}

‎Modules/_ssl.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2214,6 +2214,8 @@ context_clear(PySSLContext *self)
22142214
static void
22152215
context_dealloc(PySSLContext *self)
22162216
{
2217+
/* bpo-31095: UnTrack is needed before calling any callbacks */
2218+
PyObject_GC_UnTrack(self);
22172219
context_clear(self);
22182220
SSL_CTX_free(self->ctx);
22192221
#ifdef OPENSSL_NPN_NEGOTIATED

‎Objects/dictobject.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,6 +1076,7 @@ dict_dealloc(register PyDictObject *mp)
10761076
{
10771077
register PyDictEntry *ep;
10781078
Py_ssize_t fill = mp->ma_fill;
1079+
/* bpo-31095: UnTrack is needed before calling any callbacks */
10791080
PyObject_GC_UnTrack(mp);
10801081
Py_TRASHCAN_SAFE_BEGIN(mp)
10811082
for (ep = mp->ma_table; fill > 0; ep++) {
@@ -2576,6 +2577,8 @@ dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
25762577
static void
25772578
dictiter_dealloc(dictiterobject *di)
25782579
{
2580+
/* bpo-31095: UnTrack is needed before calling any callbacks */
2581+
_PyObject_GC_UNTRACK(di);
25792582
Py_XDECREF(di->di_dict);
25802583
Py_XDECREF(di->di_result);
25812584
PyObject_GC_Del(di);
@@ -2855,6 +2858,8 @@ typedef struct {
28552858
static void
28562859
dictview_dealloc(dictviewobject *dv)
28572860
{
2861+
/* bpo-31095: UnTrack is needed before calling any callbacks */
2862+
_PyObject_GC_UNTRACK(dv);
28582863
Py_XDECREF(dv->dv_dict);
28592864
PyObject_GC_Del(dv);
28602865
}

‎Objects/setobject.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,7 @@ set_dealloc(PySetObject *so)
549549
{
550550
register setentry *entry;
551551
Py_ssize_t fill = so->fill;
552+
/* bpo-31095: UnTrack is needed before calling any callbacks */
552553
PyObject_GC_UnTrack(so);
553554
Py_TRASHCAN_SAFE_BEGIN(so)
554555
if (so->weakreflist != NULL)
@@ -811,6 +812,8 @@ typedef struct {
811812
static void
812813
setiter_dealloc(setiterobject *si)
813814
{
815+
/* bpo-31095: UnTrack is needed before calling any callbacks */
816+
_PyObject_GC_UNTRACK(si);
814817
Py_XDECREF(si->si_set);
815818
PyObject_GC_Del(si);
816819
}

0 commit comments

Comments
 (0)