Issue python#135: fix memory leaks of types with Stackless extensions.

Anselm Kruis · Anselm Kruis · commit e217840072f9 · 2017-10-09T20:56:38.000+02:00
Fix several problems related with slp_prepare_slots():
- set Py_TPFLAGS_HAVE_STACKLESS_EXTENSION in slp_prepare_slots().
- for heap types free the allocated buffer in tp_dealloc
- Allocate the memory from the object domain, because the blocks are
  small (about 74 bytes).
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
@@ -3085,6 +3085,23 @@ type_dealloc(PyTypeObject *type)
     Py_XDECREF(et->ht_slots);
     if (et->ht_cached_keys)
         _PyDictKeys_DecRef(et->ht_cached_keys);
+
+#ifdef STACKLESS
+    /* A type's tp_as_mapping is heap allocated, if
+     * the flag Py_TPFLAGS_HAVE_STACKLESS_EXTENSION is set.
+     * The allocation happens in stacklessmodule.c
+     * function slp_prepare_slots().
+     */
+    if (type->tp_as_mapping &&
+            (type->tp_flags & Py_TPFLAGS_HAVE_STACKLESS_EXTENSION)) {
+        void *tp_as_mapping = type->tp_as_mapping;
+        assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE);
+        type->tp_flags &= ~Py_TPFLAGS_HAVE_STACKLESS_EXTENSION;
+        type->tp_as_mapping = NULL;
+        PyObject_Free(tp_as_mapping);
+    }
+#endif
+
     Py_TYPE(type)->tp_free((PyObject *)type);
 }
 
diff --git a/Stackless/module/stacklessmodule.c b/Stackless/module/stacklessmodule.c
@@ -1649,29 +1649,45 @@ Please refer to their documentation.\n\
 
 /* This function is called to make sure the type has
  * space in its tp_as_mapping to hold the slpslots.  If it has
- * the Py_TMPFLAGS_HAVE_STACKLESS_EXTENSION bit, then any struct present
+ * the Py_TPFLAGS_HAVE_STACKLESS_EXTENSION bit, then any struct present
  * is deemed large enough.  But if it is missing, then a struct already
  * present needs to be grown.
+ *
+ * This function is the only place where we set Py_TPFLAGS_HAVE_STACKLESS_EXTENSION
+ * for heap types (== type with Py_TPFLAGS_HEAPTYPE). This way we can free
+ * the memory later in type_dealloc() in typeobject.c.
  */
 int
 slp_prepare_slots(PyTypeObject * type)
 {
     PyMappingMethods *map, *old = type->tp_as_mapping;
     if (old == NULL || !(type->tp_flags & Py_TPFLAGS_HAVE_STACKLESS_EXTENSION)) {
         /* need to allocate a new struct */
-        map = (PyMappingMethods *)PyMem_MALLOC(sizeof(PyMappingMethods));
+
+        /* For heap types ensure, that 'old' is part of the
+         * PyHeapTypeObject. Otherwise we would leak 'old'.
+         */
+        assert(!(type->tp_flags & Py_TPFLAGS_HEAPTYPE) ||
+                (old == (PyMappingMethods*)
+                        &((PyHeapTypeObject*)type)->as_mapping));
+
+        map = (PyMappingMethods *)PyObject_Calloc(1, sizeof(PyMappingMethods));
         if (map == NULL) {
             PyErr_NoMemory();
             return -1;
-            }
-        memset(map, 0, sizeof(PyMappingMethods));
+        }
         if (old) {
             /* copy the old method slots, and mark the type */
             memcpy(map, old, offsetof(PyMappingMethods, slpflags));
-            type->tp_flags |= Py_TPFLAGS_HAVE_STACKLESS_EXTENSION;
         }
-        /* we are leaking this pointer */
+        /* We are leaking 'map' and 'old', except for heap types
+         * (with Py_TPFLAGS_HEAPTYPE set)). Theses types have a
+         * tp_dealloc function, which frees 'map'.
+         * For heap types 'old' is a member of PyHeapTypeObject and
+         * must not be freed.
+         */
         type->tp_as_mapping = map;
+        type->tp_flags |= Py_TPFLAGS_HAVE_STACKLESS_EXTENSION;
     }
     return 0;
 }
@@ -1692,7 +1708,6 @@ static int init_stackless_methods(void)
         if (ind)
             t = *((PyTypeObject **)t);
         /* this type has stackless slot methods.  Set the flag */
-        t->tp_flags |= Py_TPFLAGS_HAVE_STACKLESS_EXTENSION;
         if (slp_prepare_slots(t))
             return -1;
         ((signed char *) t->tp_as_mapping)[ofs] = -1;
