Fix Memory Leak Caused By Wrong Limited API Default

When a type has nullptr as tp_dealloc, there apply different defaults.

Static types had object_dealloc as default, while new heaptypes
created with type_new have subtype_dealloc as default.

A problem was now that PyType_FromSpec also has
subtype_dealloc as default. But that is wrong, because a type that
was written with the static type approach is already written with
object_dealloc in mind and takes somehow care about further issues
with that type.

When we now convert this type and suddenly use subtype_dealloc
instead of object_dealloc, things get pretty wrong.

Finding that out was pretty hard and took quite long to understand.

The fix was then very easy and is the best proof:
Replacing our former (wrong) solution of supplying an
SbkDummyDealloc with a function object_dealloc works perfectly,
and the leakage completely vanished.

The documentation now is also corrected.

Task-number: PYSIDE-832
Change-Id: Ifc20c28172eb5663cd5e60dac52e0a43acfb626c
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>

13 files changed, 31 insertions, 26 deletions

diff --git a/sources/pyside2/PySide2/QtQml/pysideqmlregistertype.cpp b/sources/pyside2/PySide2/QtQml/pysideqmlregistertype.cpp
index 0b427c251..1a1f09511 100644
--- a/sources/pyside2/PySide2/QtQml/pysideqmlregistertype.cpp
+++ b/sources/pyside2/PySide2/QtQml/pysideqmlregistertype.cpp
@@ -235,7 +235,7 @@ void propListTpFree(void* self)
 static PyType_Slot PropertyListType_slots[] = {
     {Py_tp_init, (void *)propListTpInit},
     {Py_tp_free, (void *)propListTpFree},
-    {Py_tp_dealloc, (void *)SbkDummyDealloc},
+    {Py_tp_dealloc, (void *)object_dealloc},
     {0, 0}
 };
 static PyType_Spec PropertyListType_spec = {
@@ -449,7 +449,7 @@ static PyType_Slot QtQml_VolatileBoolType_slots[] = {
     {Py_tp_str, (void *)reinterpret_cast<reprfunc>(QtQml_VolatileBoolObject_str)},
     {Py_tp_methods, (void *)QtQml_VolatileBoolObject_methods},
     {Py_tp_new, (void *)QtQml_VolatileBoolObject_new},
-    {Py_tp_dealloc, (void *)SbkDummyDealloc},
+    {Py_tp_dealloc, (void *)object_dealloc},
     {0, 0}
 };
 static PyType_Spec QtQml_VolatileBoolType_spec = {
diff --git a/sources/pyside2/libpyside/pysideclassinfo.cpp b/sources/pyside2/libpyside/pysideclassinfo.cpp
index 24a0c642b..5593825c3 100644
--- a/sources/pyside2/libpyside/pysideclassinfo.cpp
+++ b/sources/pyside2/libpyside/pysideclassinfo.cpp
@@ -60,7 +60,7 @@ static PyType_Slot PySideClassInfoType_slots[] = {
     {Py_tp_init, (void *)classInfoTpInit},
     {Py_tp_new, (void *)classInfoTpNew},
     {Py_tp_free, (void *)classInfoFree},
-    {Py_tp_dealloc, (void *)SbkDummyDealloc},
+    {Py_tp_dealloc, (void *)object_dealloc},
     {0, 0}
 };
 static PyType_Spec PySideClassInfoType_spec = {
diff --git a/sources/pyside2/libpyside/pysidemetafunction.cpp b/sources/pyside2/libpyside/pysidemetafunction.cpp
index 9839a1098..a9fbbc7fc 100644
--- a/sources/pyside2/libpyside/pysidemetafunction.cpp
+++ b/sources/pyside2/libpyside/pysidemetafunction.cpp
@@ -62,7 +62,7 @@ static PyType_Slot PySideMetaFunctionType_slots[] = {
     {Py_tp_call, (void *)functionCall},
     {Py_tp_new, (void *)PyType_GenericNew},
     {Py_tp_free, (void *)functionFree},
-    {Py_tp_dealloc, (void *)SbkDummyDealloc},
+    {Py_tp_dealloc, (void *)object_dealloc},
     {0, 0}
 };
 static PyType_Spec PySideMetaFunctionType_spec = {
diff --git a/sources/pyside2/libpyside/pysideqflags.cpp b/sources/pyside2/libpyside/pysideqflags.cpp
index 7a8fa2a05..684628e57 100644
--- a/sources/pyside2/libpyside/pysideqflags.cpp
+++ b/sources/pyside2/libpyside/pysideqflags.cpp
@@ -151,7 +151,7 @@ namespace QFlags
 #endif
         {Py_tp_new, (void *)PySideQFlagsNew},
         {Py_tp_richcompare, (void *)PySideQFlagsRichCompare},
-        {Py_tp_dealloc, (void *)SbkDummyDealloc},
+        {Py_tp_dealloc, (void *)object_dealloc},
         {0, 0}
     };
     static PyType_Spec SbkNewQFlagsType_spec = {
diff --git a/sources/pyside2/libpyside/pysidesignal.cpp b/sources/pyside2/libpyside/pysidesignal.cpp
index 2d423a634..c3dc65968 100644
--- a/sources/pyside2/libpyside/pysidesignal.cpp
+++ b/sources/pyside2/libpyside/pysidesignal.cpp
@@ -103,7 +103,7 @@ static PyType_Slot PySideSignalMetaType_slots[] = {
     {Py_tp_methods, (void *)Signal_methods},
     {Py_tp_base, (void *)&PyType_Type},
     {Py_tp_free, (void *)PyObject_GC_Del},
-    {Py_tp_dealloc, (void *)SbkDummyDealloc},
+    {Py_tp_dealloc, (void *)object_dealloc},
     {0, 0}
 };
 static PyType_Spec PySideSignalMetaType_spec = {
@@ -135,7 +135,7 @@ static PyType_Slot PySideSignalType_slots[] = {
     {Py_tp_init, (void *)signalTpInit},
     {Py_tp_new, (void *)PyType_GenericNew},
     {Py_tp_free, (void *)signalFree},
-    {Py_tp_dealloc, (void *)SbkDummyDealloc},
+    {Py_tp_dealloc, (void *)object_dealloc},
     {0, 0}
 };
 static PyType_Spec PySideSignalType_spec = {
@@ -174,7 +174,7 @@ static PyType_Slot PySideSignalInstanceType_slots[] = {
     {Py_tp_methods, (void *)SignalInstance_methods},
     {Py_tp_new, (void *)PyType_GenericNew},
     {Py_tp_free, (void *)signalInstanceFree},
-    {Py_tp_dealloc, (void *)SbkDummyDealloc},
+    {Py_tp_dealloc, (void *)object_dealloc},
     {0, 0}
 };
 static PyType_Spec PySideSignalInstanceType_spec = {
diff --git a/sources/pyside2/libpyside/pysideslot.cpp b/sources/pyside2/libpyside/pysideslot.cpp
index fddff54fa..6ae664c42 100644
--- a/sources/pyside2/libpyside/pysideslot.cpp
+++ b/sources/pyside2/libpyside/pysideslot.cpp
@@ -66,7 +66,7 @@ static PyType_Slot PySideSlotType_slots[] = {
     {Py_tp_call, (void *)slotCall},
     {Py_tp_init, (void *)slotTpInit},
     {Py_tp_new, (void *)PyType_GenericNew},
-    {Py_tp_dealloc, (void *)SbkDummyDealloc},
+    {Py_tp_dealloc, (void *)object_dealloc},
     {0, 0}
 };
 static PyType_Spec PySideSlotType_spec = {
diff --git a/sources/pyside2/libpyside/pysideweakref.cpp b/sources/pyside2/libpyside/pysideweakref.cpp
index 906aafd7c..6c38d39c4 100644
--- a/sources/pyside2/libpyside/pysideweakref.cpp
+++ b/sources/pyside2/libpyside/pysideweakref.cpp
@@ -53,7 +53,7 @@ static PyObject* CallableObject_call(PyObject* callable_object, PyObject* args,
 
 static PyType_Slot PySideCallableObjectType_slots[] = {
     {Py_tp_call, (void *)CallableObject_call},
-    {Py_tp_dealloc, (void *)SbkDummyDealloc},
+    {Py_tp_dealloc, (void *)object_dealloc},
     {0, 0}
 };
 static PyType_Spec PySideCallableObjectType_spec = {
diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
index b0d778231..9e1d6926e 100644
--- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
@@ -3704,7 +3704,7 @@ void CppGenerator::writeClassDefinition(QTextStream &s,
     if (metaClass->isNamespace() || metaClass->hasPrivateDestructor()) {
         tp_dealloc = metaClass->hasPrivateDestructor() ?
                      QLatin1String("SbkDeallocWrapperWithPrivateDtor") :
-                     QLatin1String("SbkDummyDealloc /* PYSIDE-595: Prevent replacement of \"0\" with subtype_dealloc. */");
+                     QLatin1String("object_dealloc /* PYSIDE-832: Prevent replacement of \"0\" with subtype_dealloc. */");
         tp_init = QLatin1String("0");
     } else {
         QString deallocClassName;
diff --git a/sources/shiboken2/libshiboken/basewrapper.cpp b/sources/shiboken2/libshiboken/basewrapper.cpp
index 0aba26679..5ca4172c8 100644
--- a/sources/shiboken2/libshiboken/basewrapper.cpp
+++ b/sources/shiboken2/libshiboken/basewrapper.cpp
@@ -416,8 +416,10 @@ PyObject* SbkQAppTpNew(PyTypeObject* subtype, PyObject *, PyObject *)
 }
 
 void
-SbkDummyDealloc(PyObject *)
-{}
+object_dealloc(PyObject *self)
+{
+    Py_TYPE(self)->tp_free(self);
+}
 
 PyObject *
 SbkDummyNew(PyTypeObject *type, PyObject*, PyObject*)
diff --git a/sources/shiboken2/libshiboken/basewrapper.h b/sources/shiboken2/libshiboken/basewrapper.h
index 06b17a151..134a3bc51 100644
--- a/sources/shiboken2/libshiboken/basewrapper.h
+++ b/sources/shiboken2/libshiboken/basewrapper.h
@@ -109,17 +109,15 @@ LIBSHIBOKEN_API PyObject* SbkObjectTpNew(PyTypeObject* subtype, PyObject*, PyObj
 LIBSHIBOKEN_API PyObject* SbkQAppTpNew(PyTypeObject *subtype, PyObject *args, PyObject *kwds);
 
 /**
- *  PYSIDE-595: Use a null deallocator instead of nullptr.
+ *  PYSIDE-832: Use object_dealloc instead of nullptr.
  *
  *  When moving to heaptypes, we were struck by a special default behavior of
  *  PyType_FromSpecWithBases that inserts subtype_dealloc when tp_dealloc is
- *  nullptr. To prevent inserting this, we use a null deallocator that is there
- *  as a placeholder.
- *
- *  The same holds for a null tp_new. We use one that raises the right error.
+ *  nullptr. But the default before conversion to heaptypes was to assign
+ *  object_dealloc. This seems to be a bug in the Limited API.
  */
-LIBSHIBOKEN_API void SbkDummyDealloc(PyObject*);
-LIBSHIBOKEN_API PyObject *SbkDummyNew(PyTypeObject *type, PyObject*, PyObject*);
+LIBSHIBOKEN_API void object_dealloc(PyObject *);
+LIBSHIBOKEN_API PyObject *SbkDummyNew(PyTypeObject *type, PyObject *, PyObject *);
 
 } // extern "C"
 
diff --git a/sources/shiboken2/libshiboken/pep384impl_doc.rst b/sources/shiboken2/libshiboken/pep384impl_doc.rst
index 9974f737b..2844249ad 100644
--- a/sources/shiboken2/libshiboken/pep384impl_doc.rst
+++ b/sources/shiboken2/libshiboken/pep384impl_doc.rst
@@ -426,11 +426,16 @@ many headaches::
         type->tp_dealloc = subtype_dealloc;
     }
 
-So, if you think you have no ``tp_dealloc`` field set, you will unwantedly
-get ``subtype_dealloc``, which in the case of PySide always was wrong!
+In fact, before the move to the new API, the ``PyType_Ready`` function
+filled empty ``tp_dealloc`` fields with ``object_dealloc``. And the code
+that has been written with that in mind now becomes pretty wrong if suddenly
+``subtype_dealloc`` is used.
+
+The way out was to explicitly provide an ``object_dealloc`` function.
+This would then again impose a problem, because ``object_dealloc`` is not
+public. Writing our own version is easy, but it again needs access to
+type objects. But fortunately, we have broken this rule, already...
 
-The way out was to use a dummy function that has no effect other than
-being something not NULL.
 
 * The new types are only partially allocated
 
diff --git a/sources/shiboken2/libshiboken/sbkenum.cpp b/sources/shiboken2/libshiboken/sbkenum.cpp
index bd007f079..d129e6380 100644
--- a/sources/shiboken2/libshiboken/sbkenum.cpp
+++ b/sources/shiboken2/libshiboken/sbkenum.cpp
@@ -507,7 +507,7 @@ static PyType_Slot SbkNewType_slots[] = {
     {Py_nb_index, (void *)enum_int},
     {Py_tp_richcompare, (void *)enum_richcompare},
     {Py_tp_hash, (void *)enum_hash},
-    {Py_tp_dealloc, (void *)SbkDummyDealloc},
+    {Py_tp_dealloc, (void *)object_dealloc},
     {0, 0}
 };
 static PyType_Spec SbkNewType_spec = {
diff --git a/sources/shiboken2/libshiboken/voidptr.cpp b/sources/shiboken2/libshiboken/voidptr.cpp
index 3a0dbb434..0d7b6b9cd 100644
--- a/sources/shiboken2/libshiboken/voidptr.cpp
+++ b/sources/shiboken2/libshiboken/voidptr.cpp
@@ -224,7 +224,7 @@ static PyType_Slot SbkVoidPtrType_slots[] = {
     {Py_tp_richcompare, (void *)SbkVoidPtrObject_richcmp},
     {Py_tp_init, (void *)SbkVoidPtrObject_init},
     {Py_tp_new, (void *)SbkVoidPtrObject_new},
-    {Py_tp_dealloc, (void *)SbkDummyDealloc},
+    {Py_tp_dealloc, (void *)object_dealloc},
     {0, 0}
 };
 static PyType_Spec SbkVoidPtrType_spec = {