aboutsummaryrefslogtreecommitdiffstats
path: root/sources/pyside6/libpyside/dynamicslot.cpp
diff options
context:
space:
mode:
authorFriedemann Kleint <Friedemann.Kleint@qt.io>2024-07-09 13:08:05 +0200
committerFriedemann Kleint <Friedemann.Kleint@qt.io>2024-08-21 22:41:13 +0200
commit33bd61d13d8d9e3794b6049891be62f3351313d9 (patch)
tree73204bed688aa7c3268d5173dbe567cf78be8b3f /sources/pyside6/libpyside/dynamicslot.cpp
parent05c4e6372e9515f81534ff1d0816695d4e6a9b27 (diff)
libpyside: Reimplement signal connections for Python callables not targeting a QMetaMethod
The code previously used a instances of class GlobalReceiverV2 inheriting QObject in a hash in SignalManager per slot tracking the list of senders to be able to use standard signal/slot connections in Qt. This was a complicated data structure and had issues with cleanups. This has been replaced by using an invoker object based on QtPrivate::QSlotObjectBase which can be passed to QObjectPrivate::connect(const QObject *, int signal, QtPrivate::QSlotObjectBase *, ...). The connections (identified by ConnectionKey) are now stored in a hash with QMetaObject::Connection as value, which can be used to disconnect using QObject::disconnect(QMetaObject::Connection). Deletion tracking is done by using signal QObject::destroyed(QObject*) which requires adapting some tests checking on the connection count and weak ref notification on receivers as was the case before. [ChangeLog][PySide6] Signal connections for Python callables not targeting a QMetaMethod has be reimplemented to simplify code and prepare for removal of the GIL. Task-number: PYSIDE-2810 Task-number: PYSIDE-2221 Change-Id: Ib55e73d4d7bfe6d7a8b7adc3ce3734eac5789bea Reviewed-by: Shyamnath Premnadh <Shyamnath.Premnadh@qt.io> Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>
Diffstat (limited to 'sources/pyside6/libpyside/dynamicslot.cpp')
-rw-r--r--sources/pyside6/libpyside/dynamicslot.cpp241
1 files changed, 212 insertions, 29 deletions
diff --git a/sources/pyside6/libpyside/dynamicslot.cpp b/sources/pyside6/libpyside/dynamicslot.cpp
index 182c2949f..b25928ea9 100644
--- a/sources/pyside6/libpyside/dynamicslot.cpp
+++ b/sources/pyside6/libpyside/dynamicslot.cpp
@@ -2,7 +2,6 @@
// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
#include "dynamicslot_p.h"
-#include "globalreceiverv2.h" // for GlobalReceiverMethodSlot
#include "pysidestaticstrings.h"
#include "pysideutils.h"
#include "pysideweakref.h"
@@ -13,10 +12,16 @@
#include <pep384ext.h>
#include <QtCore/QDebug>
+#include <QtCore/QtCompare>
+#include <QtCore/QCoreApplication>
+#include <QtCore/QHash>
+#include <QtCore/QPointer>
namespace PySide
{
+static void disconnectReceiver(PyObject *pythonSelf);
+
DynamicSlot::SlotType DynamicSlot::slotType(PyObject *callback)
{
if (PyMethod_Check(callback) != 0)
@@ -78,6 +83,8 @@ public:
explicit MethodDynamicSlot(PyObject *function, PyObject *pythonSelf);
~MethodDynamicSlot() override;
+ PyObject *pythonSelf() const { return m_pythonSelf; }
+
void call(const QByteArrayList &parameterTypes, const char *returnType,
void **cppArgs) override;
void formatDebug(QDebug &debug) const override;
@@ -139,45 +146,43 @@ TrackingMethodDynamicSlot::TrackingMethodDynamicSlot(PyObject *function, PyObjec
TrackingMethodDynamicSlot::~TrackingMethodDynamicSlot()
{
- Shiboken::GilState gil;
- Py_XDECREF(m_weakRef);
- m_weakRef = nullptr;
+ if (m_weakRef != nullptr) {
+ Shiboken::GilState gil;
+ // weakrefs must not be de-refed after the object has been deleted,
+ // else they get negative refcounts.
+ if (PyWeakref_GetObject(m_weakRef) != Py_None)
+ Py_DECREF(m_weakRef);
+ }
}
-// Delete the GlobalReceiver on pythonSelf deletion
-class GlobalReceiverMethodSlot : public TrackingMethodDynamicSlot
+// Delete the connection on receiver deletion by weakref
+class PysideReceiverMethodSlot : public TrackingMethodDynamicSlot
{
- Q_DISABLE_COPY_MOVE(GlobalReceiverMethodSlot)
+ Q_DISABLE_COPY_MOVE(PysideReceiverMethodSlot)
public:
- explicit GlobalReceiverMethodSlot(PyObject *function, PyObject *pythonSelf,
- GlobalReceiverV2 *parent);
- ~GlobalReceiverMethodSlot() override = default;
-
- GlobalReceiverV2 *parent() const { return m_parent; }
+ explicit PysideReceiverMethodSlot(PyObject *function, PyObject *pythonSelf);
-private:
- GlobalReceiverV2 *m_parent;
+ ~PysideReceiverMethodSlot() override = default;
};
-static void onGlobalReceiverSlotDestroyed(void *data)
+static void onPysideReceiverSlotDestroyed(void *data)
{
- auto *self = reinterpret_cast<GlobalReceiverMethodSlot *>(data);
+ auto *self = reinterpret_cast<PysideReceiverMethodSlot *>(data);
+ // Ensure the weakref is gone in case the connection stored in
+ // Qt's internals outlives Python.
self->releaseWeakRef();
Py_BEGIN_ALLOW_THREADS
- SignalManager::deleteGlobalReceiver(self->parent());
+ disconnectReceiver(self->pythonSelf());
Py_END_ALLOW_THREADS
}
-// monitor class from method lifetime
-GlobalReceiverMethodSlot::GlobalReceiverMethodSlot(PyObject *function, PyObject *pythonSelf,
- GlobalReceiverV2 *parent) :
+PysideReceiverMethodSlot::PysideReceiverMethodSlot(PyObject *function, PyObject *pythonSelf) :
TrackingMethodDynamicSlot(function, pythonSelf,
- WeakRef::create(pythonSelf, onGlobalReceiverSlotDestroyed, this)),
- m_parent(parent)
+ WeakRef::create(pythonSelf, onPysideReceiverSlotDestroyed, this))
{
}
-DynamicSlot* DynamicSlot::create(PyObject *callback, GlobalReceiverV2 *parent)
+DynamicSlot* DynamicSlot::create(PyObject *callback)
{
Shiboken::GilState gil;
switch (slotType(callback)) {
@@ -185,9 +190,7 @@ DynamicSlot* DynamicSlot::create(PyObject *callback, GlobalReceiverV2 *parent)
PyObject *function = PyMethod_GET_FUNCTION(callback);
Py_INCREF(function);
PyObject *pythonSelf = PyMethod_GET_SELF(callback);
- if (parent != nullptr)
- return new GlobalReceiverMethodSlot(function, pythonSelf, parent);
- return new MethodDynamicSlot(function, pythonSelf);
+ return new PysideReceiverMethodSlot(function, pythonSelf);
}
case SlotType::CompiledMethod: {
// PYSIDE-1523: PyMethod_Check is not accepting compiled form, we just go by attributes.
@@ -195,9 +198,7 @@ DynamicSlot* DynamicSlot::create(PyObject *callback, GlobalReceiverV2 *parent)
Py_DECREF(function);
PyObject *pythonSelf = PyObject_GetAttr(callback, PySide::PySideName::im_self());
Py_DECREF(pythonSelf);
- if (parent != nullptr)
- return new GlobalReceiverMethodSlot(function, pythonSelf, parent);
- return new MethodDynamicSlot(function, pythonSelf);
+ return new PysideReceiverMethodSlot(function, pythonSelf);
}
case SlotType::Callable:
break;
@@ -217,4 +218,186 @@ QDebug operator<<(QDebug debug, const DynamicSlot *ds)
return debug;
}
+// Connection code for signal connections that use a Python callable not
+// targeting the QMetaMethod of a QObject (no index). They require
+// invocation in a class inheriting QtPrivate::QSlotObjectBase (PySideQSlotObject
+// aggregating DynamicSlot), which is passed to:
+// QObjectPrivate::connect(const QObject *, int signal, QtPrivate::QSlotObjectBase *, ...).
+// For each of those connections (identified by ConnectionKey), we maintain a
+// hash of ConnectionKey->QMetaObject::Connection for:
+//
+// - Disconnecting: Retrieve QMetaObject::Connection for the connection parameters
+//
+// - Tracking sender (QObject) life time and clean hash on destroyed()
+//
+// - Tracking receiver (PyObject*) life time via weakref in case of a
+// connection to a method and proactively disconnect on weakref
+// notification as not to cause leaks of the instance.
+
+struct ConnectionKey
+{
+ const QObject *sender;
+ int senderIndex;
+ const PyObject *object;
+ const PyObject *method;
+
+ friend constexpr size_t qHash(const ConnectionKey &k, size_t seed = 0) noexcept
+ {
+ return qHashMulti(seed, k.sender, k.senderIndex, k.object, k.method);
+ }
+
+ friend constexpr bool comparesEqual(const ConnectionKey &lhs,
+ const ConnectionKey &rhs) noexcept
+ {
+ return lhs.sender == rhs.sender && lhs.senderIndex == rhs.senderIndex
+ && lhs.object == rhs.object && lhs.method == rhs.method;
+ }
+
+ Q_DECLARE_EQUALITY_COMPARABLE(ConnectionKey);
+};
+
+QDebug operator<<(QDebug debug, const ConnectionKey &k)
+{
+ QDebugStateSaver saver(debug);
+ debug.noquote();
+ debug.nospace();
+ debug << "ConnectionKey(" << static_cast<const void *>(k.sender)
+ << '/' << k.sender->metaObject()->className();
+ auto on = k.sender->objectName();
+ if (!on.isEmpty())
+ debug << "/\"" << on << '"';
+ debug << ", index=" << k.senderIndex << ", target="
+ << PySide::debugPyObject(const_cast<PyObject *>(k.object));
+ if (k.method != nullptr)
+ debug << ", method=" << PySide::debugPyObject(const_cast<PyObject *>(k.method));
+ debug << ')';
+ return debug;
+}
+
+QDebug operator<<(QDebug debug, const QMetaObject::Connection &c)
+{
+ QDebugStateSaver saver(debug);
+ debug.noquote();
+ debug.nospace();
+ debug << "Connection(";
+ if (c)
+ debug << static_cast<const void *>(&c); // d-ptr;
+ else
+ debug << '0';
+ debug << ')';
+ return debug;
+}
+
+using ConnectionHash = QHash<ConnectionKey, QMetaObject::Connection>;
+
+static ConnectionHash connectionHash;
+
+static ConnectionKey connectionKey(const QObject *sender, int senderIndex,
+ PyObject *callback)
+{
+ PyObject *object{};
+ PyObject *method{};
+
+ switch (DynamicSlot::slotType(callback)) {
+ case DynamicSlot::SlotType::Method:
+ // PYSIDE-1422: Avoid hash on self which might be unhashable.
+ object = PyMethod_GET_SELF(callback);
+ method = PyMethod_GET_FUNCTION(callback);
+ break;
+ case DynamicSlot::SlotType::CompiledMethod: {
+ // PYSIDE-1589: Fix for slots in compiled functions
+ Shiboken::AutoDecRef self(PyObject_GetAttr(callback, PySide::PySideName::im_self()));
+ Shiboken::AutoDecRef func(PyObject_GetAttr(callback, PySide::PySideName::im_func()));
+ object = self.object();
+ method = func.object();
+ break;
+ }
+ case DynamicSlot::SlotType::Callable:
+ method = callback;
+ break;
+ }
+
+ return {sender, senderIndex, object, method};
+}
+
+// Listens to QObject::destroyed of senders and removes them from the hash.
+class SenderSignalDeletionTracker : public QObject
+{
+ Q_OBJECT
+public:
+ using QObject::QObject;
+
+public Q_SLOTS:
+ void senderDestroyed(QObject *o);
+};
+
+void SenderSignalDeletionTracker::senderDestroyed(QObject *o)
+{
+ for (auto it = connectionHash.begin(); it != connectionHash.end(); ) {
+ if (it.key().sender == o)
+ it = connectionHash.erase(it);
+ else
+ ++it;
+ }
+}
+
+static QPointer<SenderSignalDeletionTracker> senderSignalDeletionTracker;
+
+static void disconnectReceiver(PyObject *pythonSelf)
+{
+ // A check for reentrancy was added for PYSIDE-88, but has not been
+ // observed yet.
+ for (bool keepGoing = true; keepGoing; ) {
+ keepGoing = false;
+ for (auto it = connectionHash.begin(); it != connectionHash.end(); ) {
+ if (it.key().object == pythonSelf) {
+ const auto oldSize = connectionHash.size();
+ QObject::disconnect(it.value());
+ it = connectionHash.erase(it);
+ // Check for a disconnection causing deletion of further objects
+ // by a re-entrant call.
+ if (connectionHash.size() < oldSize - 1) {
+ keepGoing = true;
+ break; // Iterators were invalidated, retry
+ }
+ } else {
+ ++it;
+ }
+ }
+ }
+}
+
+static void clearConnectionHash()
+{
+ connectionHash.clear();
+}
+
+void registerSlotConnection(QObject *source, int signalIndex, PyObject *callback,
+ const QMetaObject::Connection &connection)
+{
+ connectionHash.insert(connectionKey(source, signalIndex, callback), connection);
+ if (senderSignalDeletionTracker.isNull()) {
+ auto *app = QCoreApplication::instance();
+ senderSignalDeletionTracker = new SenderSignalDeletionTracker(app);
+ Py_AtExit(clearConnectionHash);
+ }
+
+ QObject::connect(source, &QObject::destroyed,
+ senderSignalDeletionTracker, &SenderSignalDeletionTracker::senderDestroyed,
+ Qt::UniqueConnection);
+}
+
+bool disconnectSlot(QObject *source, int signalIndex, PyObject *callback)
+{
+ auto it = connectionHash.find(connectionKey(source, signalIndex, callback));
+ const bool ok = it != connectionHash.end();
+ if (ok) {
+ QObject::disconnect(it.value());
+ connectionHash.erase(it);
+ }
+ return ok;
+}
+
} // namespace PySide
+
+#include "dynamicslot.moc"
generated by cgit v1.2.3 (git 2.25.1) at 2025-12-17 15:49:28 +0000