From aa687902f21dc32a72f578a992cc9e44444ced44 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Thu, 1 Jan 2009 14:11:22 +0000 Subject: [PATCH] Issue #3680: Reference cycles created through a dict, set or deque iterator did not get collected. --- Lib/test/test_deque.py | 21 +++++++++++++++++++-- Lib/test/test_dict.py | 14 ++++++++++++++ Lib/test/test_set.py | 17 +++++++++++++++-- Misc/NEWS | 3 +++ Modules/_collectionsmodule.c | 23 ++++++++++++++++------- Objects/dictobject.c | 25 +++++++++++++++++-------- Objects/setobject.c | 16 ++++++++++++---- 7 files changed, 96 insertions(+), 23 deletions(-) diff --git a/Lib/test/test_deque.py b/Lib/test/test_deque.py index 0f0d09847e6..4e2de3d0058 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 f715657d366..14d62f5024b 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 d38a6759075..8d057123651 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 9639b1897f2..8ab8dd2e74f 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 da000d0e80b..37633d2dda8 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 f2ef4b0d58b..f4d86835e95 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 ea3970e9b70..a55bbb70adb 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; }