Home Original page

bpo-31095: fix potential crash during GC (GH-2974) · python/cpython@a6296d3

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

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

17171717

static void

17181718

dequeiter_dealloc(dequeiterobject *dio)

17191719

{

1720+

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

1721+

PyObject_GC_UnTrack(dio);

17201722

Py_XDECREF(dio->deque);

17211723

PyObject_GC_Del(dio);

17221724

}

@@ -2097,6 +2099,8 @@ static PyMemberDef defdict_members[] = {

20972099

static void

20982100

defdict_dealloc(defdictobject *dd)

20992101

{

2102+

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

2103+

PyObject_GC_UnTrack(dd);

21002104

Py_CLEAR(dd->default_factory);

21012105

PyDict_Type.tp_dealloc((PyObject *)dd);

21022106

}

Original file line numberDiff line numberDiff line change

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

627627

static void

628628

element_dealloc(ElementObject* self)

629629

{

630+

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

630631

PyObject_GC_UnTrack(self);

631632

Py_TRASHCAN_SAFE_BEGIN(self)

632633

@@ -2076,14 +2077,15 @@ elementiter_dealloc(ElementIterObject *it)

20762077

{

20772078

Py_ssize_t i = it->parent_stack_used;

20782079

it->parent_stack_used = 0;

2080+

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

2081+

PyObject_GC_UnTrack(it);

20792082

while (i--)

20802083

Py_XDECREF(it->parent_stack[i].parent);

20812084

PyMem_Free(it->parent_stack);

20822085
20832086

Py_XDECREF(it->sought_tag);

20842087

Py_XDECREF(it->root_element);

20852088
2086-

PyObject_GC_UnTrack(it);

20872089

PyObject_GC_Del(it);

20882090

}

20892091
Original file line numberDiff line numberDiff line change

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

113113

static void

114114

partial_dealloc(partialobject *pto)

115115

{

116+

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

116117

PyObject_GC_UnTrack(pto);

117118

if (pto->weakreflist != NULL)

118119

PyObject_ClearWeakRefs((PyObject *) pto);

@@ -1073,7 +1074,11 @@ lru_cache_clear_list(lru_list_elem *link)

10731074

static void

10741075

lru_cache_dealloc(lru_cache_object *obj)

10751076

{

1076-

lru_list_elem *list = lru_cache_unlink_list(obj);

1077+

lru_list_elem *list;

1078+

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

1079+

PyObject_GC_UnTrack(obj);

1080+
1081+

list = lru_cache_unlink_list(obj);

10771082

Py_XDECREF(obj->maxsize_O);

10781083

Py_XDECREF(obj->func);

10791084

Py_XDECREF(obj->cache);

Original file line numberDiff line numberDiff line change

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

10841084

static void

10851085

bytesiobuf_dealloc(bytesiobuf *self)

10861086

{

1087+

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

1088+

PyObject_GC_UnTrack(self);

10871089

Py_CLEAR(self->source);

10881090

Py_TYPE(self)->tp_free(self);

10891091

}

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

}

@@ -1778,7 +1779,8 @@ encoder_listencode_list(PyEncoderObject *s, _PyAccu *acc,

17781779

static void

17791780

encoder_dealloc(PyObject *self)

17801781

{

1781-

/* Deallocate Encoder */

1782+

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

1783+

PyObject_GC_UnTrack(self);

17821784

encoder_clear(self);

17831785

Py_TYPE(self)->tp_free(self);

17841786

}

Original file line numberDiff line numberDiff line change

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

27782778

static void

27792779

context_dealloc(PySSLContext *self)

27802780

{

2781+

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

2782+

PyObject_GC_UnTrack(self);

27812783

context_clear(self);

27822784

SSL_CTX_free(self->ctx);

27832785

#ifdef OPENSSL_NPN_NEGOTIATED

@@ -4292,6 +4294,7 @@ static PyTypeObject PySSLMemoryBIO_Type = {

42924294

static void

42934295

PySSLSession_dealloc(PySSLSession *self)

42944296

{

4297+

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

42954298

PyObject_GC_UnTrack(self);

42964299

Py_XDECREF(self->ctx);

42974300

if (self->session != NULL) {

Original file line numberDiff line numberDiff line change

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

15891589

static void

15901590

unpackiter_dealloc(unpackiterobject *self)

15911591

{

1592+

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

1593+

PyObject_GC_UnTrack(self);

15921594

Py_XDECREF(self->so);

15931595

PyBuffer_Release(&self->buf);

15941596

PyObject_GC_Del(self);