Fix Python 3.8 problems

This patch fixes some refcounting problems with Python 3.8 .
One incompatible change was announced in the what's new
document, but actually there were two more problems which
were not explicitly mentioned but took much time to sort out.

The patch is compatible with the limited API changes
(tested with debug build and API error disabled).
It is also independent of the Python version which is
full Limited API support.

For more info, see the documentation mentioned below.

The flag error is circumvented now! We either find a better
solution or leave it as it is. For now this is ok.

Fixes: PYSIDE-939
Change-Id: Iff4a9816857a6ebe86efd4b654d8921e4e464939
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>

10 files changed, 121 insertions, 16 deletions

diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
index 5460fd7c7..765657099 100644
--- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
@@ -3871,7 +3871,7 @@ void CppGenerator::writeClassDefinition(QTextStream &s,
     if (metaClass->isNamespace() || metaClass->hasPrivateDestructor()) {
         tp_dealloc = metaClass->hasPrivateDestructor() ?
                      QLatin1String("SbkDeallocWrapperWithPrivateDtor") :
-                     QLatin1String("object_dealloc /* PYSIDE-832: Prevent replacement of \"0\" with subtype_dealloc. */");
+                     QLatin1String("Sbk_object_dealloc /* PYSIDE-832: Prevent replacement of \"0\" with subtype_dealloc. */");
         tp_init.clear();
     } else {
         QString deallocClassName;
diff --git a/sources/shiboken2/libshiboken/basewrapper.cpp b/sources/shiboken2/libshiboken/basewrapper.cpp
index 000035627..4030fe1c4 100644
--- a/sources/shiboken2/libshiboken/basewrapper.cpp
+++ b/sources/shiboken2/libshiboken/basewrapper.cpp
@@ -58,6 +58,10 @@
 #include "qapp_macro.h"
 #include "voidptr.h"
 
+#if defined(__APPLE__)
+#include <dlfcn.h>
+#endif
+
 namespace {
     void _destroyParentInfo(SbkObject *obj, bool keepReference);
 }
@@ -74,6 +78,17 @@ static void callDestructor(const Shiboken::DtorAccumulatorVisitor::DestructorEnt
 extern "C"
 {
 
+// PYSIDE-939: A general replacement for object_dealloc.
+void Sbk_object_dealloc(PyObject *self)
+{
+    if (PepRuntime_38_flag) {
+        // PYSIDE-939: Handling references correctly.
+        // This was not needed before Python 3.8 (Python issue 35810)
+        Py_DECREF(Py_TYPE(self));
+    }
+    Py_TYPE(self)->tp_free(self);
+}
+
 static void SbkObjectTypeDealloc(PyObject *pyObj);
 static PyObject *SbkObjectTypeTpNew(PyTypeObject *metatype, PyObject *args, PyObject *kwds);
 
@@ -309,8 +324,32 @@ static void SbkDeallocWrapperCommon(PyObject *pyObj, bool canDelete)
     // Need to decref the type if this is the dealloc func; if type
     // is subclassed, that dealloc func will decref (see subtype_dealloc
     // in typeobject.c in the python sources)
-    bool needTypeDecref = (PyType_GetSlot(pyType, Py_tp_dealloc) == SbkDeallocWrapper
+    bool needTypeDecref = (false
+                           || PyType_GetSlot(pyType, Py_tp_dealloc) == SbkDeallocWrapper
                            || PyType_GetSlot(pyType, Py_tp_dealloc) == SbkDeallocWrapperWithPrivateDtor);
+    if (PepRuntime_38_flag) {
+        // PYSIDE-939: Additional rule: Also when a subtype is heap allocated,
+        // then the subtype_dealloc deref will be suppressed, and we need again
+        // to supply a decref.
+        needTypeDecref |= (pyType->tp_base->tp_flags & Py_TPFLAGS_HEAPTYPE) != 0;
+    }
+
+#if defined(__APPLE__)
+    // Just checking once that our assumptions are right.
+    if (false) {
+        void *p = PyType_GetSlot(pyType, Py_tp_dealloc);
+        Dl_info dl_info;
+        dladdr(p, &dl_info);
+        fprintf(stderr, "tp_dealloc is %s\n", dl_info.dli_sname);
+    }
+    // Gives one of our functions
+    //  "Sbk_object_dealloc"
+    //  "SbkDeallocWrapperWithPrivateDtor"
+    //  "SbkDeallocQAppWrapper"
+    //  "SbkDeallocWrapper"
+    // but for typedealloc_test.py we get
+    //  "subtype_dealloc"
+#endif
 
     // Ensure that the GC is no longer tracking this object to avoid a
     // possible reentrancy problem.  Since there are multiple steps involved
@@ -369,6 +408,11 @@ static void SbkDeallocWrapperCommon(PyObject *pyObj, bool canDelete)
 
     if (needTypeDecref)
         Py_DECREF(pyType);
+    if (PepRuntime_38_flag) {
+        // PYSIDE-939: Handling references correctly.
+        // This was not needed before Python 3.8 (Python issue 35810)
+        Py_DECREF(pyType);
+    }
 }
 
 void SbkDeallocWrapper(PyObject *pyObj)
@@ -412,6 +456,11 @@ void SbkObjectTypeDealloc(PyObject *pyObj)
 #ifndef Py_LIMITED_API
     Py_TRASHCAN_SAFE_END(pyObj);
 #endif
+    if (PepRuntime_38_flag) {
+        // PYSIDE-939: Handling references correctly.
+        // This was not needed before Python 3.8 (Python issue 35810)
+        Py_DECREF(Py_TYPE(pyObj));
+    }
 }
 
 PyObject *SbkObjectTypeTpNew(PyTypeObject *metatype, PyObject *args, PyObject *kwds)
@@ -453,7 +502,16 @@ PyObject *SbkObjectTypeTpNew(PyTypeObject *metatype, PyObject *args, PyObject *k
 
     // The meta type creates a new type when the Python programmer extends a wrapped C++ class.
     auto type_new = reinterpret_cast<newfunc>(PyType_Type.tp_new);
+
+    // PYSIDE-939: This is a temporary patch that circumvents the problem
+    // with Py_TPFLAGS_METHOD_DESCRIPTOR until this is finally solved.
+    PyObject *ob_PyType_Type = reinterpret_cast<PyObject *>(&PyType_Type);
+    PyObject *mro = PyObject_GetAttr(ob_PyType_Type, Shiboken::PyName::mro());
+    auto hold = Py_TYPE(mro)->tp_flags;
+    Py_TYPE(mro)->tp_flags &= ~Py_TPFLAGS_METHOD_DESCRIPTOR;
     auto *newType = reinterpret_cast<SbkObjectType *>(type_new(metatype, args, kwds));
+    Py_TYPE(mro)->tp_flags = hold;
+
     if (!newType)
         return nullptr;
 #if PY_VERSION_HEX < 0x03000000
@@ -554,12 +612,6 @@ PyObject *SbkQAppTpNew(PyTypeObject *subtype, PyObject *, PyObject *)
     return self == nullptr ? nullptr : _setupNew(self, subtype);
 }
 
-void
-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 7faf223bd..813870dac 100644
--- a/sources/shiboken2/libshiboken/basewrapper.h
+++ b/sources/shiboken2/libshiboken/basewrapper.h
@@ -64,6 +64,9 @@ struct LIBSHIBOKEN_API SbkObject
 };
 
 
+/// PYSIDE-939: A general replacement for object_dealloc.
+LIBSHIBOKEN_API void Sbk_object_dealloc(PyObject *self);
+
 /// Dealloc the python object \p pyObj and the C++ object represented by it.
 LIBSHIBOKEN_API void SbkDeallocWrapper(PyObject *pyObj);
 LIBSHIBOKEN_API void SbkDeallocQAppWrapper(PyObject *pyObj);
@@ -116,7 +119,7 @@ LIBSHIBOKEN_API PyObject *SbkQAppTpNew(PyTypeObject *subtype, PyObject *args, Py
  *  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 object_dealloc(PyObject *);
+/// PYSIDE-939: Replaced by Sbk_object_dealloc.
 LIBSHIBOKEN_API PyObject *SbkDummyNew(PyTypeObject *type, PyObject *, PyObject *);
 
 } // extern "C"
diff --git a/sources/shiboken2/libshiboken/pep384impl.cpp b/sources/shiboken2/libshiboken/pep384impl.cpp
index 5729100bf..41d56cba1 100644
--- a/sources/shiboken2/libshiboken/pep384impl.cpp
+++ b/sources/shiboken2/libshiboken/pep384impl.cpp
@@ -41,6 +41,8 @@
 #include "autodecref.h"
 #include "sbkstaticstrings.h"
 #include "sbkstaticstrings_p.h"
+#include <stdlib.h>
+
 
 extern "C"
 {
@@ -161,7 +163,8 @@ check_PyTypeObject_valid()
         || probe_tp_free            != check->tp_free
         || probe_tp_is_gc           != check->tp_is_gc
         || probe_tp_bases           != typetype->tp_bases
-        || probe_tp_mro             != typetype->tp_mro)
+        || probe_tp_mro             != typetype->tp_mro
+        || Py_TPFLAGS_DEFAULT       != (check->tp_flags & Py_TPFLAGS_DEFAULT))
         Py_FatalError("The structure of type objects has changed!");
     Py_DECREF(check);
     Py_DECREF(probe_tp_base);
@@ -669,6 +672,25 @@ _Pep_PrivateMangle(PyObject *self, PyObject *name)
 
 /*****************************************************************************
  *
+ * Runtime support for Python 3.8 incompatibilities
+ *
+ */
+
+int PepRuntime_38_flag = 0;
+
+static void
+init_PepRuntime()
+{
+    // We expect a string of the form "\d\.\d+\."
+    const char *version = Py_GetVersion();
+    if (version[0] < '3')
+        return;
+    if (std::atoi(version + 2) >= 8)
+        PepRuntime_38_flag = 1;
+}
+
+/*****************************************************************************
+ *
  * Module Initialization
  *
  */
@@ -677,6 +699,7 @@ void
 Pep384_Init()
 {
     check_PyTypeObject_valid();
+    init_PepRuntime();
 #ifdef Py_LIMITED_API
     Pep_GetVerboseFlag();
     PepMethod_TypePtr = getMethodType();
diff --git a/sources/shiboken2/libshiboken/pep384impl.h b/sources/shiboken2/libshiboken/pep384impl.h
index 1aa7e6fc0..e9f65e446 100644
--- a/sources/shiboken2/libshiboken/pep384impl.h
+++ b/sources/shiboken2/libshiboken/pep384impl.h
@@ -89,7 +89,7 @@ typedef struct _typeobject {
     Py_ssize_t tp_basicsize;
     void *X03; // Py_ssize_t tp_itemsize;
     void *X04; // destructor tp_dealloc;
-    void *X05; // printfunc tp_print;
+    void *X05; // Py_ssize_t tp_vectorcall_offset;
     void *X06; // getattrfunc tp_getattr;
     void *X07; // setattrfunc tp_setattr;
     void *X08; // PyAsyncMethods *tp_as_async;
@@ -103,7 +103,7 @@ typedef struct _typeobject {
     void *X16; // getattrofunc tp_getattro;
     void *X17; // setattrofunc tp_setattro;
     void *X18; // PyBufferProcs *tp_as_buffer;
-    void *X19; // unsigned long tp_flags;
+    unsigned long tp_flags;
     void *X20; // const char *tp_doc;
     traverseproc tp_traverse;
     inquiry tp_clear;
@@ -129,6 +129,13 @@ typedef struct _typeobject {
 
 } PyTypeObject;
 
+#ifndef PyObject_IS_GC
+/* Test if an object has a GC head */
+#define PyObject_IS_GC(o) \
+    (PyType_IS_GC(Py_TYPE(o)) \
+     && (Py_TYPE(o)->tp_is_gc == NULL || Py_TYPE(o)->tp_is_gc(o)))
+#endif
+
 // This was a macro error in the limited API from the beginning.
 // It was fixed in Python master, but did make it only in Python 3.8 .
 #define PY_ISSUE33738_SOLVED 0x03080000
@@ -292,7 +299,7 @@ LIBSHIBOKEN_API PyObject *PyRun_String(const char *, int, PyObject *, PyObject *
 // But this is no problem as we check it's validity for every version.
 
 #define PYTHON_BUFFER_VERSION_COMPATIBLE    (PY_VERSION_HEX >= 0x03030000 && \
-                                             PY_VERSION_HEX <  0x0307FFFF)
+                                             PY_VERSION_HEX <  0x0308FFFF)
 #if !PYTHON_BUFFER_VERSION_COMPATIBLE
 # error Please check the buffer compatibility for this python version!
 #endif
@@ -488,6 +495,19 @@ extern LIBSHIBOKEN_API PyTypeObject *PepMethodDescr_TypePtr;
 
 /*****************************************************************************
  *
+ * Runtime support for Python 3.8 incompatibilities
+ *
+ */
+
+#ifndef Py_TPFLAGS_METHOD_DESCRIPTOR
+/* Objects behave like an unbound method */
+#define Py_TPFLAGS_METHOD_DESCRIPTOR (1UL << 17)
+#endif
+
+extern LIBSHIBOKEN_API int PepRuntime_38_flag;
+
+/*****************************************************************************
+ *
  * Module Initialization
  *
  */
diff --git a/sources/shiboken2/libshiboken/sbkenum.cpp b/sources/shiboken2/libshiboken/sbkenum.cpp
index 71fcf5f64..db390802a 100644
--- a/sources/shiboken2/libshiboken/sbkenum.cpp
+++ b/sources/shiboken2/libshiboken/sbkenum.cpp
@@ -321,6 +321,11 @@ void SbkEnumTypeDealloc(PyObject *pyObj)
 #ifndef Py_LIMITED_API
     Py_TRASHCAN_SAFE_END(pyObj);
 #endif
+    if (PepRuntime_38_flag) {
+        // PYSIDE-939: Handling references correctly.
+        // This was not needed before Python 3.8 (Python issue 35810)
+        Py_DECREF(Py_TYPE(pyObj));
+    }
 }
 
 PyObject *SbkEnumTypeTpNew(PyTypeObject *metatype, PyObject *args, PyObject *kwds)
@@ -512,7 +517,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 *)object_dealloc},
+    {Py_tp_dealloc, (void *)Sbk_object_dealloc},
     {0, nullptr}
 };
 static PyType_Spec SbkNewType_spec = {
diff --git a/sources/shiboken2/libshiboken/sbkstaticstrings.cpp b/sources/shiboken2/libshiboken/sbkstaticstrings.cpp
index 3727eb494..42d20d133 100644
--- a/sources/shiboken2/libshiboken/sbkstaticstrings.cpp
+++ b/sources/shiboken2/libshiboken/sbkstaticstrings.cpp
@@ -61,6 +61,7 @@ STATIC_STRING_IMPL(compile, "compile");
 STATIC_STRING_IMPL(function, "function")
 STATIC_STRING_IMPL(marshal, "marshal")
 STATIC_STRING_IMPL(method, "method")
+STATIC_STRING_IMPL(mro, "mro")
 STATIC_STRING_IMPL(overload, "overload")
 STATIC_STRING_IMPL(staticmethod, "staticmethod")
 } // namespace PyName
diff --git a/sources/shiboken2/libshiboken/sbkstaticstrings_p.h b/sources/shiboken2/libshiboken/sbkstaticstrings_p.h
index 42c5585fa..12c11376f 100644
--- a/sources/shiboken2/libshiboken/sbkstaticstrings_p.h
+++ b/sources/shiboken2/libshiboken/sbkstaticstrings_p.h
@@ -49,6 +49,7 @@ PyObject *compile();
 PyObject *function();
 PyObject *marshal();
 PyObject *method();
+PyObject *mro();
 PyObject *overload();
 PyObject *staticmethod();
 } // namespace PyName
diff --git a/sources/shiboken2/libshiboken/typespec.cpp b/sources/shiboken2/libshiboken/typespec.cpp
index 510ed51e6..902ed55af 100644
--- a/sources/shiboken2/libshiboken/typespec.cpp
+++ b/sources/shiboken2/libshiboken/typespec.cpp
@@ -714,7 +714,7 @@ PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases)
     if (PyType_Ready(type) < 0)
         goto fail;
 
-    // no ht_hached_keys in Python 2
+    // no ht_cached_keys in Python 2
     // if (type->tp_dictoffset) {
     //     res->ht_cached_keys = _PyDict_NewKeysForClass();
     // }
diff --git a/sources/shiboken2/libshiboken/voidptr.cpp b/sources/shiboken2/libshiboken/voidptr.cpp
index d4ce58c87..46f49b67e 100644
--- a/sources/shiboken2/libshiboken/voidptr.cpp
+++ b/sources/shiboken2/libshiboken/voidptr.cpp
@@ -335,7 +335,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 *)object_dealloc},
+    {Py_tp_dealloc, (void *)Sbk_object_dealloc},
     {Py_tp_methods, (void *)SbkVoidPtrObject_methods},
     {0, nullptr}
 };