Issue #22653: Fix an assertion failure in debug mode when doing a reentrant dict insertion in debug mode.

pitrou · pitrou · commit 9a2fac1c2970 · 2014-10-18T00:35:00.000+02:00
diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py
@@ -906,6 +906,35 @@ class Foo: pass
         f.a = 'a'
         self.assertEqual(f.__dict__, {1:1, 'a':'a'})
 
+    def check_reentrant_insertion(self, mutate):
+        # This object will trigger mutation of the dict when replaced
+        # by another value.  Note this relies on refcounting: the test
+        # won't achieve its purpose on fully-GCed Python implementations.
+        class Mutating:
+            def __del__(self):
+                mutate(d)
+
+        d = {k: Mutating() for k in 'abcdefghijklmnopqr'}
+        for k in list(d):
+            d[k] = k
+
+    def test_reentrant_insertion(self):
+        # Reentrant insertion shouldn't crash (see issue #22653)
+        def mutate(d):
+            d['b'] = 5
+        self.check_reentrant_insertion(mutate)
+
+        def mutate(d):
+            d.update(self.__dict__)
+            d.clear()
+        self.check_reentrant_insertion(mutate)
+
+        def mutate(d):
+            while d:
+                d.popitem()
+        self.check_reentrant_insertion(mutate)
+
+
 from test import mapping_tests
 
 class GeneralMappingTests(mapping_tests.BasicTestMappingProtocol):
diff --git a/Misc/NEWS b/Misc/NEWS
@@ -11,6 +11,9 @@ Release date: TBA
 Core and Builtins
 -----------------
 
+- Issue #22653: Fix an assertion failure in debug mode when doing a reentrant
+  dict insertion in debug mode.
+
 - Issue #22643: Fix integer overflow in Unicode case operations (upper, lower,
   title, swapcase, casefold).
 
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
@@ -814,13 +814,14 @@ insertdict(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject *value)
     if (ep == NULL) {
         return -1;
     }
+    assert(PyUnicode_CheckExact(key) || mp->ma_keys->dk_lookup == lookdict);
     Py_INCREF(value);
     MAINTAIN_TRACKING(mp, key, value);
     old_value = *value_addr;
     if (old_value != NULL) {
         assert(ep->me_key != NULL && ep->me_key != dummy);
         *value_addr = value;
-        Py_DECREF(old_value); /* which **CAN** re-enter */
+        Py_DECREF(old_value); /* which **CAN** re-enter (see issue #22653) */
     }
     else {
         if (ep->me_key == NULL) {
@@ -851,9 +852,8 @@ insertdict(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject *value)
         }
         mp->ma_used++;
         *value_addr = value;
+        assert(ep->me_key != NULL && ep->me_key != dummy);
     }
-    assert(ep->me_key != NULL && ep->me_key != dummy);
-    assert(PyUnicode_CheckExact(key) || mp->ma_keys->dk_lookup == lookdict);
     return 0;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -814,13 +814,14 @@ insertdict(PyDictObject mp, PyObject key, Py_hash_t hash, PyObject *value)`
`814`	`814`	`if (ep == NULL) {`
`815`	`815`	`return -1;`
`816`	`816`	`}`
	`817`	`+ assert(PyUnicode_CheckExact(key) \|\| mp->ma_keys->dk_lookup == lookdict);`
`817`	`818`	`Py_INCREF(value);`
`818`	`819`	`MAINTAIN_TRACKING(mp, key, value);`
`819`	`820`	`old_value = *value_addr;`
`820`	`821`	`if (old_value != NULL) {`
`821`	`822`	`assert(ep->me_key != NULL && ep->me_key != dummy);`
`822`	`823`	`*value_addr = value;`
`823`		`- Py_DECREF(old_value); /* which CAN re-enter */`
	`824`	`+ Py_DECREF(old_value); /* which CAN re-enter (see issue #22653) */`
`824`	`825`	`}`
`825`	`826`	`else {`
`826`	`827`	`if (ep->me_key == NULL) {`
`@@ -851,9 +852,8 @@ insertdict(PyDictObject mp, PyObject key, Py_hash_t hash, PyObject *value)`
`851`	`852`	`}`
`852`	`853`	`mp->ma_used++;`
`853`	`854`	`*value_addr = value;`
	`855`	`+ assert(ep->me_key != NULL && ep->me_key != dummy);`
`854`	`856`	`}`
`855`		`- assert(ep->me_key != NULL && ep->me_key != dummy);`
`856`		`- assert(PyUnicode_CheckExact(key) \|\| mp->ma_keys->dk_lookup == lookdict);`
`857`	`857`	`return 0;`
`858`	`858`	`}`
`859`	`859`