From e90ec366fb1f29705f3f6ed970091c5b3fa131a8 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Mon, 27 Jun 2011 16:22:46 -0500 Subject: [PATCH] don't memoize objects that are their own copies (closes #12422) Patch mostly by Alex Gaynor. --- Lib/copy.py | 12 +++++++----- Lib/test/test_copy.py | 19 +++++++++++++++++-- Misc/NEWS | 3 +++ 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/Lib/copy.py b/Lib/copy.py index 089d101c7ca..497f21fbc79 100644 --- a/Lib/copy.py +++ b/Lib/copy.py @@ -173,8 +173,10 @@ def deepcopy(x, memo=None, _nil=[]): "un(deep)copyable object of type %s" % cls) y = _reconstruct(x, rv, 1, memo) - memo[d] = y - _keep_alive(x, memo) # Make sure x lives at least as long as d + # If is its own copy, don't memoize. + if y is not x: + memo[d] = y + _keep_alive(x, memo) # Make sure x lives at least as long as d return y _deepcopy_dispatch = d = {} @@ -214,9 +216,10 @@ def _deepcopy_tuple(x, memo): y = [] for a in x: y.append(deepcopy(a, memo)) - d = id(x) + # We're not going to put the tuple in the memo, but it's still important we + # check for it, in case the tuple contains recursive mutable structures. try: - return memo[d] + return memo[id(x)] except KeyError: pass for i in range(len(x)): @@ -225,7 +228,6 @@ def _deepcopy_tuple(x, memo): break else: y = x - memo[d] = y return y d[tuple] = _deepcopy_tuple diff --git a/Lib/test/test_copy.py b/Lib/test/test_copy.py index a84c109b87b..e450f70d07f 100644 --- a/Lib/test/test_copy.py +++ b/Lib/test/test_copy.py @@ -321,9 +321,24 @@ class TestCopy(unittest.TestCase): def test_deepcopy_keepalive(self): memo = {} - x = 42 + x = [] y = copy.deepcopy(x, memo) - self.assertTrue(memo[id(x)] is x) + self.assertIs(memo[id(memo)][0], x) + + def test_deepcopy_dont_memo_immutable(self): + memo = {} + x = [1, 2, 3, 4] + y = copy.deepcopy(x, memo) + self.assertEqual(y, x) + # There's the entry for the new list, and the keep alive. + self.assertEqual(len(memo), 2) + + memo = {} + x = [(1, 2)] + y = copy.deepcopy(x, memo) + self.assertEqual(y, x) + # Tuples with immutable contents are immutable for deepcopy. + self.assertEqual(len(memo), 2) def test_deepcopy_inst_vanilla(self): class C: diff --git a/Misc/NEWS b/Misc/NEWS index cbbc975f3ae..bcd597a79dc 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -200,6 +200,9 @@ Core and Builtins Library ------- +- Issue #12422: In the copy module, don't store objects that are their own copy + in the memo dict. + - Issue #12303: Add sigwaitinfo() and sigtimedwait() to the signal module. - Issue #12404: Remove C89 incompatible code from mmap module. Patch by Akira