Home Original page

bpo-31095: Fix potential crash during GC (GH-3197) · python/cpython@4cde4bd

10 files changed

lines changed

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 */

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

@@ -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

}

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);

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);

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

}

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

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

}

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

}