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

10 files changed

lines changed

Original file line number Diff line number Diff line change
@@ -748,8 +748,9 @@ simplified::
748 748 uniformity across these boring implementations.
749 749
750 750 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 +::
753 754
754 755 static int
755 756 Noddy_clear(Noddy *self)
@@ -767,13 +768,6 @@ to use it::
767 768 return 0;
768 769 }
769 770
770 - static void
771 - Noddy_dealloc(Noddy* self)
772 - {
773 - Noddy_clear(self);
774 - self->ob_type->tp_free((PyObject*)self);
775 - }
776 -
777 771 Notice the use of a temporary variable in :c:func:`Noddy_clear`. We use the
778 772 temporary variable so that we can set each member to *NULL* before decrementing
779 773 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
796 790 return 0;
797 791 }
798 792
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 +
799 810 Finally, we add the :const:`Py_TPFLAGS_HAVE_GC` flag to the class flags::
800 811
801 812 Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE Py_TPFLAGS_HAVE_GC, /* tp_flags */
Original file line number Diff line number Diff line change
@@ -46,6 +46,7 @@ Noddy_clear(Noddy *self)
46 46 static void
47 47 Noddy_dealloc(Noddy* self)
48 48 {
49 +PyObject_GC_UnTrack(self);
49 50 Noddy_clear(self);
50 51 Py_TYPE(self)->tp_free((PyObject*)self);
51 52 }
Original file line number Diff line number Diff 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 number Diff line number Diff line change
@@ -1271,6 +1271,8 @@ dequeiter_traverse(dequeiterobject *dio, visitproc visit, void *arg)
1271 1271 static void
1272 1272 dequeiter_dealloc(dequeiterobject *dio)
1273 1273 {
1274 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1275 +PyObject_GC_UnTrack(dio);
1274 1276 Py_XDECREF(dio->deque);
1275 1277 PyObject_GC_Del(dio);
1276 1278 }
@@ -1556,6 +1558,8 @@ static PyMemberDef defdict_members[] = {
1556 1558 static void
1557 1559 defdict_dealloc(defdictobject *dd)
1558 1560 {
1561 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1562 +PyObject_GC_UnTrack(dd);
1559 1563 Py_CLEAR(dd->default_factory);
1560 1564 PyDict_Type.tp_dealloc((PyObject *)dd);
1561 1565 }
Original file line number Diff line number Diff line change
@@ -143,6 +143,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw)
143 143 static void
144 144 partial_dealloc(partialobject *pto)
145 145 {
146 +/* bpo-31095: UnTrack is needed before calling any callbacks */
146 147 PyObject_GC_UnTrack(pto);
147 148 if (pto->weakreflist != NULL)
148 149 PyObject_ClearWeakRefs((PyObject *) pto);
Original file line number Diff line number Diff line change
@@ -745,6 +745,7 @@ bytesio_setstate(bytesio *self, PyObject *state)
745 745 static void
746 746 bytesio_dealloc(bytesio *self)
747 747 {
748 +/* bpo-31095: UnTrack is needed before calling any callbacks */
748 749 _PyObject_GC_UNTRACK(self);
749 750 if (self->buf != NULL) {
750 751 PyMem_Free(self->buf);
Original file line number Diff line number Diff line change
@@ -840,7 +840,8 @@ py_encode_basestring_ascii(PyObject* self UNUSED, PyObject *pystr)
840 840 static void
841 841 scanner_dealloc(PyObject *self)
842 842 {
843 -/* Deallocate scanner object */
843 +/* bpo-31095: UnTrack is needed before calling any callbacks */
844 +PyObject_GC_UnTrack(self);
844 845 scanner_clear(self);
845 846 Py_TYPE(self)->tp_free(self);
846 847 }
@@ -2298,7 +2299,8 @@ encoder_listencode_list(PyEncoderObject *s, PyObject *rval, PyObject *seq, Py_ss
2298 2299 static void
2299 2300 encoder_dealloc(PyObject *self)
2300 2301 {
2301 -/* Deallocate Encoder */
2302 +/* bpo-31095: UnTrack is needed before calling any callbacks */
2303 +PyObject_GC_UnTrack(self);
2302 2304 encoder_clear(self);
2303 2305 Py_TYPE(self)->tp_free(self);
2304 2306 }
Original file line number Diff line number Diff line change
@@ -2214,6 +2214,8 @@ context_clear(PySSLContext *self)
2214 2214 static void
2215 2215 context_dealloc(PySSLContext *self)
2216 2216 {
2217 +/* bpo-31095: UnTrack is needed before calling any callbacks */
2218 +PyObject_GC_UnTrack(self);
2217 2219 context_clear(self);
2218 2220 SSL_CTX_free(self->ctx);
2219 2221 #ifdef OPENSSL_NPN_NEGOTIATED
Original file line number Diff line number Diff line change
@@ -1076,6 +1076,7 @@ dict_dealloc(register PyDictObject *mp)
1076 1076 {
1077 1077 register PyDictEntry *ep;
1078 1078 Py_ssize_t fill = mp->ma_fill;
1079 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1079 1080 PyObject_GC_UnTrack(mp);
1080 1081 Py_TRASHCAN_SAFE_BEGIN(mp)
1081 1082 for (ep = mp->ma_table; fill > 0; ep++) {
@@ -2576,6 +2577,8 @@ dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
2576 2577 static void
2577 2578 dictiter_dealloc(dictiterobject *di)
2578 2579 {
2580 +/* bpo-31095: UnTrack is needed before calling any callbacks */
2581 +_PyObject_GC_UNTRACK(di);
2579 2582 Py_XDECREF(di->di_dict);
2580 2583 Py_XDECREF(di->di_result);
2581 2584 PyObject_GC_Del(di);
@@ -2855,6 +2858,8 @@ typedef struct {
2855 2858 static void
2856 2859 dictview_dealloc(dictviewobject *dv)
2857 2860 {
2861 +/* bpo-31095: UnTrack is needed before calling any callbacks */
2862 +_PyObject_GC_UNTRACK(dv);
2858 2863 Py_XDECREF(dv->dv_dict);
2859 2864 PyObject_GC_Del(dv);
2860 2865 }
Original file line number Diff line number Diff line change
@@ -549,6 +549,7 @@ set_dealloc(PySetObject *so)
549 549 {
550 550 register setentry *entry;
551 551 Py_ssize_t fill = so->fill;
552 +/* bpo-31095: UnTrack is needed before calling any callbacks */
552 553 PyObject_GC_UnTrack(so);
553 554 Py_TRASHCAN_SAFE_BEGIN(so)
554 555 if (so->weakreflist != NULL)
@@ -811,6 +812,8 @@ typedef struct {
811 812 static void
812 813 setiter_dealloc(setiterobject *si)
813 814 {
815 +/* bpo-31095: UnTrack is needed before calling any callbacks */
816 +_PyObject_GC_UNTRACK(si);
814 817 Py_XDECREF(si->si_set);
815 818 PyObject_GC_Del(si);
816 819 }