Skip to content

Commit 3660617

Browse files
committed
gh-154594: Fix copy.deepcopy() re-copying objects whose __deepcopy__ returns None
deepcopy() used None both as the memo dict's "not cached" sentinel and as a value callers can legitimately store there, so a __deepcopy__ that returns None was indistinguishable from a cache miss and got invoked again on every subsequent reference to the same object. Use a direct memo[d] lookup (matching the existing pattern in _deepcopy_tuple/_deepcopy_frozendict) instead of memo.get(d, None), so every return value -- including None -- is memoized correctly. This also avoids reintroducing the non-immortal-sentinel refcount contention under free-threading that gh-132657/gh-138429 fixed, since no sentinel object is used at all.
1 parent 20b50f8 commit 3660617

3 files changed

Lines changed: 23 additions & 3 deletions

File tree

Lib/copy.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,10 @@ def deepcopy(x, memo=None):
122122
if memo is None:
123123
memo = {}
124124
else:
125-
y = memo.get(d, None)
126-
if y is not None:
127-
return y
125+
try:
126+
return memo[d]
127+
except KeyError:
128+
pass
128129

129130
copier = _deepcopy_dispatch.get(cls)
130131
if copier is not None:

Lib/test/test_copy.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,22 @@ def __eq__(self, other):
509509
self.assertIsNot(y, x)
510510
self.assertIsNot(y.foo, x.foo)
511511

512+
def test_deepcopy_inst_deepcopy_returns_none(self):
513+
# gh-154594: the memo used to use None as its own "not cached"
514+
# sentinel, so a __deepcopy__ that legitimately returns None was
515+
# indistinguishable from a memo miss and got invoked again on
516+
# every subsequent reference to the same object.
517+
calls = []
518+
class C:
519+
def __deepcopy__(self, memo):
520+
calls.append(1)
521+
memo[id(self)] = None
522+
return None
523+
x = C()
524+
y = copy.deepcopy([x, x, x])
525+
self.assertEqual(y, [None, None, None])
526+
self.assertEqual(len(calls), 1)
527+
512528
def test_deepcopy_inst_getinitargs(self):
513529
class C:
514530
def __init__(self, foo):
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix :func:`copy.deepcopy` re-invoking an object's :meth:`~object.__deepcopy__`
2+
on every subsequent reference to that object when it returns :const:`None`.
3+
The memo dict now distinguishes a cached ``None`` result from a cache miss.

0 commit comments

Comments
 (0)