From 3355b71d6b643b644dad9a4c872691ef2730a382 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 17 Jan 2014 15:13:56 -0800 Subject: [PATCH 1/7] switch back to returning iterator from killNodeAtHashIterator --- libraries/shared/src/NodeList.cpp | 13 ++++--------- libraries/shared/src/NodeList.h | 2 +- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/libraries/shared/src/NodeList.cpp b/libraries/shared/src/NodeList.cpp index 06d2bbe05b..d71188892f 100644 --- a/libraries/shared/src/NodeList.cpp +++ b/libraries/shared/src/NodeList.cpp @@ -276,9 +276,7 @@ void NodeList::clear() { // iterate the nodes in the list while (nodeItem != _nodeHash.end()) { - NodeHash::iterator previousNodeItem = nodeItem; - ++nodeItem; - killNodeAtHashIterator(previousNodeItem); + nodeItem = killNodeAtHashIterator(nodeItem); } } @@ -444,10 +442,10 @@ void NodeList::killNodeWithUUID(const QUuid& nodeUUID) { } } -void NodeList::killNodeAtHashIterator(NodeHash::iterator& nodeItemToKill) { +NodeHash::iterator NodeList::killNodeAtHashIterator(NodeHash::iterator& nodeItemToKill) { qDebug() << "Killed" << *nodeItemToKill.value(); emit nodeKilled(nodeItemToKill.value()); - _nodeHash.erase(nodeItemToKill); + return _nodeHash.erase(nodeItemToKill); } void NodeList::sendKillNode(const char* nodeTypes, int numNodeTypes) { @@ -790,10 +788,7 @@ void NodeList::removeSilentNodes() { if ((usecTimestampNow() - node->getLastHeardMicrostamp()) > NODE_SILENCE_THRESHOLD_USECS) { // call our private method to kill this node (removes it and emits the right signal) - NodeHash::iterator previousNodeItem = nodeItem; - ++nodeItem; - - killNodeAtHashIterator(previousNodeItem); + nodeItem = killNodeAtHashIterator(nodeItem); } else { // we didn't kill this node, push the iterator forwards ++nodeItem; diff --git a/libraries/shared/src/NodeList.h b/libraries/shared/src/NodeList.h index 391e329200..b665141b66 100644 --- a/libraries/shared/src/NodeList.h +++ b/libraries/shared/src/NodeList.h @@ -140,7 +140,7 @@ private: void processSTUNResponse(unsigned char* packetData, size_t dataBytes); void processKillNode(unsigned char* packetData, size_t dataBytes); - void killNodeAtHashIterator(NodeHash::iterator& nodeItemToKill); + NodeHash::iterator killNodeAtHashIterator(NodeHash::iterator& nodeItemToKill); NodeHash _nodeHash; QString _domainHostname; From 816b324236a27f375be9e9fe6b928b6cf41b0fc4 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 17 Jan 2014 15:16:59 -0800 Subject: [PATCH 2/7] switch to explicit lock and unlock in removeSilentNodes --- libraries/shared/src/NodeList.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/shared/src/NodeList.cpp b/libraries/shared/src/NodeList.cpp index d71188892f..d44953c37d 100644 --- a/libraries/shared/src/NodeList.cpp +++ b/libraries/shared/src/NodeList.cpp @@ -784,7 +784,7 @@ void NodeList::removeSilentNodes() { while (nodeItem != _nodeHash.end()) { SharedNodePointer node = nodeItem.value(); - QMutexLocker(&node->getMutex()); + node->getMutex().lock(); if ((usecTimestampNow() - node->getLastHeardMicrostamp()) > NODE_SILENCE_THRESHOLD_USECS) { // call our private method to kill this node (removes it and emits the right signal) @@ -793,6 +793,8 @@ void NodeList::removeSilentNodes() { // we didn't kill this node, push the iterator forwards ++nodeItem; } + + node->getMutex().unlock(); } } From b24eca199a067041112ad829d5fc244aa32c7858 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 17 Jan 2014 15:23:58 -0800 Subject: [PATCH 3/7] use new signal slot syntax in JurisdictionListener --- libraries/octree/src/JurisdictionListener.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/octree/src/JurisdictionListener.cpp b/libraries/octree/src/JurisdictionListener.cpp index ae27259f83..d9a0a57ef6 100644 --- a/libraries/octree/src/JurisdictionListener.cpp +++ b/libraries/octree/src/JurisdictionListener.cpp @@ -22,7 +22,7 @@ JurisdictionListener::JurisdictionListener(NODE_TYPE type, PacketSenderNotify* n ReceivedPacketProcessor::_dontSleep = true; // we handle sleeping so this class doesn't need to NodeList* nodeList = NodeList::getInstance(); - connect(nodeList, SIGNAL(nodeKilled(SharedNodePointer)), SLOT(nodeKilled(SharedNodePointer))); + connect(nodeList, &NodeList::nodeKilled, this, &JurisdictionListener::nodeKilled); //qDebug("JurisdictionListener::JurisdictionListener(NODE_TYPE type=%c)\n", type); From 9007a15381574a745deee8d687791701b83210ba Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 17 Jan 2014 15:27:44 -0800 Subject: [PATCH 4/7] comment out the JurisdictionListener's connection for now --- libraries/octree/src/JurisdictionListener.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/libraries/octree/src/JurisdictionListener.cpp b/libraries/octree/src/JurisdictionListener.cpp index d9a0a57ef6..d8919ac48e 100644 --- a/libraries/octree/src/JurisdictionListener.cpp +++ b/libraries/octree/src/JurisdictionListener.cpp @@ -20,12 +20,9 @@ JurisdictionListener::JurisdictionListener(NODE_TYPE type, PacketSenderNotify* n { _nodeType = type; ReceivedPacketProcessor::_dontSleep = true; // we handle sleeping so this class doesn't need to - NodeList* nodeList = NodeList::getInstance(); - connect(nodeList, &NodeList::nodeKilled, this, &JurisdictionListener::nodeKilled); - - //qDebug("JurisdictionListener::JurisdictionListener(NODE_TYPE type=%c)\n", type); - +// connect(nodeList, &NodeList::nodeKilled, this, &JurisdictionListener::nodeKilled); +// qDebug("JurisdictionListener::JurisdictionListener(NODE_TYPE type=%c)\n", type); } void JurisdictionListener::nodeKilled(SharedNodePointer node) { From fad7bde0e173ff1505ea1399541ddec2f8a18f46 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 17 Jan 2014 16:02:57 -0800 Subject: [PATCH 5/7] add mutex locking to NodeHash --- libraries/shared/src/NodeList.cpp | 24 +++++++++++++++++++++++- libraries/shared/src/NodeList.h | 1 + 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/libraries/shared/src/NodeList.cpp b/libraries/shared/src/NodeList.cpp index d44953c37d..1813e79e8e 100644 --- a/libraries/shared/src/NodeList.cpp +++ b/libraries/shared/src/NodeList.cpp @@ -56,6 +56,7 @@ NodeList* NodeList::getInstance() { NodeList::NodeList(char newOwnerType, unsigned short int newSocketListenPort) : _nodeHash(), + _nodeHashMutex(), _domainHostname(DEFAULT_DOMAIN_HOSTNAME), _domainSockAddr(HifiSockAddr(QHostAddress::Null, DEFAULT_DOMAIN_SERVER_PORT)), _nodeSocket(), @@ -266,11 +267,16 @@ SharedNodePointer NodeList::nodeWithAddress(const HifiSockAddr &senderSockAddr) } SharedNodePointer NodeList::nodeWithUUID(const QUuid& nodeUUID) { - return _nodeHash.value(nodeUUID); + _nodeHashMutex.lock(); + SharedNodePointer matchingNode = _nodeHash.value(nodeUUID); + _nodeHashMutex.unlock(); + return matchingNode; } void NodeList::clear() { qDebug() << "Clearing the NodeList. Deleting all nodes in list."; + + _nodeHashMutex.lock(); NodeHash::iterator nodeItem = _nodeHash.begin(); @@ -278,6 +284,8 @@ void NodeList::clear() { while (nodeItem != _nodeHash.end()) { nodeItem = killNodeAtHashIterator(nodeItem); } + + _nodeHashMutex.unlock(); } void NodeList::reset() { @@ -436,10 +444,14 @@ void NodeList::processSTUNResponse(unsigned char* packetData, size_t dataBytes) } void NodeList::killNodeWithUUID(const QUuid& nodeUUID) { + _nodeHashMutex.lock(); + NodeHash::iterator nodeItemToKill = _nodeHash.find(nodeUUID); if (nodeItemToKill != _nodeHash.end()) { killNodeAtHashIterator(nodeItemToKill); } + + _nodeHashMutex.unlock(); } NodeHash::iterator NodeList::killNodeAtHashIterator(NodeHash::iterator& nodeItemToKill) { @@ -672,6 +684,8 @@ void NodeList::pingPublicAndLocalSocketsForInactiveNode(Node* node) { SharedNodePointer NodeList::addOrUpdateNode(const QUuid& uuid, char nodeType, const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket) { + _nodeHashMutex.lock(); + SharedNodePointer matchingNode = _nodeHash.value(uuid); if (!matchingNode) { @@ -681,12 +695,16 @@ SharedNodePointer NodeList::addOrUpdateNode(const QUuid& uuid, char nodeType, _nodeHash.insert(newNode->getUUID(), newNodeSharedPointer); + _nodeHashMutex.unlock(); + qDebug() << "Added" << *newNode; emit nodeAdded(newNodeSharedPointer); return newNodeSharedPointer; } else { + _nodeHashMutex.unlock(); + QMutexLocker(&matchingNode->getMutex()); if (matchingNode->getType() == NODE_TYPE_AUDIO_MIXER || @@ -779,6 +797,8 @@ SharedNodePointer NodeList::soloNodeOfType(char nodeType) { void NodeList::removeSilentNodes() { + _nodeHashMutex.lock(); + NodeHash::iterator nodeItem = _nodeHash.begin(); while (nodeItem != _nodeHash.end()) { @@ -796,6 +816,8 @@ void NodeList::removeSilentNodes() { node->getMutex().unlock(); } + + _nodeHashMutex.unlock(); } const QString QSETTINGS_GROUP_NAME = "NodeList"; diff --git a/libraries/shared/src/NodeList.h b/libraries/shared/src/NodeList.h index b665141b66..d8acc6fa24 100644 --- a/libraries/shared/src/NodeList.h +++ b/libraries/shared/src/NodeList.h @@ -143,6 +143,7 @@ private: NodeHash::iterator killNodeAtHashIterator(NodeHash::iterator& nodeItemToKill); NodeHash _nodeHash; + QMutex _nodeHashMutex; QString _domainHostname; HifiSockAddr _domainSockAddr; QUdpSocket _nodeSocket; From 99cd6c8ce8d59ea9b7458db3516dbf74e41b0ba2 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 17 Jan 2014 16:04:04 -0800 Subject: [PATCH 6/7] remove two unused variable warnings --- interface/src/avatar/MyAvatar.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 3c5fdb243f..3bbba8b749 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -175,8 +175,6 @@ void MyAvatar::simulate(float deltaTime, Transmitter* transmitter) { // Compute instantaneous acceleration float forwardAcceleration = glm::length(glm::dot(getBodyFrontDirection(), getVelocity() - oldVelocity)) / deltaTime; - const float ACCELERATION_PITCH_DECAY = 0.4f; - const float ACCELERATION_PULL_THRESHOLD = 0.2f; const float OCULUS_ACCELERATION_PULL_THRESHOLD = 1.0f; const int OCULUS_YAW_OFFSET_THRESHOLD = 10; From 594165782ae1c9194b31764f5380602478752230 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 17 Jan 2014 16:20:22 -0800 Subject: [PATCH 7/7] repair badly scoped usage of QMutexLocker --- interface/src/Application.cpp | 4 ++-- libraries/shared/src/NodeList.cpp | 23 +++++++++++------------ libraries/shared/src/NodeList.h | 3 ++- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 64c883b7f6..229cedf55c 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2006,7 +2006,7 @@ void Application::updateAvatars(float deltaTime, glm::vec3 mouseRayOrigin, glm:: PerformanceWarning warn(showWarnings, "Application::updateAvatars()"); foreach (const SharedNodePointer& node, NodeList::getInstance()->getNodeHash()) { - QMutexLocker(&node->getMutex()); + QMutexLocker locker(&node->getMutex()); if (node->getLinkedData()) { Avatar *avatar = (Avatar *)node->getLinkedData(); if (!avatar->isInitialized()) { @@ -3659,7 +3659,7 @@ void Application::renderAvatars(bool forceRenderHead, bool selfAvatarOnly) { NodeList* nodeList = NodeList::getInstance(); foreach (const SharedNodePointer& node, nodeList->getNodeHash()) { - QMutexLocker(&node->getMutex()); + QMutexLocker locker(&node->getMutex()); if (node->getLinkedData() != NULL && node->getType() == NODE_TYPE_AGENT) { Avatar *avatar = (Avatar *)node->getLinkedData(); diff --git a/libraries/shared/src/NodeList.cpp b/libraries/shared/src/NodeList.cpp index 1813e79e8e..f33137a092 100644 --- a/libraries/shared/src/NodeList.cpp +++ b/libraries/shared/src/NodeList.cpp @@ -231,7 +231,7 @@ void NodeList::processBulkNodeData(const HifiSockAddr& senderAddress, unsigned c } int NodeList::updateNodeWithData(Node *node, const HifiSockAddr& senderSockAddr, unsigned char *packetData, int dataBytes) { - QMutexLocker(&node->getMutex()); + QMutexLocker locker(&node->getMutex()); node->setLastHeardMicrostamp(usecTimestampNow()); @@ -267,16 +267,19 @@ SharedNodePointer NodeList::nodeWithAddress(const HifiSockAddr &senderSockAddr) } SharedNodePointer NodeList::nodeWithUUID(const QUuid& nodeUUID) { - _nodeHashMutex.lock(); - SharedNodePointer matchingNode = _nodeHash.value(nodeUUID); - _nodeHashMutex.unlock(); - return matchingNode; + QMutexLocker locker(&_nodeHashMutex); + return _nodeHash.value(nodeUUID); +} + +NodeHash NodeList::getNodeHash() { + QMutexLocker locker(&_nodeHashMutex); + return NodeHash(_nodeHash); } void NodeList::clear() { qDebug() << "Clearing the NodeList. Deleting all nodes in list."; - _nodeHashMutex.lock(); + QMutexLocker locker(&_nodeHashMutex); NodeHash::iterator nodeItem = _nodeHash.begin(); @@ -284,8 +287,6 @@ void NodeList::clear() { while (nodeItem != _nodeHash.end()) { nodeItem = killNodeAtHashIterator(nodeItem); } - - _nodeHashMutex.unlock(); } void NodeList::reset() { @@ -444,14 +445,12 @@ void NodeList::processSTUNResponse(unsigned char* packetData, size_t dataBytes) } void NodeList::killNodeWithUUID(const QUuid& nodeUUID) { - _nodeHashMutex.lock(); + QMutexLocker locker(&_nodeHashMutex); NodeHash::iterator nodeItemToKill = _nodeHash.find(nodeUUID); if (nodeItemToKill != _nodeHash.end()) { killNodeAtHashIterator(nodeItemToKill); } - - _nodeHashMutex.unlock(); } NodeHash::iterator NodeList::killNodeAtHashIterator(NodeHash::iterator& nodeItemToKill) { @@ -705,7 +704,7 @@ SharedNodePointer NodeList::addOrUpdateNode(const QUuid& uuid, char nodeType, } else { _nodeHashMutex.unlock(); - QMutexLocker(&matchingNode->getMutex()); + QMutexLocker locker(&matchingNode->getMutex()); if (matchingNode->getType() == NODE_TYPE_AUDIO_MIXER || matchingNode->getType() == NODE_TYPE_VOXEL_SERVER || diff --git a/libraries/shared/src/NodeList.h b/libraries/shared/src/NodeList.h index d8acc6fa24..b5e27564b1 100644 --- a/libraries/shared/src/NodeList.h +++ b/libraries/shared/src/NodeList.h @@ -21,6 +21,7 @@ #include // not on windows, not needed for mac or windows #endif +#include #include #include #include @@ -81,7 +82,7 @@ public: void(*linkedDataCreateCallback)(Node *); - const NodeHash& getNodeHash() { return _nodeHash; } + NodeHash getNodeHash(); int size() const { return _nodeHash.size(); } int getNumNoReplyDomainCheckIns() const { return _numNoReplyDomainCheckIns; }