diff options
author | Adrian Herrmann <adrian.herrmann@qt.io> | 2023-05-10 15:54:54 +0200 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2023-05-11 15:38:03 +0000 |
commit | 58ec2ac3d01a8f01a951699ae7b1d2a64ffef845 (patch) | |
tree | 3b14d3fd42f2d999dc5ff10d03de98ec089817d0 | |
parent | 32a097c74d8318f2f14e2caab752e9d9b691df25 (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.cpp | 6 | ||||
-rw-r--r-- | sources/pyside6/tests/signals/anonymous_slot_leak_test.py | 72 |
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() |