diff options
author | Christian Tismer <tismer@stackless.com> | 2023-02-01 18:46:11 +0100 |
---|---|---|
committer | Christian Tismer <tismer@stackless.com> | 2023-02-07 15:17:16 +0100 |
commit | 08ec50ff3b569e2062285ee633f5c7e548c67545 (patch) | |
tree | 7b8dfc5e578e9b062492aa9cb909cdd49441eb97 | |
parent | ffc97efcd23fb6ae72ea2d864a1d82c16646c543 (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>
-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 d3455dd60..b3387f700 100644 --- a/build_history/blacklist.txt +++ b/build_history/blacklist.txt @@ -42,8 +42,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 a60b7131a..aa6dcc9a9 100644 --- a/sources/pyside6/libpyside/pyside.cpp +++ b/sources/pyside6/libpyside/pyside.cpp @@ -494,6 +494,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; @@ -506,15 +507,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 cdc3042c0..f7c9073c2 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) @@ -1032,10 +1042,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 d81160eea..56873b197 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); |