From 366061748989f2e3b079fe3f1608fc29af9f762c Mon Sep 17 00:00:00 2001 From: Sreehari Annam Date: Tue, 28 Jul 2026 09:33:58 -0400 Subject: [PATCH 1/2] 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. --- Lib/copy.py | 7 ++++--- Lib/test/test_copy.py | 16 ++++++++++++++++ ...026-07-28-10-00-00.gh-issue-154594.lRVvrN.rst | 3 +++ 3 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-07-28-10-00-00.gh-issue-154594.lRVvrN.rst diff --git a/Lib/copy.py b/Lib/copy.py index 6149301ad1389e2..b7fcccebd370ec7 100644 --- a/Lib/copy.py +++ b/Lib/copy.py @@ -122,9 +122,10 @@ def deepcopy(x, memo=None): if memo is None: memo = {} else: - y = memo.get(d, None) - if y is not None: - return y + try: + return memo[d] + except KeyError: + pass copier = _deepcopy_dispatch.get(cls) if copier is not None: diff --git a/Lib/test/test_copy.py b/Lib/test/test_copy.py index 98f56b5ae87f964..c9be475542ce70d 100644 --- a/Lib/test/test_copy.py +++ b/Lib/test/test_copy.py @@ -509,6 +509,22 @@ def __eq__(self, other): self.assertIsNot(y, x) self.assertIsNot(y.foo, x.foo) + def test_deepcopy_inst_deepcopy_returns_none(self): + # gh-154594: the memo used to use None as its own "not cached" + # sentinel, so a __deepcopy__ that legitimately returns None was + # indistinguishable from a memo miss and got invoked again on + # every subsequent reference to the same object. + calls = [] + class C: + def __deepcopy__(self, memo): + calls.append(1) + memo[id(self)] = None + return None + x = C() + y = copy.deepcopy([x, x, x]) + self.assertEqual(y, [None, None, None]) + self.assertEqual(len(calls), 1) + def test_deepcopy_inst_getinitargs(self): class C: def __init__(self, foo): diff --git a/Misc/NEWS.d/next/Library/2026-07-28-10-00-00.gh-issue-154594.lRVvrN.rst b/Misc/NEWS.d/next/Library/2026-07-28-10-00-00.gh-issue-154594.lRVvrN.rst new file mode 100644 index 000000000000000..9de7191f708edea --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-28-10-00-00.gh-issue-154594.lRVvrN.rst @@ -0,0 +1,3 @@ +Fix :func:`copy.deepcopy` re-invoking an object's :meth:`~object.__deepcopy__` +on every subsequent reference to that object when it returns :const:`None`. +The memo dict now distinguishes a cached ``None`` result from a cache miss. From 6a7529db6bf1a6cb55005354885080d8a1cfa1d1 Mon Sep 17 00:00:00 2001 From: Sreehari Annam Date: Tue, 4 Aug 2026 12:08:40 -0400 Subject: [PATCH 2/2] Switch deepcopy memo check to `d in memo` pochmann benchmarked this issue's candidate fixes on gh-154594 and found the try/except form is ~3.5x slower on a memo miss (the common case in real deepcopy workloads) despite being fastest on a hit. `d in memo` ties or beats the current `is not None` check on both hit and miss, and matches where the competing PR (gh-154595) landed after the same discussion. --- Lib/copy.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Lib/copy.py b/Lib/copy.py index b7fcccebd370ec7..e9874320490e50f 100644 --- a/Lib/copy.py +++ b/Lib/copy.py @@ -121,11 +121,8 @@ def deepcopy(x, memo=None): d = id(x) if memo is None: memo = {} - else: - try: - return memo[d] - except KeyError: - pass + elif d in memo: + return memo[d] copier = _deepcopy_dispatch.get(cls) if copier is not None: