Issue #28147: Fix a memory leak in split-table dictionaries
setattr() must not convert combined table into split table.
diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py
index 6d68e76..b85d333 100644
--- a/Lib/test/test_dict.py
+++ b/Lib/test/test_dict.py
@@ -836,6 +836,24 @@
pass
self._tracked(MyDict())
+ @support.cpython_only
+ def test_splittable_setattr_after_pop(self):
+ """setattr must not convert combined table into split table"""
+ # Issue 28147
+ import _testcapi
+
+ class C:
+ pass
+ a = C()
+ a.a = 2
+ self.assertTrue(_testcapi.dict_hassplittable(a.__dict__))
+ # dict.popitem() convert it to combined table
+ a.__dict__.popitem()
+ self.assertFalse(_testcapi.dict_hassplittable(a.__dict__))
+ # But C should not convert a.__dict__ to split table again.
+ a.a = 3
+ self.assertFalse(_testcapi.dict_hassplittable(a.__dict__))
+
def test_iterator_pickling(self):
for proto in range(pickle.HIGHEST_PROTOCOL + 1):
data = {1:"a", 2:"b", 3:"c"}
diff --git a/Misc/NEWS b/Misc/NEWS
index 33b777b..f09f0ac 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,9 @@
Core and Builtins
-----------------
+- Issue #28147: Fix a memory leak in split-table dictionaries: setattr()
+ must not convert combined table into split table.
+
- Issue #25677: Correct the positioning of the syntax error caret for
indented blocks. Based on patch by Michael Layzell.
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 29a28a7..060a92d 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -249,6 +249,15 @@
}
+static PyObject*
+dict_hassplittable(PyObject *self, PyObject *arg)
+{
+ if (!PyArg_Parse(arg, "O!:dict_hassplittable", &PyDict_Type, &arg)) {
+ return NULL;
+ }
+ return PyBool_FromLong(_PyDict_HasSplitTable((PyDictObject*)arg));
+}
+
/* Issue #4701: Check that PyObject_Hash implicitly calls
* PyType_Ready if it hasn't already been called
*/
@@ -3858,6 +3867,7 @@
{"test_datetime_capi", test_datetime_capi, METH_NOARGS},
{"test_list_api", (PyCFunction)test_list_api, METH_NOARGS},
{"test_dict_iteration", (PyCFunction)test_dict_iteration,METH_NOARGS},
+ {"dict_hassplittable", dict_hassplittable, METH_O},
{"test_lazy_hash_inheritance", (PyCFunction)test_lazy_hash_inheritance,METH_NOARGS},
{"test_long_api", (PyCFunction)test_long_api, METH_NOARGS},
{"test_xincref_doesnt_leak",(PyCFunction)test_xincref_doesnt_leak, METH_NOARGS},
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index e79ab36..795c1f6 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -985,8 +985,10 @@
return NULL;
}
else if (mp->ma_keys->dk_lookup == lookdict_unicode) {
- /* Remove dummy keys */
- if (dictresize(mp, DK_SIZE(mp->ma_keys)))
+ /* Remove dummy keys
+ * -1 is required since dictresize() uses key size > minused
+ */
+ if (dictresize(mp, DK_SIZE(mp->ma_keys) - 1))
return NULL;
}
assert(mp->ma_keys->dk_lookup == lookdict_unicode_nodummy);
@@ -2473,7 +2475,8 @@
}
/* Convert split table to combined table */
if (mp->ma_keys->dk_lookup == lookdict_split) {
- if (dictresize(mp, DK_SIZE(mp->ma_keys))) {
+ /* -1 is required since dictresize() uses key size > minused */
+ if (dictresize(mp, DK_SIZE(mp->ma_keys) - 1)) {
Py_DECREF(res);
return NULL;
}
@@ -3848,10 +3851,16 @@
CACHED_KEYS(tp) = NULL;
DK_DECREF(cached);
}
- } else {
+ }
+ else {
+ int was_shared = cached == ((PyDictObject *)dict)->ma_keys;
res = PyDict_SetItem(dict, key, value);
- if (cached != ((PyDictObject *)dict)->ma_keys) {
- /* Either update tp->ht_cached_keys or delete it */
+ /* PyDict_SetItem() may call dictresize() and convert split table
+ * into combined table. In such case, convert it to split
+ * table again and update type's shared key only when this is
+ * the only dict sharing key with the type.
+ */
+ if (was_shared && cached != ((PyDictObject *)dict)->ma_keys) {
if (cached->dk_refcnt == 1) {
CACHED_KEYS(tp) = make_keys_shared(dict);
} else {