From 932dc4d3bbc1e67e414396e0d93b2f5d2c606a68 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Thu, 5 Jul 2018 15:51:34 -0400 Subject: [PATCH 1/3] bpo-34042: Fix dict.copy() to maintain correct total refcount --- Lib/test/test_dict.py | 16 ++++++++++++++++ .../2018-07-05-15-51-29.bpo-34042.Gr9XUH.rst | 2 ++ Objects/dictobject.c | 7 +++++++ 3 files changed, 25 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-07-05-15-51-29.bpo-34042.Gr9XUH.rst diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index aa149d31eb0e22..774ba6ecb93d39 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -299,6 +299,22 @@ def test_copy_fuzz(self): self.assertNotEqual(d, d2) self.assertEqual(len(d2), len(d) + 1) + def test_copy_global_refcount(self): + # See issue #34042 for more details. + first_ref_count = second_ref_count = None + gettotalrefcount = sys.gettotalrefcount + dct = {'a': 1} + + for i in range(10): + dct.copy() + first_ref_count = gettotalrefcount() + + for i in range(10): + dct.copy() + second_ref_count = gettotalrefcount() + + self.assertEqual(first_ref_count, second_ref_count) + def test_copy_maintains_tracking(self): class A: pass diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-07-05-15-51-29.bpo-34042.Gr9XUH.rst b/Misc/NEWS.d/next/Core and Builtins/2018-07-05-15-51-29.bpo-34042.Gr9XUH.rst new file mode 100644 index 00000000000000..fd1730d4308b69 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-07-05-15-51-29.bpo-34042.Gr9XUH.rst @@ -0,0 +1,2 @@ +Fix dict.copy() to maintain correct total refcount (as reported by +sys.gettotalrefcount()). diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 40d7d8af6ec224..413557d6674127 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -656,6 +656,13 @@ clone_combined_dict(PyDictObject *orig) /* Maintain tracking. */ _PyObject_GC_TRACK(new); } + + /* Since we copied the keys table we now have an extra reference + in the system. Manually call _Py_INC_REFTOTAL to signal that + we have it now; calling DK_INCREF would be an error as + keys->dk_refcnt is already set to 1 (after memcpy). */ + _Py_INC_REFTOTAL; + return (PyObject *)new; } From 1c6fdb60ee28cf8e7e1d73074cd61bb03d468f4d Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Thu, 5 Jul 2018 16:22:22 -0400 Subject: [PATCH 2/3] tests requires a debug build --- Lib/test/test_dict.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 774ba6ecb93d39..8fd4a4189e5bf5 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -299,6 +299,8 @@ def test_copy_fuzz(self): self.assertNotEqual(d, d2) self.assertEqual(len(d2), len(d) + 1) + @unittest.skipUnless(hasattr(sys, 'gettotalrefcount'), + 'debug build required') def test_copy_global_refcount(self): # See issue #34042 for more details. first_ref_count = second_ref_count = None From efb7449e7018ebbd97edb043c4423768872d52aa Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Fri, 6 Jul 2018 12:04:36 -0400 Subject: [PATCH 3/3] Drop test --- Lib/test/test_dict.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py index 8fd4a4189e5bf5..aa149d31eb0e22 100644 --- a/Lib/test/test_dict.py +++ b/Lib/test/test_dict.py @@ -299,24 +299,6 @@ def test_copy_fuzz(self): self.assertNotEqual(d, d2) self.assertEqual(len(d2), len(d) + 1) - @unittest.skipUnless(hasattr(sys, 'gettotalrefcount'), - 'debug build required') - def test_copy_global_refcount(self): - # See issue #34042 for more details. - first_ref_count = second_ref_count = None - gettotalrefcount = sys.gettotalrefcount - dct = {'a': 1} - - for i in range(10): - dct.copy() - first_ref_count = gettotalrefcount() - - for i in range(10): - dct.copy() - second_ref_count = gettotalrefcount() - - self.assertEqual(first_ref_count, second_ref_count) - def test_copy_maintains_tracking(self): class A: pass