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

14 files changed

lines changed

Original file line number Diff line number Diff line change
@@ -728,8 +728,9 @@ functions. With :c:func:`Py_VISIT`, :c:func:`Noddy_traverse` can be simplified:
728 728 uniformity across these boring implementations.
729 729
730 730 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 +::
733 734
734 735 static int
735 736 Noddy_clear(Noddy *self)
@@ -747,13 +748,6 @@ to use it::
747 748 return 0;
748 749 }
749 750
750 - static void
751 - Noddy_dealloc(Noddy* self)
752 - {
753 - Noddy_clear(self);
754 - Py_TYPE(self)->tp_free((PyObject*)self);
755 - }
756 -
757 751 Notice the use of a temporary variable in :c:func:`Noddy_clear`. We use the
758 752 temporary variable so that we can set each member to *NULL* before decrementing
759 753 its reference count. We do this because, as was discussed earlier, if the
@@ -776,6 +770,23 @@ be simplified::
776 770 return 0;
777 771 }
778 772
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 +
779 790 Finally, we add the :const:`Py_TPFLAGS_HAVE_GC` flag to the class flags::
780 791
781 792 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
@@ -1717,6 +1717,8 @@ dequeiter_traverse(dequeiterobject *dio, visitproc visit, void *arg)
1717 1717 static void
1718 1718 dequeiter_dealloc(dequeiterobject *dio)
1719 1719 {
1720 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1721 +PyObject_GC_UnTrack(dio);
1720 1722 Py_XDECREF(dio->deque);
1721 1723 PyObject_GC_Del(dio);
1722 1724 }
@@ -2097,6 +2099,8 @@ static PyMemberDef defdict_members[] = {
2097 2099 static void
2098 2100 defdict_dealloc(defdictobject *dd)
2099 2101 {
2102 +/* bpo-31095: UnTrack is needed before calling any callbacks */
2103 +PyObject_GC_UnTrack(dd);
2100 2104 Py_CLEAR(dd->default_factory);
2101 2105 PyDict_Type.tp_dealloc((PyObject *)dd);
2102 2106 }
Original file line number Diff line number Diff line change
@@ -627,6 +627,7 @@ element_gc_clear(ElementObject *self)
627 627 static void
628 628 element_dealloc(ElementObject* self)
629 629 {
630 +/* bpo-31095: UnTrack is needed before calling any callbacks */
630 631 PyObject_GC_UnTrack(self);
631 632 Py_TRASHCAN_SAFE_BEGIN(self)
632 633
@@ -2076,14 +2077,15 @@ elementiter_dealloc(ElementIterObject *it)
2076 2077 {
2077 2078 Py_ssize_t i = it->parent_stack_used;
2078 2079 it->parent_stack_used = 0;
2080 +/* bpo-31095: UnTrack is needed before calling any callbacks */
2081 +PyObject_GC_UnTrack(it);
2079 2082 while (i--)
2080 2083 Py_XDECREF(it->parent_stack[i].parent);
2081 2084 PyMem_Free(it->parent_stack);
2082 2085
2083 2086 Py_XDECREF(it->sought_tag);
2084 2087 Py_XDECREF(it->root_element);
2085 2088
2086 -PyObject_GC_UnTrack(it);
2087 2089 PyObject_GC_Del(it);
2088 2090 }
2089 2091
Original file line number Diff line number Diff line change
@@ -113,6 +113,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw)
113 113 static void
114 114 partial_dealloc(partialobject *pto)
115 115 {
116 +/* bpo-31095: UnTrack is needed before calling any callbacks */
116 117 PyObject_GC_UnTrack(pto);
117 118 if (pto->weakreflist != NULL)
118 119 PyObject_ClearWeakRefs((PyObject *) pto);
@@ -1073,7 +1074,11 @@ lru_cache_clear_list(lru_list_elem *link)
1073 1074 static void
1074 1075 lru_cache_dealloc(lru_cache_object *obj)
1075 1076 {
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);
1077 1082 Py_XDECREF(obj->maxsize_O);
1078 1083 Py_XDECREF(obj->func);
1079 1084 Py_XDECREF(obj->cache);
Original file line number Diff line number Diff line change
@@ -1084,6 +1084,8 @@ bytesiobuf_traverse(bytesiobuf *self, visitproc visit, void *arg)
1084 1084 static void
1085 1085 bytesiobuf_dealloc(bytesiobuf *self)
1086 1086 {
1087 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1088 +PyObject_GC_UnTrack(self);
1087 1089 Py_CLEAR(self->source);
1088 1090 Py_TYPE(self)->tp_free(self);
1089 1091 }
Original file line number Diff line number Diff line change
@@ -655,7 +655,8 @@ py_encode_basestring(PyObject* self UNUSED, PyObject *pystr)
655 655 static void
656 656 scanner_dealloc(PyObject *self)
657 657 {
658 -/* Deallocate scanner object */
658 +/* bpo-31095: UnTrack is needed before calling any callbacks */
659 +PyObject_GC_UnTrack(self);
659 660 scanner_clear(self);
660 661 Py_TYPE(self)->tp_free(self);
661 662 }
@@ -1778,7 +1779,8 @@ encoder_listencode_list(PyEncoderObject *s, _PyAccu *acc,
1778 1779 static void
1779 1780 encoder_dealloc(PyObject *self)
1780 1781 {
1781 -/* Deallocate Encoder */
1782 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1783 +PyObject_GC_UnTrack(self);
1782 1784 encoder_clear(self);
1783 1785 Py_TYPE(self)->tp_free(self);
1784 1786 }
Original file line number Diff line number Diff line change
@@ -2778,6 +2778,8 @@ context_clear(PySSLContext *self)
2778 2778 static void
2779 2779 context_dealloc(PySSLContext *self)
2780 2780 {
2781 +/* bpo-31095: UnTrack is needed before calling any callbacks */
2782 +PyObject_GC_UnTrack(self);
2781 2783 context_clear(self);
2782 2784 SSL_CTX_free(self->ctx);
2783 2785 #ifdef OPENSSL_NPN_NEGOTIATED
@@ -4292,6 +4294,7 @@ static PyTypeObject PySSLMemoryBIO_Type = {
4292 4294 static void
4293 4295 PySSLSession_dealloc(PySSLSession *self)
4294 4296 {
4297 +/* bpo-31095: UnTrack is needed before calling any callbacks */
4295 4298 PyObject_GC_UnTrack(self);
4296 4299 Py_XDECREF(self->ctx);
4297 4300 if (self->session != NULL) {
Original file line number Diff line number Diff line change
@@ -1589,6 +1589,8 @@ typedef struct {
1589 1589 static void
1590 1590 unpackiter_dealloc(unpackiterobject *self)
1591 1591 {
1592 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1593 +PyObject_GC_UnTrack(self);
1592 1594 Py_XDECREF(self->so);
1593 1595 PyBuffer_Release(&self->buf);
1594 1596 PyObject_GC_Del(self);
Original file line number Diff line number Diff line change
@@ -1847,6 +1847,8 @@ dict_dealloc(PyDictObject *mp)
1847 1847 PyObject **values = mp->ma_values;
1848 1848 PyDictKeysObject *keys = mp->ma_keys;
1849 1849 Py_ssize_t i, n;
1850 +
1851 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1850 1852 PyObject_GC_UnTrack(mp);
1851 1853 Py_TRASHCAN_SAFE_BEGIN(mp)
1852 1854 if (values != NULL) {
@@ -3270,6 +3272,8 @@ dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
3270 3272 static void
3271 3273 dictiter_dealloc(dictiterobject *di)
3272 3274 {
3275 +/* bpo-31095: UnTrack is needed before calling any callbacks */
3276 +_PyObject_GC_UNTRACK(di);
3273 3277 Py_XDECREF(di->di_dict);
3274 3278 Py_XDECREF(di->di_result);
3275 3279 PyObject_GC_Del(di);
@@ -3629,6 +3633,8 @@ dictiter_reduce(dictiterobject *di)
3629 3633 static void
3630 3634 dictview_dealloc(_PyDictViewObject *dv)
3631 3635 {
3636 +/* bpo-31095: UnTrack is needed before calling any callbacks */
3637 +_PyObject_GC_UNTRACK(dv);
3632 3638 Py_XDECREF(dv->dv_dict);
3633 3639 PyObject_GC_Del(dv);
3634 3640 }
Original file line number Diff line number Diff line change
@@ -553,6 +553,7 @@ set_dealloc(PySetObject *so)
553 553 setentry *entry;
554 554 Py_ssize_t used = so->used;
555 555
556 +/* bpo-31095: UnTrack is needed before calling any callbacks */
556 557 PyObject_GC_UnTrack(so);
557 558 Py_TRASHCAN_SAFE_BEGIN(so)
558 559 if (so->weakreflist != NULL)
@@ -809,6 +810,8 @@ typedef struct {
809 810 static void
810 811 setiter_dealloc(setiterobject *si)
811 812 {
813 +/* bpo-31095: UnTrack is needed before calling any callbacks */
814 +_PyObject_GC_UNTRACK(si);
812 815 Py_XDECREF(si->si_set);
813 816 PyObject_GC_Del(si);
814 817 }
Original file line number Diff line number Diff line change
@@ -633,6 +633,8 @@ def visitModule(self, mod):
633 633 static void
634 634 ast_dealloc(AST_object *self)
635 635 {
636 + /* bpo-31095: UnTrack is needed before calling any callbacks */
637 + PyObject_GC_UnTrack(self);
636 638 Py_CLEAR(self->dict);
637 639 Py_TYPE(self)->tp_free(self);
638 640 }
Original file line number Diff line number Diff line change
@@ -520,6 +520,8 @@ typedef struct {
520 520 static void
521 521 ast_dealloc(AST_object *self)
522 522 {
523 +/* bpo-31095: UnTrack is needed before calling any callbacks */
524 +PyObject_GC_UnTrack(self);
523 525 Py_CLEAR(self->dict);
524 526 Py_TYPE(self)->tp_free(self);
525 527 }