From 66a58efd807e3b168915699289d3e98f4305441a Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Thu, 28 Mar 2024 09:32:09 +0100 Subject: libpyside/Signal manager: Ensure cleanup of main thread targets Restore a notification on a sender's QObject::destroy() signal for main thread objects. Instead of triggering instant deletion as was done before (causing issues with recursion and threads), start an idle timer to call the Signal manager cleanup. Amends 1270a9e82e5bc3bd53a1131698ece60403da1192. Task-number: PYSIDE-2646 Task-number: PYSIDE-2141 Change-Id: Ifdc28f729cab64d58ac2ab300daece98b167d915 Reviewed-by: Qt CI Bot Reviewed-by: Cristian Maureira-Fredes (cherry picked from commit 279658b856c3d7c5ce5cd936af2da018fe33d13a) Reviewed-by: Qt Cherry-pick Bot --- sources/pyside6/libpyside/signalmanager.cpp | 69 ++++++++++++++++++++++++++-- sources/pyside6/tests/signals/lambda_test.py | 24 +++++++++- 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/sources/pyside6/libpyside/signalmanager.cpp b/sources/pyside6/libpyside/signalmanager.cpp index 422d0eeaa..625e4a405 100644 --- a/sources/pyside6/libpyside/signalmanager.cpp +++ b/sources/pyside6/libpyside/signalmanager.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -43,6 +44,9 @@ using namespace Qt::StringLiterals; static PyObject *metaObjectAttr = nullptr; static PyObject *parseArguments(const QMetaMethod &method, void **args); static bool emitShortCircuitSignal(QObject *source, int signalIndex, PyObject *args); + +static bool qAppRunning = false; + static void destroyMetaObject(PyObject *obj) { void *ptr = PyCapsule_GetPointer(obj, nullptr); @@ -225,6 +229,39 @@ using GlobalReceiverV2Map = QHashtimerId() == m_timerId) { + killTimer(std::exchange(m_timerId, -1)); + SignalManager::instance().purgeEmptyGlobalReceivers(); + } +} + struct SignalManager::SignalManagerPrivate { Q_DISABLE_COPY_MOVE(SignalManagerPrivate) @@ -243,6 +280,8 @@ struct SignalManager::SignalManagerPrivate static int qtPropertyMetacall(QObject *object, QMetaObject::Call call, int id, void **args); static int qtMethodMetacall(QObject *object, int id, void **args); + + QPointer m_listener; }; SignalManager::QmlMetaCallErrorHandler @@ -316,16 +355,27 @@ void SignalManager::setQmlMetaCallErrorHandler(QmlMetaCallErrorHandler handler) static void qAppAboutToQuit() { + qAppRunning = false; SignalManager::instance().purgeEmptyGlobalReceivers(); } +static bool isInMainThread(const QObject *o) +{ + if (o->isWidgetType() || o->isWindowType() || o->isQuickItemType()) + return true; + auto *app = QCoreApplication::instance(); + return app != nullptr && app->thread() == o->thread(); +} + QObject *SignalManager::globalReceiver(QObject *sender, PyObject *callback, QObject *receiver) { - static bool registerQuitHandler = true; - if (registerQuitHandler) { + if (m_d->m_listener.isNull() && !QCoreApplication::closingDown()) { if (auto *app = QCoreApplication::instance()) { - registerQuitHandler = false; + // The signal manager potentially outlives QCoreApplication, ensure deletion + m_d->m_listener = new SignalManagerDestroyListener(app); + m_d->m_listener->setObjectName("qt_pyside_signalmanagerdestroylistener"); QObject::connect(app, &QCoreApplication::aboutToQuit, qAppAboutToQuit); + qAppRunning = true; } } @@ -336,9 +386,18 @@ QObject *SignalManager::globalReceiver(QObject *sender, PyObject *callback, QObj auto gr = std::make_shared(callback, receiver); it = globalReceivers.insert(key, gr); } - if (sender) + + if (sender != nullptr) { it.value()->incRef(sender); // create a link reference + // For main thread-objects, add a notification for destroy (PYSIDE-2646, 2141) + if (qAppRunning && !m_d->m_listener.isNull() && isInMainThread(sender)) { + QObject::connect(sender, &QObject::destroyed, + m_d->m_listener, &SignalManagerDestroyListener::destroyNotify, + Qt::UniqueConnection); + } + } + return it.value().get(); } @@ -776,3 +835,5 @@ static bool emitShortCircuitSignal(QObject *source, int signalIndex, PyObject *a source->qt_metacall(QMetaObject::InvokeMetaMethod, signalIndex, signalArgs); return true; } + +#include "signalmanager.moc" diff --git a/sources/pyside6/tests/signals/lambda_test.py b/sources/pyside6/tests/signals/lambda_test.py index c3198c305..23fcdf5fa 100644 --- a/sources/pyside6/tests/signals/lambda_test.py +++ b/sources/pyside6/tests/signals/lambda_test.py @@ -7,13 +7,14 @@ import os import sys import unittest +import weakref from pathlib import Path sys.path.append(os.fspath(Path(__file__).resolve().parents[1])) from init_paths import init_test_paths init_test_paths(False) -from PySide6.QtCore import QObject, Signal, SIGNAL, QProcess +from PySide6.QtCore import QCoreApplication, QObject, Signal, SIGNAL, QProcess from helper.usesqapplication import UsesQApplication @@ -96,6 +97,27 @@ class QtSigLambda(UsesQApplication): self.assertTrue(dummy.called) self.assertEqual(dummy.exit_code, proc.exitCode()) + def testRelease(self): + """PYSIDE-2646: Test whether main thread target slot lambda/methods + (and their captured objects) are released by the signal manager + after a while.""" + + def do_connect(sender): + receiver = Receiver() + sender.void_signal.connect(lambda: setattr(receiver, 'called', True)) + return receiver + + sender = Sender() + receiver = weakref.ref(do_connect(sender)) + sender.emit_void() + self.assertTrue(receiver().called) + del sender + for i in range(3): + if not receiver(): + break + QCoreApplication.processEvents() + self.assertFalse(receiver()) + if __name__ == '__main__': unittest.main() -- cgit v1.2.3