Home Original page

bpo-31095: fix potential crash during GC (GH-2974) (#3196) · python/cpython@0fcc033

14 files changed

lines changed

Original file line numberDiff line numberDiff line change

@@ -728,8 +728,9 @@ functions. With :c:func:`Py_VISIT`, :c:func:`Noddy_traverse` can be simplified:

728728

uniformity across these boring implementations.

729729
730730

We also need to provide a method for clearing any subobjects that can

731-

participate in cycles. We implement the method and reimplement the deallocator

732-

to use it::

731+

participate in cycles.

732+
733+

::

733734
734735

static int

735736

Noddy_clear(Noddy *self)

@@ -747,13 +748,6 @@ to use it::

747748

return 0;

748749

}

749750
750-

static void

751-

Noddy_dealloc(Noddy* self)

752-

{

753-

Noddy_clear(self);

754-

Py_TYPE(self)->tp_free((PyObject*)self);

755-

}

756-
757751

Notice the use of a temporary variable in :c:func:`Noddy_clear`. We use the

758752

temporary variable so that we can set each member to *NULL* before decrementing

759753

its reference count. We do this because, as was discussed earlier, if the

@@ -776,6 +770,23 @@ be simplified::

776770

return 0;

777771

}

778772
773+

Note that :c:func:`Noddy_dealloc` may call arbitrary functions through

774+

``__del__`` method or weakref callback. It means circular GC can be

775+

triggered inside the function. Since GC assumes reference count is not zero,

776+

we need to untrack the object from GC by calling :c:func:`PyObject_GC_UnTrack`

777+

before clearing members. Here is reimplemented deallocator which uses

778+

:c:func:`PyObject_GC_UnTrack` and :c:func:`Noddy_clear`.

779+
780+

::

781+
782+

static void

783+

Noddy_dealloc(Noddy* self)

784+

{

785+

PyObject_GC_UnTrack(self);

786+

Noddy_clear(self);

787+

Py_TYPE(self)->tp_free((PyObject*)self);

788+

}

789+
779790

Finally, we add the :const:`Py_TPFLAGS_HAVE_GC` flag to the class flags::

780791
781792

Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, /* tp_flags */

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

}

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()``.

Original file line numberDiff line numberDiff line change

@@ -1641,6 +1641,8 @@ dequeiter_traverse(dequeiterobject *dio, visitproc visit, void *arg)

16411641

static void

16421642

dequeiter_dealloc(dequeiterobject *dio)

16431643

{

1644+

/* bpo-31095: UnTrack is needed before calling any callbacks */

1645+

PyObject_GC_UnTrack(dio);

16441646

Py_XDECREF(dio->deque);

16451647

PyObject_GC_Del(dio);

16461648

}

@@ -2021,6 +2023,8 @@ static PyMemberDef defdict_members[] = {

20212023

static void

20222024

defdict_dealloc(defdictobject *dd)

20232025

{

2026+

/* bpo-31095: UnTrack is needed before calling any callbacks */

2027+

PyObject_GC_UnTrack(dd);

20242028

Py_CLEAR(dd->default_factory);

20252029

PyDict_Type.tp_dealloc((PyObject *)dd);

20262030

}

Original file line numberDiff line numberDiff line change

@@ -653,6 +653,7 @@ element_gc_clear(ElementObject *self)

653653

static void

654654

element_dealloc(ElementObject* self)

655655

{

656+

/* bpo-31095: UnTrack is needed before calling any callbacks */

656657

PyObject_GC_UnTrack(self);

657658

Py_TRASHCAN_SAFE_BEGIN(self)

658659

@@ -2025,6 +2026,9 @@ static void

20252026

elementiter_dealloc(ElementIterObject *it)

20262027

{

20272028

ParentLocator *p = it->parent_stack;

2029+

/* bpo-31095: UnTrack is needed before calling any callbacks */

2030+

PyObject_GC_UnTrack(it);

2031+
20282032

while (p) {

20292033

ParentLocator *temp = p;

20302034

Py_XDECREF(p->parent);

@@ -2034,8 +2038,6 @@ elementiter_dealloc(ElementIterObject *it)

20342038
20352039

Py_XDECREF(it->sought_tag);

20362040

Py_XDECREF(it->root_element);

2037-
2038-

PyObject_GC_UnTrack(it);

20392041

PyObject_GC_Del(it);

20402042

}

20412043
Original file line numberDiff line numberDiff line change

@@ -116,6 +116,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw)

116116

static void

117117

partial_dealloc(partialobject *pto)

118118

{

119+

/* bpo-31095: UnTrack is needed before calling any callbacks */

119120

PyObject_GC_UnTrack(pto);

120121

if (pto->weakreflist != NULL)

121122

PyObject_ClearWeakRefs((PyObject *) pto);

@@ -1039,7 +1040,11 @@ lru_cache_clear_list(lru_list_elem *link)

10391040

static void

10401041

lru_cache_dealloc(lru_cache_object *obj)

10411042

{

1042-

lru_list_elem *list = lru_cache_unlink_list(obj);

1043+

lru_list_elem *list;

1044+

/* bpo-31095: UnTrack is needed before calling any callbacks */

1045+

PyObject_GC_UnTrack(obj);

1046+
1047+

list = lru_cache_unlink_list(obj);

10431048

Py_XDECREF(obj->maxsize_O);

10441049

Py_XDECREF(obj->func);

10451050

Py_XDECREF(obj->cache);

Original file line numberDiff line numberDiff line change

@@ -1133,6 +1133,8 @@ bytesiobuf_traverse(bytesiobuf *self, visitproc visit, void *arg)

11331133

static void

11341134

bytesiobuf_dealloc(bytesiobuf *self)

11351135

{

1136+

/* bpo-31095: UnTrack is needed before calling any callbacks */

1137+

PyObject_GC_UnTrack(self);

11361138

Py_CLEAR(self->source);

11371139

Py_TYPE(self)->tp_free(self);

11381140

}

Original file line numberDiff line numberDiff line change

@@ -655,7 +655,8 @@ py_encode_basestring(PyObject* self UNUSED, PyObject *pystr)

655655

static void

656656

scanner_dealloc(PyObject *self)

657657

{

658-

/* Deallocate scanner object */

658+

/* bpo-31095: UnTrack is needed before calling any callbacks */

659+

PyObject_GC_UnTrack(self);

659660

scanner_clear(self);

660661

Py_TYPE(self)->tp_free(self);

661662

}

@@ -1793,7 +1794,8 @@ encoder_listencode_list(PyEncoderObject *s, _PyAccu *acc,

17931794

static void

17941795

encoder_dealloc(PyObject *self)

17951796

{

1796-

/* Deallocate Encoder */

1797+

/* bpo-31095: UnTrack is needed before calling any callbacks */

1798+

PyObject_GC_UnTrack(self);

17971799

encoder_clear(self);

17981800

Py_TYPE(self)->tp_free(self);

17991801

}

Original file line numberDiff line numberDiff line change

@@ -2465,6 +2465,8 @@ context_clear(PySSLContext *self)

24652465

static void

24662466

context_dealloc(PySSLContext *self)

24672467

{

2468+

/* bpo-31095: UnTrack is needed before calling any callbacks */

2469+

PyObject_GC_UnTrack(self);

24682470

context_clear(self);

24692471

SSL_CTX_free(self->ctx);

24702472

#ifdef OPENSSL_NPN_NEGOTIATED

Original file line numberDiff line numberDiff line change

@@ -1581,6 +1581,8 @@ typedef struct {

15811581

static void

15821582

unpackiter_dealloc(unpackiterobject *self)

15831583

{

1584+

/* bpo-31095: UnTrack is needed before calling any callbacks */

1585+

PyObject_GC_UnTrack(self);

15841586

Py_XDECREF(self->so);

15851587

PyBuffer_Release(&self->buf);

15861588

PyObject_GC_Del(self);