From be202bd1baf600c7b422c96cc6b47a327e7a9d23 Mon Sep 17 00:00:00 2001
From: Friedemann Kleint <Friedemann.Kleint@qt.io>
Date: Wed, 25 Jul 2018 11:42:39 +0200
Subject: shiboken: Fix the allow-thread attribute to actually have an effect

Previously, calls to BEGIN_ALLOW_THREADS/END_ALLOW_THREADS were always
generated since the value of XML attribute was not used.

Fix it to actually use the value. Since having it default to "no"
caused a number of deadlocks (related to thread functions or functions
calling a virtual function (potentially reimplemented in Python),
introduce "auto" as default value. "auto" turns off
BEGIN_ALLOW_THREADS/END_ALLOW_THREADS for simple getter functions.

Task-number: PYSIDE-743
Change-Id: I4833afef14f2552c75b3424417c2702ce25cb379
Reviewed-by: Christian Tismer <tismer@stackless.com>
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>
---
 .../shiboken2/generator/shiboken2/cppgenerator.cpp | 36 ++++++++++++++--------
 .../shiboken2/generator/shiboken2/cppgenerator.h   |  4 ++-
 2 files changed, 26 insertions(+), 14 deletions(-)

(limited to 'sources/shiboken2/generator/shiboken2')

diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
index 8bddef700..f230782d1 100644
--- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
@@ -186,18 +186,19 @@ QVector<AbstractMetaFunctionList> CppGenerator::filterGroupedOperatorFunctions(c
     return result;
 }
 
-bool CppGenerator::hasBoolCast(const AbstractMetaClass* metaClass) const
+const AbstractMetaFunction *CppGenerator::boolCast(const AbstractMetaClass* metaClass) const
 {
     if (!useIsNullAsNbNonZero())
-        return false;
+        return nullptr;
     // TODO: This could be configurable someday
     const AbstractMetaFunction* func = metaClass->findFunction(QLatin1String("isNull"));
     if (!func || !func->type() || !func->type()->typeEntry()->isPrimitive() || !func->isPublic())
-        return false;
+        return nullptr;
     const PrimitiveTypeEntry* pte = static_cast<const PrimitiveTypeEntry*>(func->type()->typeEntry());
     while (pte->referencedTypeEntry())
         pte = pte->referencedTypeEntry();
-    return func && func->isConstant() && pte->name() == QLatin1String("bool") && func->arguments().isEmpty();
+    return func && func->isConstant() && pte->name() == QLatin1String("bool")
+        && func->arguments().isEmpty() ? func : nullptr;
 }
 
 typedef QMap<QString, AbstractMetaFunctionList> FunctionGroupMap;
@@ -490,16 +491,20 @@ void CppGenerator::generateClass(QTextStream &s, GeneratorContext &classContext)
         }
     }
 
-    if (hasBoolCast(metaClass)) {
+    if (const AbstractMetaFunction *f = boolCast(metaClass)) {
         ErrorCode errorCode(-1);
         s << "static int " << cpythonBaseName(metaClass) << "___nb_bool(PyObject* " PYTHON_SELF_VAR ")" << endl;
         s << '{' << endl;
         writeCppSelfDefinition(s, classContext);
-        s << INDENT << "int result;" << endl;
-        s << INDENT << BEGIN_ALLOW_THREADS << endl;
-        s << INDENT << "result = !" CPP_SELF_VAR "->isNull();" << endl;
-        s << INDENT << END_ALLOW_THREADS << endl;
-        s << INDENT << "return result;" << endl;
+        if (f->allowThread()) {
+            s << INDENT << "int result;" << endl;
+            s << INDENT << BEGIN_ALLOW_THREADS << endl;
+            s << INDENT << "result = !" CPP_SELF_VAR "->isNull();" << endl;
+            s << INDENT << END_ALLOW_THREADS << endl;
+            s << INDENT << "return result;" << endl;
+        } else {
+            s << INDENT << "return !" << CPP_SELF_VAR "->isNull();" << endl;
+        }
         s << '}' << endl << endl;
     }
 
@@ -3270,7 +3275,10 @@ void CppGenerator::writeMethodCall(QTextStream &s, const AbstractMetaFunction *f
         }
 
         if (!injectedCodeCallsCppFunction(func)) {
-            s << INDENT << BEGIN_ALLOW_THREADS << endl << INDENT;
+            const bool allowThread = func->allowThread();
+            if (allowThread)
+                s << INDENT << BEGIN_ALLOW_THREADS << endl;
+            s << INDENT;
             if (isCtor) {
                 s << (useVAddr.isEmpty() ?
                       QString::fromLatin1("cptr = %1;").arg(methodCall) : useVAddr) << endl;
@@ -3303,7 +3311,8 @@ void CppGenerator::writeMethodCall(QTextStream &s, const AbstractMetaFunction *f
             } else {
                 s << methodCall << ';' << endl;
             }
-            s << INDENT << END_ALLOW_THREADS << endl;
+            if (allowThread)
+                s << INDENT << END_ALLOW_THREADS << endl;
 
             if (!func->conversionRule(TypeSystem::TargetLangCode, 0).isEmpty()) {
                 writeConversionRule(s, func, TypeSystem::TargetLangCode, QLatin1String(PYTHON_RETURN_VAR));
@@ -4010,7 +4019,8 @@ void CppGenerator::writeTypeAsNumberDefinition(QTextStream& s, const AbstractMet
 
     QString baseName = cpythonBaseName(metaClass);
 
-    nb[QLatin1String("bool")] = hasBoolCast(metaClass) ? baseName + QLatin1String("___nb_bool") : QString();
+    if (hasBoolCast(metaClass))
+        nb.insert(QLatin1String("bool"), baseName + QLatin1String("___nb_bool"));
 
     for (QHash<QString, QString>::const_iterator it = m_nbFuncs.cbegin(), end = m_nbFuncs.cend(); it != end; ++it) {
         const QString &nbName = it.key();
diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.h b/sources/shiboken2/generator/shiboken2/cppgenerator.h
index 49a1e1835..d810665e9 100644
--- a/sources/shiboken2/generator/shiboken2/cppgenerator.h
+++ b/sources/shiboken2/generator/shiboken2/cppgenerator.h
@@ -328,7 +328,9 @@ private:
 
     QString writeReprFunction(QTextStream &s, GeneratorContext &context);
 
-    bool hasBoolCast(const AbstractMetaClass* metaClass) const;
+    const AbstractMetaFunction *boolCast(const AbstractMetaClass* metaClass) const;
+    bool hasBoolCast(const AbstractMetaClass* metaClass) const
+    { return boolCast(metaClass) != nullptr; }
 
     // Number protocol structure members names.
     static QHash<QString, QString> m_nbFuncs;
-- 
cgit v1.2.3