aboutsummaryrefslogtreecommitdiffstats
path: root/sources/pyside6
diff options
context:
space:
mode:
authorFriedemann Kleint <Friedemann.Kleint@qt.io>2025-07-31 08:55:29 +0200
committerFriedemann Kleint <Friedemann.Kleint@qt.io>2025-07-31 15:26:25 +0200
commitd580aae898e10e7b4491f2692f320d7706899c1e (patch)
tree630e156fb6624b1f684bedb563f349c30239c2f7 /sources/pyside6
parent160884d98fc8fee21b1c7725f5706d1fae47ccb4 (diff)
libpyside: Fix corruption connecting several signals to one non-QObject receiver
The deletion of the weak ref in ~TrackingMethodDynamicSlot() did not reliably stop the notification from being triggered, causing corruption later on since the address of the dynamic slot was used as user data. Use the Python receiver object as user data instead (which is only used for map look up) to delete the connection to protect against multiple invocation. Remove the deletion of the weakref since it is removed in the notification. Remove class TrackingMethodDynamicSlot as it does not really have any functionality any more. The comment about releasing the weakref in case DynamicSlot outlives Python is apparently not an issue since otherwise the deletion of the function in ~MethodDynamicSlot() would have caused issues. Amends 33bd61d13d8d9e3794b6049891be62f3351313d9. Pick-to: 6.9 6.8 Fixes: PYSIDE-3148 Task-number: PYSIDE-2810 Change-Id: Idc07d0774afaf99df93185c90e975291a42ffeaf Reviewed-by: Shyamnath Premnadh <Shyamnath.Premnadh@qt.io>
Diffstat (limited to 'sources/pyside6')
-rw-r--r--sources/pyside6/libpyside/dynamicslot.cpp51
-rw-r--r--sources/pyside6/tests/signals/CMakeLists.txt1
-rw-r--r--sources/pyside6/tests/signals/nonqobject_receivers_test.py71
3 files changed, 82 insertions, 41 deletions
diff --git a/sources/pyside6/libpyside/dynamicslot.cpp b/sources/pyside6/libpyside/dynamicslot.cpp
index cc6d50d82..3d9b9c1be 100644
--- a/sources/pyside6/libpyside/dynamicslot.cpp
+++ b/sources/pyside6/libpyside/dynamicslot.cpp
@@ -134,41 +134,8 @@ void MethodDynamicSlot::formatDebug(QDebug &debug) const
<< ", function=" << PySide::debugPyObject(m_function) << ')';
}
-// Store a weak reference on pythonSelf.
-class TrackingMethodDynamicSlot : public MethodDynamicSlot
-{
- Q_DISABLE_COPY_MOVE(TrackingMethodDynamicSlot)
-public:
- explicit TrackingMethodDynamicSlot(PyObject *function, PyObject *pythonSelf,
- PyObject *weakRef);
- ~TrackingMethodDynamicSlot() override;
-
- void releaseWeakRef() { m_weakRef = nullptr; }
-
-private:
- PyObject *m_weakRef;
-};
-
-TrackingMethodDynamicSlot::TrackingMethodDynamicSlot(PyObject *function, PyObject *pythonSelf,
- PyObject *weakRef) :
- MethodDynamicSlot(function, pythonSelf),
- m_weakRef(weakRef)
-{
-}
-
-TrackingMethodDynamicSlot::~TrackingMethodDynamicSlot()
-{
- 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 (PepExt_Weakref_IsAlive(m_weakRef))
- Py_DECREF(m_weakRef);
- }
-}
-
// Delete the connection on receiver deletion by weakref
-class PysideReceiverMethodSlot : public TrackingMethodDynamicSlot
+class PysideReceiverMethodSlot : public MethodDynamicSlot
{
Q_DISABLE_COPY_MOVE(PysideReceiverMethodSlot)
public:
@@ -179,19 +146,21 @@ public:
static void onPysideReceiverSlotDestroyed(void *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();
+ auto *pythonSelf = reinterpret_cast<PyObject *>(data);
Py_BEGIN_ALLOW_THREADS
- disconnectReceiver(self->pythonSelf());
+ disconnectReceiver(pythonSelf);
Py_END_ALLOW_THREADS
}
PysideReceiverMethodSlot::PysideReceiverMethodSlot(PyObject *function, PyObject *pythonSelf) :
- TrackingMethodDynamicSlot(function, pythonSelf,
- WeakRef::create(pythonSelf, onPysideReceiverSlotDestroyed, this))
+ MethodDynamicSlot(function, pythonSelf)
{
+ // PYSIDE-3148: The weakref is automatically deleted when the notification triggers.
+ // Note that notifications may trigger after deletion of TrackingMethodDynamicSlot in case
+ // of multiple connections to the same receiver, so, &DynamicSlot must not be used as user
+ // data. Also trying to actively deref a pending weak ref from ~TrackingMethodDynamicSlot()
+ // does not reliably prevent the notification from being triggered.
+ WeakRef::create(pythonSelf, onPysideReceiverSlotDestroyed, pythonSelf);
}
DynamicSlot* DynamicSlot::create(PyObject *callback)
diff --git a/sources/pyside6/tests/signals/CMakeLists.txt b/sources/pyside6/tests/signals/CMakeLists.txt
index dd8f4a016..094caaaf6 100644
--- a/sources/pyside6/tests/signals/CMakeLists.txt
+++ b/sources/pyside6/tests/signals/CMakeLists.txt
@@ -21,6 +21,7 @@ PYSIDE_TEST(qobject_callable_connect_test.py)
PYSIDE_TEST(qobject_destroyed_test.py)
PYSIDE_TEST(qobject_receivers_test.py)
PYSIDE_TEST(qobject_sender_test.py)
+PYSIDE_TEST(nonqobject_receivers_test.py)
PYSIDE_TEST(ref01_test.py)
PYSIDE_TEST(ref02_test.py)
PYSIDE_TEST(ref03_test.py)
diff --git a/sources/pyside6/tests/signals/nonqobject_receivers_test.py b/sources/pyside6/tests/signals/nonqobject_receivers_test.py
new file mode 100644
index 000000000..4bb47e5df
--- /dev/null
+++ b/sources/pyside6/tests/signals/nonqobject_receivers_test.py
@@ -0,0 +1,71 @@
+# Copyright (C) 2025 The Qt Company Ltd.
+# SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+'''Test case for non-QObject.receivers'''
+
+import os
+import sys
+import unittest
+
+from pathlib import Path
+sys.path.append(os.fspath(Path(__file__).resolve().parents[1]))
+from init_paths import init_test_paths # noqa: F401
+init_test_paths(False)
+
+from helper.usesqapplication import UsesQApplication # noqa: F401
+
+from PySide6.QtGui import QAction # noqa: F401
+
+
+receiver_instances = 0
+
+
+class Receiver:
+
+ def __init__(self):
+ global receiver_instances
+ receiver_instances += 1
+ self.slot1Triggered = 0
+ self.slot2Triggered = 0
+
+ def __del__(self):
+ global receiver_instances
+ receiver_instances -= 1
+
+ def slot1(self):
+ self.slot1Triggered += 1
+
+ def slot2(self):
+ self.slot2Triggered += 1
+
+
+class TestQObjectReceivers(UsesQApplication):
+ '''Test case for non-QObject.receivers'''
+
+ @unittest.skipUnless(hasattr(sys, "getrefcount"), f"{sys.implementation.name} has no refcount")
+ def testBasic(self):
+ '''The test verifies that connections to methods of a non-QObject work
+ (TrackingMethodDynamicSlot). Also, despite being stored in the signal manager,
+ making connections should not increase references on the receiver or prevent
+ the receivers from being deleted, which is achieved using weak reference
+ tracking notifications.
+ 2 connections are used to trigger a corruption caused by multiple weak reference
+ notifications (PYSIDE-3148).'''
+ action1 = QAction("a1", qApp) # noqa: F821
+ action2 = QAction("a2", qApp) # noqa: F821
+ receiver = Receiver()
+ self.assertEqual(receiver_instances, 1)
+ base_ref_count = sys.getrefcount(receiver)
+ action1.triggered.connect(receiver.slot1)
+ action2.triggered.connect(receiver.slot2)
+ self.assertEqual(sys.getrefcount(receiver), base_ref_count)
+ action1.trigger()
+ action2.trigger()
+ self.assertEqual(receiver.slot1Triggered, 1)
+ self.assertEqual(receiver.slot2Triggered, 1)
+ receiver = 0
+ self.assertEqual(receiver_instances, 0)
+
+
+if __name__ == '__main__':
+ unittest.main()
generated by cgit v1.2.3 (git 2.25.1) at 2025-12-12 05:52:40 +0000