Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit a6296d3

Browse filesBrowse files
methanemethane
authored
bpo-31095: fix potential crash during GC (GH-2974)
1 parent bf9075a commit a6296d3
Copy full SHA for a6296d3

File tree

Expand file treeCollapse file tree

14 files changed

+60
-13
lines changed
Filter options
Expand file treeCollapse file tree

14 files changed

+60
-13
lines changed

‎Doc/extending/newtypes.rst

Copy file name to clipboardExpand all lines: Doc/extending/newtypes.rst
+20-9Lines changed: 20 additions & 9 deletions
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 */

‎Doc/includes/noddy4.c

Copy file name to clipboardExpand all lines: Doc/includes/noddy4.c
+1Lines changed: 1 addition & 0 deletions
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
}

‎Misc/NEWS.d/next/Core and Builtins/2017-08-01-18-48-30.bpo-31095.bXWZDb.rst

Copy file name to clipboard
+2Lines changed: 2 additions & 0 deletions
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()``.

‎Modules/_collectionsmodule.c

Copy file name to clipboardExpand all lines: Modules/_collectionsmodule.c
+4Lines changed: 4 additions & 0 deletions
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
}

‎Modules/_elementtree.c

Copy file name to clipboardExpand all lines: Modules/_elementtree.c
+3-1Lines changed: 3 additions & 1 deletion
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

‎Modules/_functoolsmodule.c

Copy file name to clipboardExpand all lines: Modules/_functoolsmodule.c
+6-1Lines changed: 6 additions & 1 deletion
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);

‎Modules/_io/bytesio.c

Copy file name to clipboardExpand all lines: Modules/_io/bytesio.c
+2Lines changed: 2 additions & 0 deletions
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
}

‎Modules/_json.c

Copy file name to clipboardExpand all lines: Modules/_json.c
+4-2Lines changed: 4 additions & 2 deletions
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
}

‎Modules/_ssl.c

Copy file name to clipboardExpand all lines: Modules/_ssl.c
+3Lines changed: 3 additions & 0 deletions
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) {

‎Modules/_struct.c

Copy file name to clipboardExpand all lines: Modules/_struct.c
+2Lines changed: 2 additions & 0 deletions
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);

‎Objects/dictobject.c

Copy file name to clipboardExpand all lines: Objects/dictobject.c
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1847,6 +1847,8 @@ dict_dealloc(PyDictObject *mp)
18471847
PyObject **values = mp->ma_values;
18481848
PyDictKeysObject *keys = mp->ma_keys;
18491849
Py_ssize_t i, n;
1850+
1851+
/* bpo-31095: UnTrack is needed before calling any callbacks */
18501852
PyObject_GC_UnTrack(mp);
18511853
Py_TRASHCAN_SAFE_BEGIN(mp)
18521854
if (values != NULL) {
@@ -3270,6 +3272,8 @@ dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
32703272
static void
32713273
dictiter_dealloc(dictiterobject *di)
32723274
{
3275+
/* bpo-31095: UnTrack is needed before calling any callbacks */
3276+
_PyObject_GC_UNTRACK(di);
32733277
Py_XDECREF(di->di_dict);
32743278
Py_XDECREF(di->di_result);
32753279
PyObject_GC_Del(di);
@@ -3629,6 +3633,8 @@ dictiter_reduce(dictiterobject *di)
36293633
static void
36303634
dictview_dealloc(_PyDictViewObject *dv)
36313635
{
3636+
/* bpo-31095: UnTrack is needed before calling any callbacks */
3637+
_PyObject_GC_UNTRACK(dv);
36323638
Py_XDECREF(dv->dv_dict);
36333639
PyObject_GC_Del(dv);
36343640
}

‎Objects/setobject.c

Copy file name to clipboardExpand all lines: Objects/setobject.c
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,7 @@ set_dealloc(PySetObject *so)
553553
setentry *entry;
554554
Py_ssize_t used = so->used;
555555

556+
/* bpo-31095: UnTrack is needed before calling any callbacks */
556557
PyObject_GC_UnTrack(so);
557558
Py_TRASHCAN_SAFE_BEGIN(so)
558559
if (so->weakreflist != NULL)
@@ -809,6 +810,8 @@ typedef struct {
809810
static void
810811
setiter_dealloc(setiterobject *si)
811812
{
813+
/* bpo-31095: UnTrack is needed before calling any callbacks */
814+
_PyObject_GC_UNTRACK(si);
812815
Py_XDECREF(si->si_set);
813816
PyObject_GC_Del(si);
814817
}

‎Parser/asdl_c.py

Copy file name to clipboardExpand all lines: Parser/asdl_c.py
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,8 @@ def visitModule(self, mod):
633633
static void
634634
ast_dealloc(AST_object *self)
635635
{
636+
/* bpo-31095: UnTrack is needed before calling any callbacks */
637+
PyObject_GC_UnTrack(self);
636638
Py_CLEAR(self->dict);
637639
Py_TYPE(self)->tp_free(self);
638640
}

‎Python/Python-ast.c

Copy file name to clipboardExpand all lines: Python/Python-ast.c
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,8 @@ typedef struct {
520520
static void
521521
ast_dealloc(AST_object *self)
522522
{
523+
/* bpo-31095: UnTrack is needed before calling any callbacks */
524+
PyObject_GC_UnTrack(self);
523525
Py_CLEAR(self->dict);
524526
Py_TYPE(self)->tp_free(self);
525527
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.