Issue #3680: Reference cycles created through a dict, set or deque iterator did not get collected.

7 files changed, 96 insertions, 23 deletions

diff --git a/Lib/test/test_deque.py b/Lib/test/test_deque.py
index 0f0d098..4e2de3d 100644
--- a/Lib/test/test_deque.py
+++ b/Lib/test/test_deque.py
@@ -1,7 +1,8 @@
 from collections import deque
 import unittest
 from test import test_support, seq_tests
-from weakref import proxy
+import gc
+import weakref
 import copy
 import cPickle as pickle
 import random
@@ -418,6 +419,22 @@ class TestBasic(unittest.TestCase):
             d.append(1)
             gc.collect()
 
+    def test_container_iterator(self):
+        # Bug # XXX: tp_traverse was not implemented for deque iterator objects
+        class C(object):
+            pass
+        for i in range(2):
+            obj = C()
+            ref = weakref.ref(obj)
+            if i == 0:
+                container = deque([obj, 1])
+            else:
+                container = reversed(deque([obj, 1]))
+            obj.x = iter(container)
+            del obj, container
+            gc.collect()
+            self.assert_(ref() is None, "Cycle was not collected")
+
 class TestVariousIteratorArgs(unittest.TestCase):
 
     def test_constructor(self):
@@ -528,7 +545,7 @@ class TestSubclass(unittest.TestCase):
 
     def test_weakref(self):
         d = deque('gallahad')
-        p = proxy(d)
+        p = weakref.proxy(d)
         self.assertEqual(str(p), str(d))
         d = None
         self.assertRaises(ReferenceError, str, p)
diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py
index f715657..14d62f5 100644
--- a/Lib/test/test_dict.py
+++ b/Lib/test/test_dict.py
@@ -2,6 +2,7 @@ import unittest
 from test import test_support
 
 import UserDict, random, string
+import gc, weakref
 
 
 class DictTest(unittest.TestCase):
@@ -554,6 +555,19 @@ class DictTest(unittest.TestCase):
             pass
         d = {}
 
+    def test_container_iterator(self):
+        # Bug # XXX: tp_traverse was not implemented for dictiter objects
+        class C(object):
+            pass
+        iterators = (dict.iteritems, dict.itervalues, dict.iterkeys)
+        for i in iterators:
+            obj = C()
+            ref = weakref.ref(obj)
+            container = {obj: 1}
+            obj.x = i(container)
+            del obj, container
+            gc.collect()
+            self.assert_(ref() is None, "Cycle was not collected")
 
 
 from test import mapping_tests
diff --git a/Lib/test/test_set.py b/Lib/test/test_set.py
index d38a675..8d05712 100644
--- a/Lib/test/test_set.py
+++ b/Lib/test/test_set.py
@@ -1,6 +1,7 @@
 import unittest
 from test import test_support
-from weakref import proxy
+import gc
+import weakref
 import operator
 import copy
 import pickle
@@ -322,6 +323,18 @@ class TestJointOps(unittest.TestCase):
         self.assertEqual(sum(elem.hash_count for elem in d), n)
         self.assertEqual(d3, dict.fromkeys(d, 123))
 
+    def test_container_iterator(self):
+        # Bug # XXX: tp_traverse was not implemented for set iterator object
+        class C(object):
+            pass
+        obj = C()
+        ref = weakref.ref(obj)
+        container = set([obj, 1])
+        obj.x = iter(container)
+        del obj, container
+        gc.collect()
+        self.assert_(ref() is None, "Cycle was not collected")
+
 class TestSet(TestJointOps):
     thetype = set
 
@@ -538,7 +551,7 @@ class TestSet(TestJointOps):
 
     def test_weakref(self):
         s = self.thetype('gallahad')
-        p = proxy(s)
+        p = weakref.proxy(s)
         self.assertEqual(str(p), str(s))
         s = None
         self.assertRaises(ReferenceError, str, p)
diff --git a/Misc/NEWS b/Misc/NEWS
index 9639b18..8ab8dd2 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,9 @@ What's New in Python 2.7 alpha 1
 Core and Builtins
 -----------------
 
+- Issue #3680: Reference cycles created through a dict, set or deque iterator
+  did not get collected.
+
 - Issue #4701: PyObject_Hash now implicitly calls PyType_Ready on types
   where the tp_hash and tp_dict slots are both NULL.
 
diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c
index da000d0..37633d2 100644
--- a/Modules/_collectionsmodule.c
+++ b/Modules/_collectionsmodule.c
@@ -958,7 +958,7 @@ deque_iter(dequeobject *deque)
 {
 	dequeiterobject *it;
 
-	it = PyObject_New(dequeiterobject, &dequeiter_type);
+	it = PyObject_GC_New(dequeiterobject, &dequeiter_type);
 	if (it == NULL)
 		return NULL;
 	it->b = deque->leftblock;
@@ -967,14 +967,22 @@ deque_iter(dequeobject *deque)
 	it->deque = deque;
 	it->state = deque->state;
 	it->counter = deque->len;
+	_PyObject_GC_TRACK(it);
 	return (PyObject *)it;
 }
 
+static int
+dequeiter_traverse(dequeiterobject *dio, visitproc visit, void *arg)
+{
+	Py_VISIT(dio->deque);
+	return 0;
+}
+
 static void
 dequeiter_dealloc(dequeiterobject *dio)
 {
 	Py_XDECREF(dio->deque);
-	Py_TYPE(dio)->tp_free(dio);
+	PyObject_GC_Del(dio);
 }
 
 static PyObject *
@@ -1039,9 +1047,9 @@ static PyTypeObject dequeiter_type = {
 	PyObject_GenericGetAttr,		/* tp_getattro */
 	0,					/* tp_setattro */
 	0,					/* tp_as_buffer */
-	Py_TPFLAGS_DEFAULT,			/* tp_flags */
+	Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,/* tp_flags */
 	0,					/* tp_doc */
-	0,					/* tp_traverse */
+	(traverseproc)dequeiter_traverse,	/* tp_traverse */
 	0,					/* tp_clear */
 	0,					/* tp_richcompare */
 	0,					/* tp_weaklistoffset */
@@ -1060,7 +1068,7 @@ deque_reviter(dequeobject *deque)
 {
 	dequeiterobject *it;
 
-	it = PyObject_New(dequeiterobject, &dequereviter_type);
+	it = PyObject_GC_New(dequeiterobject, &dequereviter_type);
 	if (it == NULL)
 		return NULL;
 	it->b = deque->rightblock;
@@ -1069,6 +1077,7 @@ deque_reviter(dequeobject *deque)
 	it->deque = deque;
 	it->state = deque->state;
 	it->counter = deque->len;
+	_PyObject_GC_TRACK(it);
 	return (PyObject *)it;
 }
 
@@ -1121,9 +1130,9 @@ static PyTypeObject dequereviter_type = {
 	PyObject_GenericGetAttr,		/* tp_getattro */
 	0,					/* tp_setattro */
 	0,					/* tp_as_buffer */
-	Py_TPFLAGS_DEFAULT,			/* tp_flags */
+	Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,/* tp_flags */
 	0,					/* tp_doc */
-	0,					/* tp_traverse */
+	(traverseproc)dequeiter_traverse,	/* tp_traverse */
 	0,					/* tp_clear */
 	0,					/* tp_richcompare */
 	0,					/* tp_weaklistoffset */
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index f2ef4b0..f4d8683 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -2331,7 +2331,7 @@ static PyObject *
 dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
 {
 	dictiterobject *di;
-	di = PyObject_New(dictiterobject, itertype);
+	di = PyObject_GC_New(dictiterobject, itertype);
 	if (di == NULL)
 		return NULL;
 	Py_INCREF(dict);
@@ -2348,6 +2348,7 @@ dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
 	}
 	else
 		di->di_result = NULL;
+	_PyObject_GC_TRACK(di);
 	return (PyObject *)di;
 }
 
@@ -2356,7 +2357,15 @@ dictiter_dealloc(dictiterobject *di)
 {
 	Py_XDECREF(di->di_dict);
 	Py_XDECREF(di->di_result);
-	PyObject_Del(di);
+	PyObject_GC_Del(di);
+}
+
+static int
+dictiter_traverse(dictiterobject *di, visitproc visit, void *arg)
+{
+	Py_VISIT(di->di_dict);
+	Py_VISIT(di->di_result);
+	return 0;
 }
 
 static PyObject *
@@ -2435,9 +2444,9 @@ PyTypeObject PyDictIterKey_Type = {
 	PyObject_GenericGetAttr,		/* tp_getattro */
 	0,					/* tp_setattro */
 	0,					/* tp_as_buffer */
-	Py_TPFLAGS_DEFAULT,			/* tp_flags */
+	Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,/* tp_flags */
  	0,					/* tp_doc */
- 	0,					/* tp_traverse */
+ 	(traverseproc)dictiter_traverse,	/* tp_traverse */
  	0,					/* tp_clear */
 	0,					/* tp_richcompare */
 	0,					/* tp_weaklistoffset */
@@ -2507,9 +2516,9 @@ PyTypeObject PyDictIterValue_Type = {
 	PyObject_GenericGetAttr,		/* tp_getattro */
 	0,					/* tp_setattro */
 	0,					/* tp_as_buffer */
-	Py_TPFLAGS_DEFAULT,			/* tp_flags */
+	Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,/* tp_flags */
  	0,					/* tp_doc */
- 	0,					/* tp_traverse */
+ 	(traverseproc)dictiter_traverse,	/* tp_traverse */
  	0,					/* tp_clear */
 	0,					/* tp_richcompare */
 	0,					/* tp_weaklistoffset */
@@ -2593,9 +2602,9 @@ PyTypeObject PyDictIterItem_Type = {
 	PyObject_GenericGetAttr,		/* tp_getattro */
 	0,					/* tp_setattro */
 	0,					/* tp_as_buffer */
-	Py_TPFLAGS_DEFAULT,			/* tp_flags */
+	Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,/* tp_flags */
  	0,					/* tp_doc */
- 	0,					/* tp_traverse */
+ 	(traverseproc)dictiter_traverse,	/* tp_traverse */
  	0,					/* tp_clear */
 	0,					/* tp_richcompare */
 	0,					/* tp_weaklistoffset */
diff --git a/Objects/setobject.c b/Objects/setobject.c
index ea3970e..a55bbb7 100644
--- a/Objects/setobject.c
+++ b/Objects/setobject.c
@@ -810,7 +810,14 @@ static void
 setiter_dealloc(setiterobject *si)
 {
 	Py_XDECREF(si->si_set);
-	PyObject_Del(si);
+	PyObject_GC_Del(si);
+}
+
+static int
+setiter_traverse(setiterobject *si, visitproc visit, void *arg)
+{
+	Py_VISIT(si->si_set);
+	return 0;
 }
 
 static PyObject *
@@ -888,9 +895,9 @@ static PyTypeObject PySetIter_Type = {
 	PyObject_GenericGetAttr,		/* tp_getattro */
 	0,					/* tp_setattro */
 	0,					/* tp_as_buffer */
-	Py_TPFLAGS_DEFAULT,			/* tp_flags */
+	Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,/* tp_flags */
  	0,					/* tp_doc */
- 	0,					/* tp_traverse */
+ 	(traverseproc)setiter_traverse,		/* tp_traverse */
  	0,					/* tp_clear */
 	0,					/* tp_richcompare */
 	0,					/* tp_weaklistoffset */
@@ -903,7 +910,7 @@ static PyTypeObject PySetIter_Type = {
 static PyObject *
 set_iter(PySetObject *so)
 {
-	setiterobject *si = PyObject_New(setiterobject, &PySetIter_Type);
+	setiterobject *si = PyObject_GC_New(setiterobject, &PySetIter_Type);
 	if (si == NULL)
 		return NULL;
 	Py_INCREF(so);
@@ -911,6 +918,7 @@ set_iter(PySetObject *so)
 	si->si_used = so->used;
 	si->si_pos = 0;
 	si->len = so->used;
+	_PyObject_GC_TRACK(si);
 	return (PyObject *)si;
 }