diff options
author | Christian Tismer <tismer@stackless.com> | 2023-02-01 18:46:11 +0100 |
---|---|---|
committer | Christian Tismer <tismer@stackless.com> | 2023-02-21 15:15:29 +0100 |
commit | 9c5b9a97e8289c25c0cc06e33e3a9f6f1042c794 (patch) | |
tree | 79d3b65970ae5f210889d6a213d5d90680f14762 | |
parent | bd2fc0d2c751752562fdbc4dfe269f928dd1ae93 (diff) |
signal: Finally clean up all leaks after complete understanding
The PYSIDE-79 bug was never understood, completely.
After getting much more clarity through the work on
PYSIDE-2201, the real problem could be found:
It turned out that the implementation of descriptors was
incomplete. Instead of respecting an already cached instance,
it created a new one, all the time, resulting in a serious leak!
This became visible by deep inspection of the control flow
with VS-Code. After the interaction of PyObject_GetAttr,
getHiddenDataFromQObject and signalDescrGet became transparent,
the function was completed and the issue finally solved.
Fixes: PYSIDE-79
Task-number: PYSIDE-68
Task-number: PYSIDE-2201
Change-Id: Ifd6098b1ce43d9bf51350218deb031bbf9ccb97a
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
(cherry picked from commit 08ec50ff3b569e2062285ee633f5c7e548c67545)
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
-rw-r--r-- | build_history/blacklist.txt | 2 | ||||
-rw-r--r-- | sources/pyside6/libpyside/pyside.cpp | 10 | ||||
-rw-r--r-- | sources/pyside6/libpyside/pysidesignal.cpp | 15 | ||||
-rw-r--r-- | sources/shiboken6/libshiboken/basewrapper.cpp | 1 |
4 files changed, 14 insertions, 14 deletions
diff --git a/build_history/blacklist.txt b/build_history/blacklist.txt index c335081a1..a2a45295c 100644 --- a/build_history/blacklist.txt +++ b/build_history/blacklist.txt @@ -45,8 +45,6 @@ # Unsolved Refcounting leaks in debug mode [pysidetest::property_python_test] debug -[signals::bug_79] - debug # PYSIDE-535: These errors are still present. Please try to remove one :) [sample::mixed_mi] diff --git a/sources/pyside6/libpyside/pyside.cpp b/sources/pyside6/libpyside/pyside.cpp index 29dd8d7e8..88a86fb2d 100644 --- a/sources/pyside6/libpyside/pyside.cpp +++ b/sources/pyside6/libpyside/pyside.cpp @@ -496,6 +496,7 @@ PyObject *getHiddenDataFromQObject(QObject *cppSelf, PyObject *self, PyObject *n { using Shiboken::AutoDecRef; + // PYSIDE-68-bis: This getattr finds signals early by `signalDescrGet`. PyObject *attr = PyObject_GenericGetAttr(self, name); if (!Shiboken::Object::isValid(reinterpret_cast<SbkObject *>(self), false)) return attr; @@ -508,15 +509,6 @@ PyObject *getHiddenDataFromQObject(QObject *cppSelf, PyObject *self, PyObject *n attr = value; } - // Mutate native signals to signal instance type - // Caution: This inserts the signal instance into the instance dict. - if (attr && PyObject_TypeCheck(attr, PySideSignal_TypeF())) { - auto *inst = Signal::initialize(reinterpret_cast<PySideSignal *>(attr), name, self); - PyObject *signalInst = reinterpret_cast<PyObject *>(inst); - PyObject_SetAttr(self, name, signalInst); - return signalInst; - } - // Search on metaobject (avoid internal attributes started with '__') if (!attr) { PyObject *type, *value, *traceback; diff --git a/sources/pyside6/libpyside/pysidesignal.cpp b/sources/pyside6/libpyside/pysidesignal.cpp index ebc7b873d..9f881d893 100644 --- a/sources/pyside6/libpyside/pysidesignal.cpp +++ b/sources/pyside6/libpyside/pysidesignal.cpp @@ -683,8 +683,18 @@ static PyObject *signalDescrGet(PyObject *self, PyObject *obj, PyObject * /*type Py_INCREF(self); return self; } + + // PYSIDE-68-bis: It is important to respect the already cached instance. Shiboken::AutoDecRef name(Py_BuildValue("s", signal->data->signalName.data())); - return reinterpret_cast<PyObject *>(PySide::Signal::initialize(signal, name, obj)); + auto *dict = SbkObject_GetDict_NoRef(obj); + auto *inst = PyDict_GetItem(dict, name); + if (inst) { + Py_INCREF(inst); + return inst; + } + inst = reinterpret_cast<PyObject *>(PySide::Signal::initialize(signal, name, obj)); + PyObject_SetAttr(obj, name, inst); + return inst; } static PyObject *signalCall(PyObject *self, PyObject *args, PyObject *kw) @@ -1030,10 +1040,9 @@ PySideSignalInstance *newObjectFromMethod(PyObject *source, const QList<QMetaMet item->deleted = false; PySideSignalInstancePrivate *selfPvt = item->d; selfPvt->source = source; - Py_INCREF(selfPvt->source); // PYSIDE-79: an INCREF is missing. QByteArray cppName(m.methodSignature()); cppName.truncate(cppName.indexOf('(')); - // separe SignalName + // separate SignalName selfPvt->signalName = cppName; selfPvt->signature = m.methodSignature(); selfPvt->attributes = m.attributes(); diff --git a/sources/shiboken6/libshiboken/basewrapper.cpp b/sources/shiboken6/libshiboken/basewrapper.cpp index 24980239d..09d320560 100644 --- a/sources/shiboken6/libshiboken/basewrapper.cpp +++ b/sources/shiboken6/libshiboken/basewrapper.cpp @@ -71,6 +71,7 @@ void setDestroyQApplication(DestroyQAppHook func) // PYSIDE-535: Use the C API in PyPy instead of `op->ob_dict`, directly LIBSHIBOKEN_API PyObject *SbkObject_GetDict_NoRef(PyObject *op) { + assert(Shiboken::Object::checkType(op)); #ifdef PYPY_VERSION Shiboken::GilState state; auto *ret = PyObject_GenericGetDict(op, nullptr); |