aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAdrian Herrmann <adrian.herrmann@qt.io>2023-05-10 15:54:54 +0200
committerQt Cherry-pick Bot <cherrypick_bot@qt-project.org>2023-05-11 15:38:03 +0000
commit58ec2ac3d01a8f01a951699ae7b1d2a64ffef845 (patch)
tree3b14d3fd42f2d999dc5ff10d03de98ec089817d0
parent32a097c74d8318f2f14e2caab752e9d9b691df25 (diff)
Fix leak connecting signals to anonymous functions
When connecting a signal to the same anonymous function repeatedly, a new GlobalReceiverV2 object would be created after each call. Each GlobalReceiverV2 would have a unique callback object despite all these callback objects sharing the same code. This would lead to a large number of GlobalReceiverV2 and callback objects, each never reaching a refcount of 0 and thus never being released. The remedy is that we only need one GlobalReceiverV2 object, whose corresponding GlobalReceiverKey references not the outer callback object, but the code object associated with it. Fixes: PYSIDE-2299 Change-Id: I474284dc5ce08dc6601636f2e7ac5e5a10ed8560 Reviewed-by: Christian Tismer <tismer@stackless.com> (cherry picked from commit 5b39b316e3c9e40cdc0784538b8d5f290e41d67b) Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r--sources/pyside6/libpyside/globalreceiverv2.cpp6
-rw-r--r--sources/pyside6/tests/signals/anonymous_slot_leak_test.py72
2 files changed, 77 insertions, 1 deletions
diff --git a/sources/pyside6/libpyside/globalreceiverv2.cpp b/sources/pyside6/libpyside/globalreceiverv2.cpp
index 8289eaff6..c623d05fa 100644
--- a/sources/pyside6/libpyside/globalreceiverv2.cpp
+++ b/sources/pyside6/libpyside/globalreceiverv2.cpp
@@ -106,7 +106,11 @@ GlobalReceiverKey DynamicSlotDataV2::key(PyObject *callback)
Shiboken::AutoDecRef func(PyObject_GetAttr(callback, PySide::PySideName::im_func()));
return {self, func};
}
- return {nullptr, callback};
+ // PYSIDE-2299: Callbacks can have the same code, but we only need one GlobalReceiverV2 for all
+ // of them. If we used the callback itself instead of the code object, we would
+ // create a new GlobalReceiverV2 for each in SignalManager::globalReceiver()
+ // (signalmanager.cpp), leaking memory.
+ return {nullptr, PyFunction_GetCode(callback)};
}
PyObject *DynamicSlotDataV2::callback()
diff --git a/sources/pyside6/tests/signals/anonymous_slot_leak_test.py b/sources/pyside6/tests/signals/anonymous_slot_leak_test.py
new file mode 100644
index 000000000..3b297111f
--- /dev/null
+++ b/sources/pyside6/tests/signals/anonymous_slot_leak_test.py
@@ -0,0 +1,72 @@
+# Copyright (C) 2023 The Qt Company Ltd.
+# SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+import os
+import sys
+import unittest
+
+from functools import partial
+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.QtWidgets import QWidget
+from helper.usesqapplication import UsesQApplication
+
+
+try:
+ sys.gettotalrefcount
+ have_debug = True
+except AttributeError:
+ have_debug = False
+
+
+def external_slot():
+ pass
+
+
+class Leaker:
+ def __init__(self, slot):
+ widget = QWidget()
+ widget.windowIconChanged.connect(slot)
+
+
+class LeakerLambda(Leaker):
+ def __init__(self):
+ super().__init__(lambda *args: None)
+
+
+class LeakerFunctoolsPartial(Leaker):
+ def __init__(self):
+ super().__init__(partial(int, 0))
+
+
+class LeakerExternal(Leaker):
+ def __init__(self):
+ super().__init__(external_slot)
+
+
+class TestBugPYSIDE2299(UsesQApplication):
+ def leak(self, leaker):
+ refs_before = sys.gettotalrefcount()
+ for _ in range(1000):
+ leaker()
+ refs_after = sys.gettotalrefcount()
+ self.assertNotAlmostEqual(refs_after - refs_before, 0, delta=10)
+
+ @unittest.skipUnless(have_debug, "You need a debug build")
+ def test_lambda(self):
+ self.leak(LeakerLambda)
+
+ @unittest.skipUnless(have_debug, "You need a debug build")
+ def test_external(self):
+ self.leak(LeakerExternal)
+
+ @unittest.skipUnless(have_debug, "You need a debug build")
+ def test_functools_partial(self):
+ self.leak(LeakerFunctoolsPartial)
+
+
+if __name__ == '__main__':
+ unittest.main()