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"); diff --git a/src/PythonQtSignalReceiver.cpp b/src/PythonQtSignalReceiver.cpp index fd1814fd4..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; @@ -176,7 +178,8 @@ PythonQtSignalReceiver::PythonQtSignalReceiver(QObject* obj) // force decorator/enum creation _objClassInfo->decorator(); - _slotCount = staticMetaObject.methodOffset(); + _nextSlotID = staticMetaObject.methodOffset(); + _nextTargetIndex = 0; } PythonQtSignalReceiver::~PythonQtSignalReceiver() @@ -189,20 +192,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 +274,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,19 +304,22 @@ 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) { - 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 a8ca3f774..7cf646b22 100644 --- a/src/PythonQtSignalReceiver.h +++ b/src/PythonQtSignalReceiver.h @@ -135,9 +135,10 @@ 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 + // Targets are sorted by slot ID. QList _targets; static int _destroyedSignal1Id;