Avoid the GIL in SignalManager::retrieveMetaObject

After massive GIL savings in the generated code, there still
exists a place where a lot of repeated GIL acquirements are
done. It was observed that up to 24 consecutive calls to
retrieveMetaObject(self) were made, all with the same value
for 'self'.

The code in question was:

(1)    Shiboken::GilState gil;
(2)    MetaObjectBuilder *builder = metaBuilderFromDict( \
                   reinterpret_cast<SbkObject *>(self)->ob_dict);
(3)    if (!builder)
(4)        builder = &(retrieveTypeUserData(self)->mo);
(5)
(6)    return builder->update();

An exact analysis of the code showed that the GIL usage (1) could
be moved out of the function into a deeper function that does
updates in a branch (6).

Function retrieveTypeUserData does not involve the Python
interpreter at all (4).

It took some time until it was proven that access to some
special Python dictionary cannot reach the Python interpreter
and therefore does not need the GIL as well (2).

This replaces the abandoned effort to write a "Lazy GIL".
It worked great for the example program, but had problems
with some never finishing tests.

After all, this solution is much simpler and works perfectly
well. More effort seems not to be necessary to handle the GIL.

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

2 files changed, 18 insertions, 1 deletions

diff --git a/sources/pyside2/libpyside/dynamicqmetaobject.cpp b/sources/pyside2/libpyside/dynamicqmetaobject.cpp
index 2c5a0ee05..51e6598b3 100644
--- a/sources/pyside2/libpyside/dynamicqmetaobject.cpp
+++ b/sources/pyside2/libpyside/dynamicqmetaobject.cpp
@@ -413,6 +413,10 @@ const QMetaObject *MetaObjectBuilderPrivate::update()
     if (!m_builder)
         return m_baseObject;
     if (m_cachedMetaObjects.empty() || m_dirty) {
+        // PYSIDE-803: The dirty branch needs to be protected by the GIL.
+        // This was moved from SignalManager::retrieveMetaObject to here,
+        // which is only the update in "return builder->update()".
+        Shiboken::GilState gil;
         m_cachedMetaObjects.push_back(m_builder->toMetaObject());
         checkMethodOrder(m_cachedMetaObjects.back());
         m_dirty = false;
diff --git a/sources/pyside2/libpyside/signalmanager.cpp b/sources/pyside2/libpyside/signalmanager.cpp
index c21a3e565..01b347a3d 100644
--- a/sources/pyside2/libpyside/signalmanager.cpp
+++ b/sources/pyside2/libpyside/signalmanager.cpp
@@ -550,6 +550,12 @@ bool SignalManager::registerMetaMethod(QObject *source, const char *signature, Q
 
 static MetaObjectBuilder *metaBuilderFromDict(PyObject *dict)
 {
+    // PYSIDE-803: The dict in this function is the ob_dict of an SbkObject.
+    // The "metaObjectAttr" entry is only handled in this file. There is no
+    // way in this function to involve the interpreter. Therefore, we need
+    // no GIL.
+    // Note that "SignalManager::registerMetaMethodGetIndex" has write actions
+    // that might involve the interpreter, but in that context the GIL is held.
     if (!dict || !PyDict_Contains(dict, metaObjectAttr))
         return nullptr;
 
@@ -605,7 +611,14 @@ int SignalManager::registerMetaMethodGetIndex(QObject *source, const char *signa
 
 const QMetaObject *SignalManager::retrieveMetaObject(PyObject *self)
 {
-    Shiboken::GilState gil;
+    // PYSIDE-803: Avoid the GIL in SignalManager::retrieveMetaObject
+    // This function had the GIL. We do not use the GIL unless we have to.
+    // metaBuilderFromDict accesses a Python dict, but in that context there
+    // is no way to reach the interpreter, see "metaBuilderFromDict".
+    //
+    // The update function is MetaObjectBuilderPrivate::update in
+    // dynamicmetaobject.c . That function now uses the GIL when the
+    // m_dirty flag is set.
     Q_ASSERT(self);
 
     MetaObjectBuilder *builder = metaBuilderFromDict(reinterpret_cast<SbkObject *>(self)->ob_dict);