bpo-31095: fix potential crash during GC (GH-3195) · python/cpython@2eea952 (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
@@ -1706,6 +1706,8 @@ dequeiter_traverse(dequeiterobject *dio, visitproc visit, void *arg)
1706 1706 static void
1707 1707 dequeiter_dealloc(dequeiterobject *dio)
1708 1708 {
1709 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1710 +PyObject_GC_UnTrack(dio);
1709 1711 Py_XDECREF(dio->deque);
1710 1712 PyObject_GC_Del(dio);
1711 1713 }
@@ -2086,6 +2088,8 @@ static PyMemberDef defdict_members[] = {
2086 2088 static void
2087 2089 defdict_dealloc(defdictobject *dd)
2088 2090 {
2091 +/* bpo-31095: UnTrack is needed before calling any callbacks */
2092 +PyObject_GC_UnTrack(dd);
2089 2093 Py_CLEAR(dd->default_factory);
2090 2094 PyDict_Type.tp_dealloc((PyObject *)dd);
2091 2095 }
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
@@ -2048,14 +2049,15 @@ elementiter_dealloc(ElementIterObject *it)
2048 2049 {
2049 2050 Py_ssize_t i = it->parent_stack_used;
2050 2051 it->parent_stack_used = 0;
2052 +/* bpo-31095: UnTrack is needed before calling any callbacks */
2053 +PyObject_GC_UnTrack(it);
2051 2054 while (i--)
2052 2055 Py_XDECREF(it->parent_stack[i].parent);
2053 2056 PyMem_Free(it->parent_stack);
2054 2057
2055 2058 Py_XDECREF(it->sought_tag);
2056 2059 Py_XDECREF(it->root_element);
2057 2060
2058 -PyObject_GC_UnTrack(it);
2059 2061 PyObject_GC_Del(it);
2060 2062 }
2061 2063
Original file line number Diff line number Diff line change
@@ -116,6 +116,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw)
116 116 static void
117 117 partial_dealloc(partialobject *pto)
118 118 {
119 +/* bpo-31095: UnTrack is needed before calling any callbacks */
119 120 PyObject_GC_UnTrack(pto);
120 121 if (pto->weakreflist != NULL)
121 122 PyObject_ClearWeakRefs((PyObject *) pto);
@@ -1038,7 +1039,11 @@ lru_cache_clear_list(lru_list_elem *link)
1038 1039 static void
1039 1040 lru_cache_dealloc(lru_cache_object *obj)
1040 1041 {
1041 -lru_list_elem *list = lru_cache_unlink_list(obj);
1042 +lru_list_elem *list;
1043 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1044 +PyObject_GC_UnTrack(obj);
1045 +
1046 +list = lru_cache_unlink_list(obj);
1042 1047 Py_XDECREF(obj->maxsize_O);
1043 1048 Py_XDECREF(obj->func);
1044 1049 Py_XDECREF(obj->cache);
Original file line number Diff line number Diff line change
@@ -1131,6 +1131,8 @@ bytesiobuf_traverse(bytesiobuf *self, visitproc visit, void *arg)
1131 1131 static void
1132 1132 bytesiobuf_dealloc(bytesiobuf *self)
1133 1133 {
1134 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1135 +PyObject_GC_UnTrack(self);
1134 1136 Py_CLEAR(self->source);
1135 1137 Py_TYPE(self)->tp_free(self);
1136 1138 }
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 }
@@ -1793,7 +1794,8 @@ encoder_listencode_list(PyEncoderObject *s, _PyAccu *acc,
1793 1794 static void
1794 1795 encoder_dealloc(PyObject *self)
1795 1796 {
1796 -/* Deallocate Encoder */
1797 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1798 +PyObject_GC_UnTrack(self);
1797 1799 encoder_clear(self);
1798 1800 Py_TYPE(self)->tp_free(self);
1799 1801 }
Original file line number Diff line number Diff line change
@@ -2776,6 +2776,8 @@ context_clear(PySSLContext *self)
2776 2776 static void
2777 2777 context_dealloc(PySSLContext *self)
2778 2778 {
2779 +/* bpo-31095: UnTrack is needed before calling any callbacks */
2780 +PyObject_GC_UnTrack(self);
2779 2781 context_clear(self);
2780 2782 SSL_CTX_free(self->ctx);
2781 2783 #ifdef OPENSSL_NPN_NEGOTIATED
@@ -4284,6 +4286,7 @@ static PyTypeObject PySSLMemoryBIO_Type = {
4284 4286 static void
4285 4287 PySSLSession_dealloc(PySSLSession *self)
4286 4288 {
4289 +/* bpo-31095: UnTrack is needed before calling any callbacks */
4287 4290 PyObject_GC_UnTrack(self);
4288 4291 Py_XDECREF(self->ctx);
4289 4292 if (self->session != NULL) {
Original file line number Diff line number Diff line change
@@ -1605,6 +1605,8 @@ typedef struct {
1605 1605 static void
1606 1606 unpackiter_dealloc(unpackiterobject *self)
1607 1607 {
1608 +/* bpo-31095: UnTrack is needed before calling any callbacks */
1609 +PyObject_GC_UnTrack(self);
1608 1610 Py_XDECREF(self->so);
1609 1611 PyBuffer_Release(&self->buf);
1610 1612 PyObject_GC_Del(self);
Original file line number Diff line number Diff line change
@@ -2006,6 +2006,8 @@ dict_dealloc(PyDictObject *mp)
2006 2006 PyObject **values = mp->ma_values;
2007 2007 PyDictKeysObject *keys = mp->ma_keys;
2008 2008 Py_ssize_t i, n;
2009 +
2010 +/* bpo-31095: UnTrack is needed before calling any callbacks */
2009 2011 PyObject_GC_UnTrack(mp);
2010 2012 Py_TRASHCAN_SAFE_BEGIN(mp)
2011 2013 if (values != NULL) {
@@ -3432,6 +3434,8 @@ dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
3432 3434 static void
3433 3435 dictiter_dealloc(dictiterobject *di)
3434 3436 {
3437 +/* bpo-31095: UnTrack is needed before calling any callbacks */
3438 +_PyObject_GC_UNTRACK(di);
3435 3439 Py_XDECREF(di->di_dict);
3436 3440 Py_XDECREF(di->di_result);
3437 3441 PyObject_GC_Del(di);
@@ -3800,6 +3804,8 @@ dictiter_reduce(dictiterobject *di)
3800 3804 static void
3801 3805 dictview_dealloc(_PyDictViewObject *dv)
3802 3806 {
3807 +/* bpo-31095: UnTrack is needed before calling any callbacks */
3808 +_PyObject_GC_UNTRACK(dv);
3803 3809 Py_XDECREF(dv->dv_dict);
3804 3810 PyObject_GC_Del(dv);
3805 3811 }
Original file line number Diff line number Diff line change
@@ -556,6 +556,7 @@ set_dealloc(PySetObject *so)
556 556 setentry *entry;
557 557 Py_ssize_t used = so->used;
558 558
559 +/* bpo-31095: UnTrack is needed before calling any callbacks */
559 560 PyObject_GC_UnTrack(so);
560 561 Py_TRASHCAN_SAFE_BEGIN(so)
561 562 if (so->weakreflist != NULL)
@@ -812,6 +813,8 @@ typedef struct {
812 813 static void
813 814 setiter_dealloc(setiterobject *si)
814 815 {
816 +/* bpo-31095: UnTrack is needed before calling any callbacks */
817 +_PyObject_GC_UNTRACK(si);
815 818 Py_XDECREF(si->si_set);
816 819 PyObject_GC_Del(si);
817 820 }
Original file line number Diff line number Diff line change
@@ -630,6 +630,8 @@ def visitModule(self, mod):
630 630 static void
631 631 ast_dealloc(AST_object *self)
632 632 {
633 + /* bpo-31095: UnTrack is needed before calling any callbacks */
634 + PyObject_GC_UnTrack(self);
633 635 Py_CLEAR(self->dict);
634 636 Py_TYPE(self)->tp_free(self);
635 637 }
Original file line number Diff line number Diff line change
@@ -512,6 +512,8 @@ typedef struct {
512 512 static void
513 513 ast_dealloc(AST_object *self)
514 514 {
515 +/* bpo-31095: UnTrack is needed before calling any callbacks */
516 +PyObject_GC_UnTrack(self);
515 517 Py_CLEAR(self->dict);
516 518 Py_TYPE(self)->tp_free(self);
517 519 }