From d205d0145c8f5a37d0a46261eb0193e27b5b0ad8 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 19 Jan 2016 14:46:25 +0200 Subject: [PATCH] Issue #25935: Garbage collector now breaks reference loops with OrderedDict. --- Lib/test/test_ordered_dict.py | 13 +++++++++++++ Misc/NEWS | 2 ++ Objects/odictobject.c | 27 ++++++++++++++------------- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index 9d6c6ffa221..8ab0a9fbd16 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -1,10 +1,12 @@ import contextlib import copy +import gc import pickle from random import randrange, shuffle import struct import sys import unittest +import weakref from collections.abc import MutableMapping from test import mapping_tests, support @@ -585,6 +587,17 @@ class OrderedDictTests: dict.update(od, [('spam', 1)]) self.assertNotIn('NULL', repr(od)) + def test_reference_loop(self): + # Issue 25935 + OrderedDict = self.OrderedDict + class A: + od = OrderedDict() + A.od[A] = None + r = weakref.ref(A) + del A + gc.collect() + self.assertIsNone(r()) + class PurePythonOrderedDictTests(OrderedDictTests, unittest.TestCase): diff --git a/Misc/NEWS b/Misc/NEWS index faad3139ad2..f44b3dde4d7 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -46,6 +46,8 @@ Core and Builtins Library ------- +- Issue #25935: Garbage collector now breaks reference loops with OrderedDict. + - Issue #16620: Fixed AttributeError in msilib.Directory.glob(). - Issue #26013: Added compatibility with broken protocol 2 pickles created diff --git a/Objects/odictobject.c b/Objects/odictobject.c index c15b408dc24..1abdd02cdfa 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -772,19 +772,17 @@ _odict_clear_nodes(PyODictObject *od) { _ODictNode *node, *next; - if (!_odict_EMPTY(od)) { - node = _odict_FIRST(od); - while (node != NULL) { - next = _odictnode_NEXT(node); - _odictnode_DEALLOC(node); - node = next; - } - _odict_FIRST(od) = NULL; - _odict_LAST(od) = NULL; - } - _odict_free_fast_nodes(od); od->od_fast_nodes = NULL; + + node = _odict_FIRST(od); + _odict_FIRST(od) = NULL; + _odict_LAST(od) = NULL; + while (node != NULL) { + next = _odictnode_NEXT(node); + _odictnode_DEALLOC(node); + node = next; + } } /* There isn't any memory management of nodes past this point. */ @@ -1233,8 +1231,6 @@ odict_clear(register PyODictObject *od) { PyDict_Clear((PyObject *)od); _odict_clear_nodes(od); - _odict_FIRST(od) = NULL; - _odict_LAST(od) = NULL; if (_odict_resize(od) < 0) return NULL; Py_RETURN_NONE; @@ -1556,8 +1552,13 @@ PyDoc_STRVAR(odict_doc, static int 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)); + } return PyDict_Type.tp_traverse((PyObject *)od, visit, arg); }