bpo-39778: Don't traverse weak-reference lists OrderedDict's tp_traverse and tp_clear (GH-18749)

miss-islington · pablogsal · web-flow · commit 69ded3944c20 · 2020-03-02T15:53:03.000-08:00
Objects do not own weak references to them directly through the __weakref__ list so these do not need to be traversed by the GC. (cherry picked from commit 0c2b509) Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py
@@ -749,6 +749,26 @@ def test_iterators_pickling(self):
                     self.assertEqual(list(unpickled), expected)
                     self.assertEqual(list(it), expected)
 
+    @support.cpython_only
+    def test_weakref_list_is_not_traversed(self):
+        # Check that the weakref list is not traversed when collecting
+        # OrderedDict objects. See bpo-39778 for more information.
+
+        gc.collect()
+
+        x = self.OrderedDict()
+        x.cycle = x
+
+        cycle = []
+        cycle.append(cycle)
+
+        x_ref = weakref.ref(x)
+        cycle.append(x_ref)
+
+        del x, cycle, x_ref
+
+        gc.collect()
+
 
 class PurePythonOrderedDictSubclassTests(PurePythonOrderedDictTests):
 
diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-03-02-19-21-21.bpo-39778._YGLEc.rst b/Misc/NEWS.d/next/Core and Builtins/2020-03-02-19-21-21.bpo-39778._YGLEc.rst
@@ -0,0 +1,2 @@
+Fixed a crash due to incorrect handling of weak references in
+``collections.OrderedDict`` classes. Patch by Pablo Galindo.
diff --git a/Objects/odictobject.c b/Objects/odictobject.c
@@ -1463,7 +1463,6 @@ odict_traverse(PyODictObject *od, visitproc visit, void *arg)
     _ODictNode *node;
 
     Py_VISIT(od->od_inst_dict);
-    Py_VISIT(od->od_weakreflist);
     _odict_FOREACH(od, node) {
         Py_VISIT(_odictnode_KEY(node));
     }
@@ -1476,7 +1475,6 @@ static int
 odict_tp_clear(PyODictObject *od)
 {
     Py_CLEAR(od->od_inst_dict);
-    Py_CLEAR(od->od_weakreflist);
     PyDict_Clear((PyObject *)od);
     _odict_clear_nodes(od);
     return 0;

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Fixed a crash due to incorrect handling of weak references in`
	`2`	+``collections.OrderedDict`` classes. Patch by Pablo Galindo.
Original file line number	Diff line number	Diff line change
`@@ -1463,7 +1463,6 @@ odict_traverse(PyODictObject od, visitproc visit, void arg)`
`1463`	`1463`	`_ODictNode *node;`
`1464`	`1464`
`1465`	`1465`	`Py_VISIT(od->od_inst_dict);`
`1466`		`- Py_VISIT(od->od_weakreflist);`
`1467`	`1466`	`_odict_FOREACH(od, node) {`
`1468`	`1467`	`Py_VISIT(_odictnode_KEY(node));`
`1469`	`1468`	`}`
`@@ -1476,7 +1475,6 @@ static int`
`1476`	`1475`	`odict_tp_clear(PyODictObject *od)`
`1477`	`1476`	`{`
`1478`	`1477`	`Py_CLEAR(od->od_inst_dict);`
`1479`		`- Py_CLEAR(od->od_weakreflist);`
`1480`	`1478`	`PyDict_Clear((PyObject *)od);`
`1481`	`1479`	`_odict_clear_nodes(od);`
`1482`	`1480`	`return 0;`