libpyside: Refactor GlobalReceiverV2Map

Use a QHash of object/method PyObject instead of a
QByteArray representing the sum of hash values of both
as a string.

The problem with using hash functions here is that 2 keys
might be identical due to the hashing.

Rename the hash() methods to key().

Task-number: PYSIDE-1422
Change-Id: Ie1344d8bd85a073ef5f6e2cb461bd2f514265a9f
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>
Reviewed-by: Christian Tismer <tismer@stackless.com>

3 files changed, 57 insertions, 43 deletions

diff --git a/sources/pyside6/libpyside/globalreceiverv2.cpp b/sources/pyside6/libpyside/globalreceiverv2.cpp
index 6d6ae3abf..8ff5d896f 100644
--- a/sources/pyside6/libpyside/globalreceiverv2.cpp
+++ b/sources/pyside6/libpyside/globalreceiverv2.cpp
@@ -45,6 +45,7 @@
 #include <autodecref.h>
 #include <gilstate.h>
 
+#include <QtCore/qhashfunctions.h>
 #include <QtCore/QMetaMethod>
 #include <QtCore/QSet>
 
@@ -58,6 +59,15 @@ namespace
 
 namespace PySide
 {
+
+size_t qHash(const GlobalReceiverKey &k, size_t seed)
+{
+    QtPrivate::QHashCombine hash;
+    seed = hash(seed, k.object);
+    seed = hash(seed, k.method);
+    return seed;
+}
+
 class DynamicSlotDataV2
 {
     Q_DISABLE_COPY(DynamicSlotDataV2)
@@ -68,34 +78,32 @@ class DynamicSlotDataV2
         int addSlot(const char *signature);
         int id(const char *signature) const;
         PyObject *callback();
-        QByteArray hash() const;
+        GlobalReceiverKey key() const { return {m_pythonSelf, m_callback}; }
         void notify();
 
         static void onCallbackDestroyed(void *data);
-        static QByteArray hash(PyObject *callback);
-
+        static GlobalReceiverKey key(PyObject *callback);
 
     private:
         bool m_isMethod;
         PyObject *m_callback;
-        PyObject *m_pythonSelf;
-        PyObject *m_pyClass;
-        PyObject *m_weakRef;
+        PyObject *m_pythonSelf = nullptr;
+        PyObject *m_pyClass = nullptr;
+        PyObject *m_weakRef = nullptr;
         QMap<QByteArray, int> m_signatures;
         GlobalReceiverV2 *m_parent;
-        QByteArray m_hash;
 };
 
 }
 
 using namespace PySide;
 
-DynamicSlotDataV2::DynamicSlotDataV2(PyObject *callback, GlobalReceiverV2 *parent)
-    : m_pythonSelf(0), m_pyClass(0), m_weakRef(0), m_parent(parent)
+DynamicSlotDataV2::DynamicSlotDataV2(PyObject *callback, GlobalReceiverV2 *parent) :
+    m_isMethod(PyMethod_Check(callback)),
+    m_parent(parent)
 {
     Shiboken::GilState gil;
 
-    m_isMethod = PyMethod_Check(callback);
     if (m_isMethod) {
         //Can not store calback pointe because this will be destroyed at the end of the scope
         //To avoid increment intance reference keep the callback information
@@ -105,31 +113,20 @@ DynamicSlotDataV2::DynamicSlotDataV2(PyObject *callback, GlobalReceiverV2 *paren
         //monitor class from method lifetime
         m_weakRef = WeakRef::create(m_pythonSelf, DynamicSlotDataV2::onCallbackDestroyed, this);
 
-        // PYSIDE-1422: Avoid hash on self which might be unhashable.
-        m_hash = QByteArray::number(static_cast<qlonglong>(PyObject_Hash(m_callback)))
-                 + QByteArray::number(reinterpret_cast<qlonglong>(m_pythonSelf));
     } else {
         m_callback = callback;
         Py_INCREF(m_callback);
-
-        m_hash = QByteArray::number(static_cast<qlonglong>(PyObject_Hash(m_callback)));
     }
 }
 
-QByteArray DynamicSlotDataV2::hash() const
-{
-    return m_hash;
-}
-
-QByteArray DynamicSlotDataV2::hash(PyObject *callback)
+GlobalReceiverKey DynamicSlotDataV2::key(PyObject *callback)
 {
     Shiboken::GilState gil;
     if (PyMethod_Check(callback)) {
         // PYSIDE-1422: Avoid hash on self which might be unhashable.
-        return  QByteArray::number(static_cast<qlonglong>(PyObject_Hash(PyMethod_GET_FUNCTION(callback))))
-              + QByteArray::number(reinterpret_cast<qlonglong>(PyMethod_GET_SELF(callback)));
+        return {PyMethod_GET_SELF(callback), PyMethod_GET_FUNCTION(callback)};
     }
-    return QByteArray::number(static_cast<qlonglong>(PyObject_Hash(callback)));
+    return {nullptr, callback};
 }
 
 PyObject *DynamicSlotDataV2::callback()
@@ -179,7 +176,7 @@ DynamicSlotDataV2::~DynamicSlotDataV2()
        Py_DECREF(m_callback);
 }
 
-GlobalReceiverV2::GlobalReceiverV2(PyObject *callback, SharedMap map) :
+GlobalReceiverV2::GlobalReceiverV2(PyObject *callback, GlobalReceiverV2MapPtr map) :
     QObject(nullptr),
     m_metaObject("__GlobalReceiver__", &QObject::staticMetaObject),
     m_sharedMap(std::move(map))
@@ -203,7 +200,7 @@ GlobalReceiverV2::~GlobalReceiverV2()
 {
     m_refs.clear();
     // Remove itself from map.
-    m_sharedMap->remove(m_data->hash());
+    m_sharedMap->remove(m_data->key());
     // Suppress handling of destroyed() for objects whose last reference is contained inside
     // the callback object that will now be deleted. The reference could be a default argument,
     // a callback local variable, etc.
@@ -286,14 +283,14 @@ void GlobalReceiverV2::notify()
     Py_END_ALLOW_THREADS
 }
 
-QByteArray GlobalReceiverV2::hash() const
+GlobalReceiverKey GlobalReceiverV2::key() const
 {
-    return m_data->hash();
+    return m_data->key();
 }
 
-QByteArray GlobalReceiverV2::hash(PyObject *callback)
+GlobalReceiverKey GlobalReceiverV2::key(PyObject *callback)
 {
-    return DynamicSlotDataV2::hash(callback);
+    return DynamicSlotDataV2::key(callback);
 }
 
 const QMetaObject *GlobalReceiverV2::metaObject() const
diff --git a/sources/pyside6/libpyside/globalreceiverv2.h b/sources/pyside6/libpyside/globalreceiverv2.h
index 433f587a9..da01e6d33 100644
--- a/sources/pyside6/libpyside/globalreceiverv2.h
+++ b/sources/pyside6/libpyside/globalreceiverv2.h
@@ -55,8 +55,26 @@ namespace PySide
 class DynamicSlotDataV2;
 class GlobalReceiverV2;
 
-typedef QMap<QByteArray, GlobalReceiverV2 *> GlobalReceiverV2Map;
-typedef QSharedPointer<GlobalReceiverV2Map> SharedMap;
+struct GlobalReceiverKey
+{
+    const PyObject *object;
+    const PyObject *method;
+};
+
+inline bool operator==(const GlobalReceiverKey &k1, const GlobalReceiverKey &k2)
+{
+    return k1.object == k2.object && k1.method == k2.method;
+}
+
+inline bool operator!=(const GlobalReceiverKey &k1, const GlobalReceiverKey &k2)
+{
+    return k1.object != k2.object || k1.method != k2.method;
+}
+
+size_t qHash(const GlobalReceiverKey &k, size_t seed = 0);
+
+using GlobalReceiverV2Map = QHash<GlobalReceiverKey, GlobalReceiverV2 *>;
+using GlobalReceiverV2MapPtr = QSharedPointer<GlobalReceiverV2Map>;
 
 /**
  * A class used to make the link between the C++ Signal/Slot and Python callback
@@ -72,7 +90,7 @@ public:
      * @param   callback    A Python callable object (can be a method or not)
      * @param   ma          A SharedPointer used on Signal manager that contains all instaces of GlobalReceiver
      **/
-    GlobalReceiverV2(PyObject *callback, SharedMap map);
+    GlobalReceiverV2(PyObject *callback, GlobalReceiverV2MapPtr map);
 
     /**
      * Destructor
@@ -125,7 +143,7 @@ public:
      *
      * @return  a string with a unique id based on GlobalReceiver contents
      **/
-    QByteArray hash() const;
+    GlobalReceiverKey key() const;
 
     /**
      * Use to retrieve the unique hash of the PyObject based on GlobalReceiver rules
@@ -133,7 +151,7 @@ public:
      * @param   callback The Python callable object used to calculate the id
      * @return  a string with a unique id based on GlobalReceiver contents
      **/
-    static QByteArray hash(PyObject *callback);
+    static GlobalReceiverKey key(PyObject *callback);
 
     const MetaObjectBuilder &metaObjectBuilder() const { return m_metaObject; }
     MetaObjectBuilder &metaObjectBuilder() { return m_metaObject; }
@@ -142,7 +160,7 @@ private:
     MetaObjectBuilder m_metaObject;
     DynamicSlotDataV2 *m_data;
     QList<const QObject *> m_refs;
-    SharedMap m_sharedMap;
+    GlobalReceiverV2MapPtr m_sharedMap;
 };
 
 }
diff --git a/sources/pyside6/libpyside/signalmanager.cpp b/sources/pyside6/libpyside/signalmanager.cpp
index 9507a3ff3..2c08f4328 100644
--- a/sources/pyside6/libpyside/signalmanager.cpp
+++ b/sources/pyside6/libpyside/signalmanager.cpp
@@ -212,11 +212,10 @@ using namespace PySide;
 
 struct SignalManager::SignalManagerPrivate
 {
-    SharedMap m_globalReceivers;
+    GlobalReceiverV2MapPtr m_globalReceivers;
 
-    SignalManagerPrivate()
+    SignalManagerPrivate() : m_globalReceivers(new GlobalReceiverV2Map{})
     {
-        m_globalReceivers = SharedMap( new QMap<QByteArray, GlobalReceiverV2 *>() );
     }
 
     ~SignalManagerPrivate()
@@ -294,13 +293,13 @@ SignalManager &SignalManager::instance()
 
 QObject *SignalManager::globalReceiver(QObject *sender, PyObject *callback)
 {
-    SharedMap globalReceivers = m_d->m_globalReceivers;
-    QByteArray hash = GlobalReceiverV2::hash(callback);
+    GlobalReceiverV2MapPtr globalReceivers = m_d->m_globalReceivers;
+    GlobalReceiverKey key = GlobalReceiverV2::key(callback);
     GlobalReceiverV2 *gr = nullptr;
-    auto it = globalReceivers->find(hash);
+    auto it = globalReceivers->find(key);
     if (it == globalReceivers->end()) {
         gr = new GlobalReceiverV2(callback, globalReceivers);
-        globalReceivers->insert(hash, gr);
+        globalReceivers->insert(key, gr);
         if (sender) {
             gr->incRef(sender); // create a link reference
             gr->decRef(); // remove extra reference