From 311f3f4406cad4974663b90bc704619721be1d2f Mon Sep 17 00:00:00 2001 From: Uwe Siems Date: Thu, 20 Aug 2026 16:45:37 +0200 Subject: [PATCH 1/3] Do not let slot IDs for signal receivers get greater than 32767 as this seems to be the limit imposed by Qt. If we reach that number, wrap around and search for free slot ID. Also always call the callables on the thread that emitted the signal, and fix a potential thread safety issue. Fixes #362 --- src/PythonQtSignalReceiver.cpp | 56 ++++++++++++++++++++++++++++------ src/PythonQtSignalReceiver.h | 6 ++-- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/PythonQtSignalReceiver.cpp b/src/PythonQtSignalReceiver.cpp index fd1814fd4..904bd4ceb 100644 --- a/src/PythonQtSignalReceiver.cpp +++ b/src/PythonQtSignalReceiver.cpp @@ -176,7 +176,8 @@ PythonQtSignalReceiver::PythonQtSignalReceiver(QObject* obj) // force decorator/enum creation _objClassInfo->decorator(); - _slotCount = staticMetaObject.methodOffset(); + _nextSlotID = staticMetaObject.methodOffset(); + _nextTargetIndex = 0; } PythonQtSignalReceiver::~PythonQtSignalReceiver() @@ -189,20 +190,49 @@ PythonQtSignalReceiver::~PythonQtSignalReceiver() } } +#define MAX_SLOT_ID 32768 + bool PythonQtSignalReceiver::addSignalHandler(const char* signal, PyObject* callable) { bool flag = false; int sigId = getSignalIndex(signal); if (sigId >= 0) { + // find next free slot ID: + const int firstIndex = staticMetaObject.methodOffset(); + if (firstIndex + _targets.size() >= MAX_SLOT_ID) { + // no free slot anymore + std::cerr << "Too many callables connected to signals of object " << _obj << std::endl; + return false; + } + // this loop is only triggered in case _nextTargetIndex was reset by removeSignalHandler: + while (_nextTargetIndex < _targets.size() && _targets[_nextTargetIndex].slotId() < _nextSlotID) { + _nextTargetIndex++; + } + // find a free slot ID (and the position in the target list) + while (_nextTargetIndex < _targets.size() && _targets[_nextTargetIndex].slotId() == _nextSlotID) { + _nextTargetIndex++; + _nextSlotID++; + if (_nextSlotID == MAX_SLOT_ID) { + // wrap around + _nextSlotID = firstIndex; + _nextTargetIndex = 0; + } + } // create PythonQtMethodInfo from signal QMetaMethod meta = _obj->metaObject()->method(sigId); const PythonQtMethodInfo* signalInfo = PythonQtMethodInfo::getCachedMethodInfo(meta, _objClassInfo); - PythonQtSignalTarget t(sigId, signalInfo, _slotCount, callable); - _targets.append(t); + PythonQtSignalTarget t(sigId, signalInfo, _nextSlotID, callable); + _targets.insert(_nextTargetIndex, t); // now connect to ourselves with the new slot id - QMetaObject::connect(_obj, sigId, this, _slotCount, Qt::AutoConnection, nullptr); + QMetaObject::connect(_obj, sigId, this, _nextSlotID, Qt::DirectConnection, nullptr); - _slotCount++; + _nextTargetIndex++; + _nextSlotID++; + if (_nextSlotID == MAX_SLOT_ID) { + // wrap around + _nextSlotID = firstIndex; + _nextTargetIndex = 0; + } flag = true; if (sigId == _destroyedSignal1Id || sigId == _destroyedSignal2Id) { @@ -242,11 +272,14 @@ bool PythonQtSignalReceiver::removeSignalHandler(const char* signal, PyObject* c } } } - if ((foundCount > 0) && ((sigId == _destroyedSignal1Id) || (sigId == _destroyedSignal2Id))) { - _destroyedSignalCount -= foundCount; - if (_destroyedSignalCount == 0) { - // make ourself child of QObject again, to get deleted when the object gets deleted - this->setParent(_obj); + if (foundCount > 0) { + _nextTargetIndex = 0; //< must find _nextTargetIndex anew on next addSignalHandler call + if ((sigId == _destroyedSignal1Id) || (sigId == _destroyedSignal2Id)) { + _destroyedSignalCount -= foundCount; + if (_destroyedSignalCount == 0) { + // make ourself child of QObject again, to get deleted when the object gets deleted + this->setParent(_obj); + } } } return foundCount > 0; @@ -269,6 +302,9 @@ int PythonQtSignalReceiver::qt_metacall(QMetaObject::Call c, int id, void** argu QObject::qt_metacall(c, id, arguments); } + // Get Global Interpreter Lock, as a safeguard against cases when an signal is emitted from a thread + // while _targets is modified because a connect/disconnect is done from Python code (which would also hold the GIL) + PYTHONQT_GIL_SCOPE bool shouldDelete = false; for (const PythonQtSignalTarget& t : qAsConst(_targets)) { if (t.slotId() == id) { diff --git a/src/PythonQtSignalReceiver.h b/src/PythonQtSignalReceiver.h index a8ca3f774..112b6c99e 100644 --- a/src/PythonQtSignalReceiver.h +++ b/src/PythonQtSignalReceiver.h @@ -135,9 +135,11 @@ class PythonQtSignalReceiver : public PythonQtSignalReceiverBase QObject* _obj; PythonQtClassInfo* _objClassInfo; - int _slotCount; + int _nextSlotID; + int _nextTargetIndex; int _destroyedSignalCount; - // linear list may get slow on multiple targets, but I think typically we have many objects and just a few signals + // Linear list may get slow on multiple targets, but I think typically we have many objects and just a few signals. + // Targets are sorted by slot ID. QList _targets; static int _destroyedSignal1Id; From 88c313a597401c159b90b8cd6a219b843b19ef4b Mon Sep 17 00:00:00 2001 From: Uwe Siems Date: Thu, 20 Aug 2026 16:56:48 +0200 Subject: [PATCH 2/3] Make use of the fact that targets are sorted by slot ID --- src/PythonQtSignalReceiver.cpp | 24 +++++++++++++----------- src/PythonQtSignalReceiver.h | 1 - 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/PythonQtSignalReceiver.cpp b/src/PythonQtSignalReceiver.cpp index 904bd4ceb..52b14f772 100644 --- a/src/PythonQtSignalReceiver.cpp +++ b/src/PythonQtSignalReceiver.cpp @@ -46,6 +46,8 @@ #include #include +#include + // use -2 to signal that the variable is uninitialized int PythonQtSignalReceiver::_destroyedSignal1Id = -2; int PythonQtSignalReceiver::_destroyedSignal2Id = -2; @@ -306,18 +308,18 @@ int PythonQtSignalReceiver::qt_metacall(QMetaObject::Call c, int id, void** argu // while _targets is modified because a connect/disconnect is done from Python code (which would also hold the GIL) PYTHONQT_GIL_SCOPE bool shouldDelete = false; - for (const PythonQtSignalTarget& t : qAsConst(_targets)) { - if (t.slotId() == id) { - const int sigId = t.signalId(); - t.call(arguments); - // if the signal is the last destroyed signal, we delete ourselves - if ((sigId == _destroyedSignal1Id) || (sigId == _destroyedSignal2Id)) { - _destroyedSignalCount--; - if (_destroyedSignalCount == 0) { - shouldDelete = true; - } + auto it = std::lower_bound(_targets.begin(), _targets.end(), id, + [](const PythonQtSignalTarget& t, int id) -> bool { return t.slotId() < id; }); + if (it != _targets.end() && it->slotId() == id) { + const PythonQtSignalTarget& t = *it; + const int sigId = t.signalId(); + t.call(arguments); + // if the signal is the last destroyed signal, we delete ourselves + if ((sigId == _destroyedSignal1Id) || (sigId == _destroyedSignal2Id)) { + _destroyedSignalCount--; + if (_destroyedSignalCount == 0) { + shouldDelete = true; } - break; } } if (shouldDelete) { diff --git a/src/PythonQtSignalReceiver.h b/src/PythonQtSignalReceiver.h index 112b6c99e..7cf646b22 100644 --- a/src/PythonQtSignalReceiver.h +++ b/src/PythonQtSignalReceiver.h @@ -138,7 +138,6 @@ class PythonQtSignalReceiver : public PythonQtSignalReceiverBase int _nextSlotID; int _nextTargetIndex; int _destroyedSignalCount; - // Linear list may get slow on multiple targets, but I think typically we have many objects and just a few signals. // Targets are sorted by slot ID. QList _targets; From 100cb30fa5a019f9690a29d81e33e888e1fe19cd Mon Sep 17 00:00:00 2001 From: Uwe Siems Date: Fri, 21 Aug 2026 12:33:49 +0200 Subject: [PATCH 3/3] Do a regular signal/slot connect if the argument is a slot object, or a signal/signal connect, if the argument is a signal object. Only if the argument is a regular callable create a new receiver object. This will change the behavior of connects with regard to threads in some cases! Fixed #363 [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/PythonQtSignal.cpp | 77 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 69 insertions(+), 8 deletions(-) diff --git a/src/PythonQtSignal.cpp b/src/PythonQtSignal.cpp index 402b47338..8b95ce651 100644 --- a/src/PythonQtSignal.cpp +++ b/src/PythonQtSignal.cpp @@ -233,18 +233,71 @@ static PyObject* PythonQtSignalFunction_typeName(PythonQtSignalFunctionObject* t return PythonQtMemberFunction_typeName(type->m_ml); } +// Find out if we can connect directly to the given Python object, without creating a separate receiver. +// If yes, provide target object and signature for the connect call. +static bool extractSignalTarget(PyObject* object, QObject*& targetObj, QByteArray& targetSignature) +{ + static PyObject* qtSlots = PyUnicode_FromString("_qtSlots"); + if (PyObject_TypeCheck(object, &PythonQtSignalFunction_Type)) { + PythonQtSignalFunctionObject* type = (PythonQtSignalFunctionObject*)object; + PythonQtInstanceWrapper* self = (PythonQtInstanceWrapper*)type->m_self; + if (self->_obj) { + // connecting to another signal + targetObj = self->_obj; + targetSignature = QByteArray("2") + type->m_ml->signature(); + return true; + } + } else if (PyMethod_Check(object)) { + PyObject* instance = PyMethod_Self(object); + if (PyObject_TypeCheck(instance, &PythonQtInstanceWrapper_Type)) { + PythonQtInstanceWrapper* typedInstance = (PythonQtInstanceWrapper*)instance; + if (!typedInstance->_wrappedPtr) { + // It's a QObject-derived class + targetObj = typedInstance->_obj; + PyObject* function = PyMethod_Function(object); + if (PyObject_HasAttr(function, qtSlots)) { + // connecting to a slot + PyObject* signatures = PyObject_GetAttr(function, qtSlots); + Py_ssize_t count = PyList_Size(signatures); + // TODO: Find the best matching signature; + // currently we only connect to the actual slot if only one slot signature is associated + // with this callable - if there are more, we just connect to the callable and let + // the callable figure the arguments out - but in this case Qt::DirectConnection is + // used instead of Qt::AutoConnection, which can be suprising if threads are involved. + if (count == 1) { + PyObject* signature = PyList_GET_ITEM(signatures, 0); + // Retrieve slot signature + QByteArray sig = PyUnicode_AsUTF8(signature); + targetSignature = QByteArray("1") + sig.split(' ')[1]; // include slot prefix + return true; + } + } + } + } + } + return false; +} + static PyObject* PythonQtSignalFunction_connect(PythonQtSignalFunctionObject* type, PyObject* args) { if (PyObject_TypeCheck(type->m_self, &PythonQtInstanceWrapper_Type)) { PythonQtInstanceWrapper* self = (PythonQtInstanceWrapper*)type->m_self; if (self->_obj) { Py_ssize_t argc = PyTuple_Size(args); + QByteArray sourceSignature = QByteArray("2") + type->m_ml->signature(); if (argc == 1) { // connect with Python callable PyObject* callable = PyTuple_GET_ITEM(args, 0); - bool result = - PythonQt::self()->addSignalHandler(self->_obj, QByteArray("2") + type->m_ml->signature(), callable); - return PythonQtConv::GetPyBool(result); + QObject* targetObj; + QByteArray targetSignature; + if (extractSignalTarget(callable, targetObj, targetSignature)) { + // Do a regular signal/slot (or signal/signal) connect. + QObject::connect(self->_obj, sourceSignature, targetObj, targetSignature, Qt::AutoConnection); + return PythonQtConv::GetPyBool(true); + } else { + bool result = PythonQt::self()->addSignalHandler(self->_obj, sourceSignature, callable); + return PythonQtConv::GetPyBool(result); + } } else { PyErr_SetString(PyExc_ValueError, "Called connect with wrong number of arguments"); } @@ -259,15 +312,23 @@ static PyObject* PythonQtSignalFunction_disconnect(PythonQtSignalFunctionObject* PythonQtInstanceWrapper* self = (PythonQtInstanceWrapper*)type->m_self; if (self->_obj) { Py_ssize_t argc = PyTuple_Size(args); - QByteArray signal = QByteArray("2") + type->m_ml->signature(); + QByteArray sourceSignature = QByteArray("2") + type->m_ml->signature(); if (argc == 1) { // disconnect with Python callable PyObject* callable = PyTuple_GET_ITEM(args, 0); - bool result = PythonQt::self()->removeSignalHandler(self->_obj, signal, callable); - return PythonQtConv::GetPyBool(result); + QObject* targetObj; + QByteArray targetSignature; + if (extractSignalTarget(callable, targetObj, targetSignature)) { + // Do a regular signal/slot (or signal/signal) disconnect. + QObject::disconnect(self->_obj, sourceSignature, targetObj, targetSignature); + return PythonQtConv::GetPyBool(true); + } else { + bool result = PythonQt::self()->removeSignalHandler(self->_obj, sourceSignature, callable); + return PythonQtConv::GetPyBool(result); + } } else if (argc == 0) { - bool result = PythonQt::self()->removeSignalHandler(self->_obj, signal, nullptr); - result |= QObject::disconnect(self->_obj, signal, nullptr, nullptr); + bool result = PythonQt::self()->removeSignalHandler(self->_obj, sourceSignature, nullptr); + result |= QObject::disconnect(self->_obj, sourceSignature, nullptr, nullptr); return PythonQtConv::GetPyBool(result); } else { PyErr_SetString(PyExc_ValueError, "Called disconnect with wrong number of arguments");