From f6854782a4ce3beee478170362e69a1835558c93 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 20 Aug 2015 14:57:35 +0200 Subject: [PATCH] Debug/Lock cleanup --- libraries/networking/src/PacketReceiver.cpp | 92 ++++++++++----------- 1 file changed, 44 insertions(+), 48 deletions(-) diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 7a9a3b1ddd..a086949ac8 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -65,45 +65,49 @@ bool PacketReceiver::registerListenerForTypes(PacketTypeList types, QObject* lis } void PacketReceiver::registerDirectListener(PacketType type, QObject* listener, const char* slot) { + Q_ASSERT_X(listener, "PacketReceiver::registerDirectListener", "No object to register"); + Q_ASSERT_X(slot, "PacketReceiver::registerDirectListener", "No slot to register"); + bool success = registerListener(type, listener, slot); if (success) { - _directConnectSetMutex.lock(); + QMutexLocker locker(&_directConnectSetMutex); // if we successfully registered, add this object to the set of objects that are directly connected _directlyConnectedObjects.insert(listener); - - _directConnectSetMutex.unlock(); } } void PacketReceiver::registerDirectListenerForTypes(PacketTypeList types, QObject* listener, const char* slot) { + Q_ASSERT_X(listener, "PacketReceiver::registerDirectListenerForTypes", "No object to register"); + Q_ASSERT_X(slot, "PacketReceiver::registerDirectListenerForTypes", "No slot to register"); + // just call register listener for types to start bool success = registerListenerForTypes(std::move(types), listener, slot); if (success) { - _directConnectSetMutex.lock(); + QMutexLocker locker(&_directConnectSetMutex); // if we successfully registered, add this object to the set of objects that are directly connected _directlyConnectedObjects.insert(listener); - - _directConnectSetMutex.unlock(); } } bool PacketReceiver::registerMessageListener(PacketType type, QObject* listener, const char* slot) { + Q_ASSERT_X(listener, "PacketReceiver::registerMessageListener", "No object to register"); + Q_ASSERT_X(slot, "PacketReceiver::registerMessageListener", "No slot to register"); + QMetaMethod matchingMethod = matchingMethodForListener(type, listener, slot); if (matchingMethod.isValid()) { QMutexLocker locker(&_packetListenerLock); if (_packetListListenerMap.contains(type)) { - qCDebug(networking) << "Warning: Registering a packet listener for packet type" << type + qCWarning(networking) << "Registering a packet listener for packet type" << type << "that will remove a previously registered listener"; } // add the mapping _packetListListenerMap[type] = ObjectMethodPair(QPointer(listener), matchingMethod); - return true; } else { return false; @@ -111,7 +115,8 @@ bool PacketReceiver::registerMessageListener(PacketType type, QObject* listener, } bool PacketReceiver::registerListener(PacketType type, QObject* listener, const char* slot) { - Q_ASSERT(listener); + Q_ASSERT_X(listener, "PacketReceiver::registerListener", "No object to register"); + Q_ASSERT_X(slot, "PacketReceiver::registerListener", "No slot to register"); QMetaMethod matchingMethod = matchingMethodForListener(type, listener, slot); @@ -124,16 +129,18 @@ bool PacketReceiver::registerListener(PacketType type, QObject* listener, const } QMetaMethod PacketReceiver::matchingMethodForListener(PacketType type, QObject* object, const char* slot) const { - Q_ASSERT(object); + Q_ASSERT_X(object, "PacketReceiver::matchingMethodForListener", "No object to call"); + Q_ASSERT_X(slot, "PacketReceiver::matchingMethodForListener", "No slot to call"); // normalize the slot with the expected parameters - + + static const QString SIGNATURE_TEMPLATE("%1(%2)"); static const QString NON_SOURCED_PACKET_LISTENER_PARAMETERS = "QSharedPointer"; static const QString NON_SOURCED_PACKETLIST_LISTENER_PARAMETERS = "QSharedPointer"; QSet possibleSignatures { - QString("%1(%2)").arg(slot).arg(NON_SOURCED_PACKET_LISTENER_PARAMETERS), - QString("%1(%2)").arg(slot).arg(NON_SOURCED_PACKETLIST_LISTENER_PARAMETERS) + SIGNATURE_TEMPLATE.arg(slot, NON_SOURCED_PACKET_LISTENER_PARAMETERS), + SIGNATURE_TEMPLATE.arg(slot, NON_SOURCED_PACKETLIST_LISTENER_PARAMETERS) }; if (!NON_SOURCED_PACKETS.contains(type)) { @@ -145,10 +152,10 @@ QMetaMethod PacketReceiver::matchingMethodForListener(PacketType type, QObject* // a sourced packet must take the shared pointer to the packet but optionally could include // a shared pointer to the node - possibleSignatures << QString("%1(%2)").arg(slot).arg(TYPEDEF_SOURCED_PACKET_LISTENER_PARAMETERS); - possibleSignatures << QString("%1(%2)").arg(slot).arg(SOURCED_PACKET_LISTENER_PARAMETERS); - possibleSignatures << QString("%1(%2)").arg(slot).arg(TYPEDEF_SOURCED_PACKETLIST_LISTENER_PARAMETERS); - possibleSignatures << QString("%1(%2)").arg(slot).arg(SOURCED_PACKETLIST_LISTENER_PARAMETERS); + possibleSignatures << SIGNATURE_TEMPLATE.arg(slot, TYPEDEF_SOURCED_PACKET_LISTENER_PARAMETERS); + possibleSignatures << SIGNATURE_TEMPLATE.arg(slot, SOURCED_PACKET_LISTENER_PARAMETERS); + possibleSignatures << SIGNATURE_TEMPLATE.arg(slot, TYPEDEF_SOURCED_PACKETLIST_LISTENER_PARAMETERS); + possibleSignatures << SIGNATURE_TEMPLATE.arg(slot, SOURCED_PACKETLIST_LISTENER_PARAMETERS); } int methodIndex = -1; @@ -184,39 +191,30 @@ QMetaMethod PacketReceiver::matchingMethodForListener(PacketType type, QObject* } void PacketReceiver::registerVerifiedListener(PacketType type, QObject* object, const QMetaMethod& slot) { - _packetListenerLock.lock(); + Q_ASSERT_X(object, "PacketReceiver::registerVerifiedListener", "No object to register"); + QMutexLocker locker(&_packetListenerLock); if (_packetListenerMap.contains(type)) { - qCDebug(networking) << "Warning: Registering a packet listener for packet type" << type + qCWarning(networking) << "Registering a packet listener for packet type" << type << "that will remove a previously registered listener"; } // add the mapping _packetListenerMap[type] = ObjectMethodPair(QPointer(object), slot); - - _packetListenerLock.unlock(); - } void PacketReceiver::unregisterListener(QObject* listener) { - _packetListenerLock.lock(); - - auto it = _packetListenerMap.begin(); - - while (it != _packetListenerMap.end()) { - if (it.value().first == listener) { - // this listener matches - erase it - it = _packetListenerMap.erase(it); - } else { - ++it; - } - } - - _packetListenerLock.unlock(); + Q_ASSERT_X(listener, "PacketReceiver::unregisterListener", "No listener to unregister"); - _directConnectSetMutex.lock(); + QMutexLocker packetListenerLocker(&_packetListenerLock); + std::remove_if(std::begin(_packetListenerMap), std::end(_packetListenerMap), + [&listener](const ObjectMethodPair& pair) { + return pair.first == listener; + }); + packetListenerLocker.unlock(); + + QMutexLocker directConnectSetLocker(&_directConnectSetMutex); _directlyConnectedObjects.remove(listener); - _directConnectSetMutex.unlock(); } void PacketReceiver::handleVerifiedPacketList(std::unique_ptr packetList) { @@ -337,7 +335,7 @@ void PacketReceiver::handleVerifiedPacketList(std::unique_ptr p } } else if (it == _packetListListenerMap.end()) { - qWarning() << "No listener found for packet type" << nlPacketList->getType(); + qCWarning(networking) << "No listener found for packet type" << nlPacketList->getType(); // insert a dummy listener so we don't print this again _packetListListenerMap.insert(nlPacketList->getType(), { nullptr, QMetaMethod() }); @@ -371,7 +369,7 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { } } - _packetListenerLock.lock(); + QMutexLocker packetListenerLocker(&_packetListenerLock); bool listenerIsDead = false; @@ -386,12 +384,10 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { bool success = false; // check if this is a directly connected listener - _directConnectSetMutex.lock(); - + QMutexLocker directConnectSetLocker(&_directConnectSetMutex); Qt::ConnectionType connectionType = _directlyConnectedObjects.contains(listener.first) ? Qt::DirectConnection : Qt::AutoConnection; - - _directConnectSetMutex.unlock(); + directConnectSetLocker.unlock(); PacketType packetType = nlPacket->getType(); @@ -456,18 +452,18 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { it = _packetListenerMap.erase(it); // if it exists, remove the listener from _directlyConnectedObjects - _directConnectSetMutex.lock(); + QMutexLocker locker(&_directConnectSetMutex); _directlyConnectedObjects.remove(listener.first); - _directConnectSetMutex.unlock(); + locker.unlock(); } } else if (it == _packetListenerMap.end()) { - qWarning() << "No listener found for packet type" << nlPacket->getType(); + qCWarning(networking) << "No listener found for packet type" << nlPacket->getType(); // insert a dummy listener so we don't print this again _packetListenerMap.insert(nlPacket->getType(), { nullptr, QMetaMethod() }); } - _packetListenerLock.unlock(); + packetListenerLocker.unlock(); }