diff options
| author | Christian Tismer <tismer@stackless.com> | 2023-01-25 19:06:12 +0100 |
|---|---|---|
| committer | Christian Tismer <tismer@stackless.com> | 2023-02-02 09:29:00 +0100 |
| commit | db40e3e07932576bc54cd922eecd423c0f675613 (patch) | |
| tree | 28e9dec9e104dcefc4052fb67970b5ce7ca925d7 | |
| parent | f799f1dd678cd17b59436a0c3a734135841eaa10 (diff) | |
signals: Introduce Weak Reference to protect lost SignalInstance
This first solution detects the vanishing object and raises
an exception.
Trying to revive the object was unsuccessful. Since this happens
only when no signals are connected, it is ok to leave it this way.
Change-Id: Ib47f85657363b675cec3616b5550ce41d93fb4d3
Fixes: PYSIDE-2201
Pick-to: 6.4
Task-number: PYSIDE-79
Task-number: PYSIDE-68
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
| -rw-r--r-- | sources/pyside6/libpyside/pysidesignal.cpp | 21 | ||||
| -rw-r--r-- | sources/pyside6/libpyside/pysidesignal.h | 1 | ||||
| -rw-r--r-- | sources/pyside6/tests/QtCore/destroysignal_test.py | 27 |
3 files changed, 48 insertions, 1 deletions
diff --git a/sources/pyside6/libpyside/pysidesignal.cpp b/sources/pyside6/libpyside/pysidesignal.cpp index 5df42261c..cdc3042c0 100644 --- a/sources/pyside6/libpyside/pysidesignal.cpp +++ b/sources/pyside6/libpyside/pysidesignal.cpp @@ -6,6 +6,7 @@ #include "pysidesignal_p.h" #include "pysideutils.h" #include "pysidestaticstrings.h" +#include "pysideweakref.h" #include "signalmanager.h" #include <shiboken.h> @@ -342,6 +343,7 @@ static void signalInstanceFree(void *vself) } delete dataPvt; self->d = nullptr; + self->deleted = true; Py_TYPE(pySelf)->tp_base->tp_free(self); } @@ -550,6 +552,13 @@ static PyObject *signalInstanceEmit(PyObject *self, PyObject *args) { PySideSignalInstance *source = reinterpret_cast<PySideSignalInstance *>(self); + // PYSIDE-2201: Check if the object has vanished meanwhile. + // Tried to revive it without exception, but this gives problems. + if (source->deleted) { + PyErr_Format(PyExc_RuntimeError, "The SignalInstance object was already deleted"); + return nullptr; + } + Shiboken::AutoDecRef pyArgs(PyList_New(0)); int numArgsGiven = PySequence_Fast_GET_SIZE(args); int numArgsInSignature = argCountInSignature(source->d->signature); @@ -932,9 +941,16 @@ static void appendSignature(PySideSignal *self, const SignalSignature &signature self->data->signatures.append({signature.m_parameterTypes, signature.m_attributes}); } +static void sourceGone(void *data) +{ + auto *self = reinterpret_cast<PySideSignalInstance *>(data); + self->deleted = true; +} + static void instanceInitialize(PySideSignalInstance *self, PyObject *name, PySideSignal *signal, PyObject *source, int index) { self->d = new PySideSignalInstancePrivate; + self->deleted = false; PySideSignalInstancePrivate *selfPvt = self->d; selfPvt->next = nullptr; if (signal->data->signalName.isEmpty()) @@ -950,6 +966,10 @@ static void instanceInitialize(PySideSignalInstance *self, PyObject *name, PySid selfPvt->homonymousMethod = signal->homonymousMethod; Py_INCREF(selfPvt->homonymousMethod); } + // PYSIDE-2201: We have no reference to source. Let's take a weakref to get + // notified when source gets deleted. + PySide::WeakRef::create(source, sourceGone, self); + index++; if (index < signal->data->signatures.size()) { @@ -1009,6 +1029,7 @@ PySideSignalInstance *newObjectFromMethod(PyObject *source, const QList<QMetaMet previous->d->next = item; item->d = new PySideSignalInstancePrivate; + item->deleted = false; PySideSignalInstancePrivate *selfPvt = item->d; selfPvt->source = source; Py_INCREF(selfPvt->source); // PYSIDE-79: an INCREF is missing. diff --git a/sources/pyside6/libpyside/pysidesignal.h b/sources/pyside6/libpyside/pysidesignal.h index c38371d81..3b82e6c58 100644 --- a/sources/pyside6/libpyside/pysidesignal.h +++ b/sources/pyside6/libpyside/pysidesignal.h @@ -30,6 +30,7 @@ extern "C" { PyObject_HEAD PySideSignalInstancePrivate *d; + bool deleted; }; }; // extern "C" diff --git a/sources/pyside6/tests/QtCore/destroysignal_test.py b/sources/pyside6/tests/QtCore/destroysignal_test.py index 34aaded9e..f1d7cfec1 100644 --- a/sources/pyside6/tests/QtCore/destroysignal_test.py +++ b/sources/pyside6/tests/QtCore/destroysignal_test.py @@ -11,7 +11,7 @@ sys.path.append(os.fspath(Path(__file__).resolve().parents[1])) from init_paths import init_test_paths init_test_paths(False) -from PySide6.QtCore import QTimer, QObject +from PySide6.QtCore import QTimer, QObject, Signal class TestDestroySignal(unittest.TestCase): @@ -43,6 +43,31 @@ class TestDestroySignal(unittest.TestCase): self.assertTrue(self._destroyed) +class Foo(QObject): + s = Signal(int) + + def __init__(self): + QObject.__init__(self) + sys.stderr.write(f"__init__ {id(self):x}\n") + + def __del__(self): + sys.stderr.write(f"__del__ {id(self):x}\n") + + def send(self, i): + self.s.emit(i) + + +# PYSIDE-2201: This crashed until we introduced a weak reference. +class TestDestroyNoConnect(unittest.TestCase): + + def testSignalDestroyedMissingReference(self): + # This works since it has one reference more to Foo + Foo().send(43) + # This crashed because we have no reference in the signal. + with self.assertRaises(RuntimeError): + Foo().s.emit(44) + + if __name__ == '__main__': unittest.main() |