[Python-checkins] cpython (3.5): Issue #28147: Fix a memory leak in split-table dictionaries

[inada.naoki]

https://hg.python.org/cpython/rev/cc40470c10f8
changeset:   105755:cc40470c10f8
branch:      3.5
parent:      105750:cf564121f9f0
user:        INADA Naoki <songofacandy at gmail.com>
date:        Tue Dec 20 09:54:24 2016 +0900
summary:
  Issue #28147: Fix a memory leak in split-table dictionaries

setattr() must not convert combined table into split table.

files:
  Lib/test/test_dict.py     |  18 ++++++++++++++++++
  Misc/NEWS                 |   3 +++
  Modules/_testcapimodule.c |  10 ++++++++++
  Objects/dictobject.c      |  21 +++++++++++++++------
  4 files changed, 46 insertions(+), 6 deletions(-)


diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py
--- 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
--- 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
--- 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
--- 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 {

-- 
Repository URL: https://hg.python.org/cpython