diff --git a/assignment-client/src/octree/OctreeQueryNode.cpp b/assignment-client/src/octree/OctreeQueryNode.cpp index 2ceb9e1040..e0ff29effd 100644 --- a/assignment-client/src/octree/OctreeQueryNode.cpp +++ b/assignment-client/src/octree/OctreeQueryNode.cpp @@ -67,6 +67,14 @@ OctreeQueryNode::~OctreeQueryNode() { } +void OctreeQueryNode::deleteLater() { + _isShuttingDown = true; + if (_octreeSendThread) { + _octreeSendThread->setIsShuttingDown(); + } + OctreeQuery::deleteLater(); +} + void OctreeQueryNode::initializeOctreeSendThread(OctreeServer* octreeServer, const QUuid& nodeUUID) { // Create octree sending thread... diff --git a/assignment-client/src/octree/OctreeQueryNode.h b/assignment-client/src/octree/OctreeQueryNode.h index eab8cb5d0a..b7e68e805d 100644 --- a/assignment-client/src/octree/OctreeQueryNode.h +++ b/assignment-client/src/octree/OctreeQueryNode.h @@ -27,6 +27,7 @@ class OctreeQueryNode : public OctreeQuery { public: OctreeQueryNode(); virtual ~OctreeQueryNode(); + virtual void deleteLater(); void init(); // called after creation to set up some virtual items virtual PacketType getMyPacketType() const = 0; diff --git a/assignment-client/src/octree/OctreeSendThread.cpp b/assignment-client/src/octree/OctreeSendThread.cpp index 9c04c4a1ad..a215d9b3c3 100644 --- a/assignment-client/src/octree/OctreeSendThread.cpp +++ b/assignment-client/src/octree/OctreeSendThread.cpp @@ -5,6 +5,8 @@ // Copyright (c) 2013 High Fidelity, Inc. All rights reserved. // +#include + #include #include #include @@ -21,7 +23,9 @@ OctreeSendThread::OctreeSendThread(const QUuid& nodeUUID, OctreeServer* myServer _nodeUUID(nodeUUID), _myServer(myServer), _packetData(), - _nodeMissingCount(0) + _nodeMissingCount(0), + _processLock(), + _isShuttingDown(false) { QString safeServerName("Octree"); if (_myServer) { @@ -43,8 +47,19 @@ OctreeSendThread::~OctreeSendThread() { OctreeServer::clientDisconnected(); } +void OctreeSendThread::setIsShuttingDown() { + QMutexLocker locker(&_processLock); // this will cause us to wait till the process loop is complete + _isShuttingDown = true; +} + bool OctreeSendThread::process() { + QMutexLocker locker(&_processLock); + + if (_isShuttingDown) { + return false; // exit early if we're shutting down + } + const int MAX_NODE_MISSING_CHECKS = 10; if (_nodeMissingCount > MAX_NODE_MISSING_CHECKS) { qDebug() << "our target node:" << _nodeUUID << "has been missing the last" << _nodeMissingCount @@ -56,7 +71,10 @@ bool OctreeSendThread::process() { // don't do any send processing until the initial load of the octree is complete... if (_myServer->isInitialLoadComplete()) { - SharedNodePointer node = NodeList::getInstance()->nodeWithUUID(_nodeUUID); + + // see if we can get access to our node, but don't wait on the lock, if the nodeList is busy + // it might not return a node that is known, but that's ok we can handle that case. + SharedNodePointer node = NodeList::getInstance()->nodeWithUUID(_nodeUUID, false); if (node) { _nodeMissingCount = 0; @@ -113,19 +131,6 @@ int OctreeSendThread::handlePacketSend(const SharedNodePointer& node, bool packetSent = false; // did we send a packet? int packetsSent = 0; - // double check that the node has an active socket, otherwise, don't send... - - quint64 lockWaitStart = usecTimestampNow(); - QMutexLocker locker(&node->getMutex()); - quint64 lockWaitEnd = usecTimestampNow(); - float lockWaitElapsedUsec = (float)(lockWaitEnd - lockWaitStart); - OctreeServer::trackNodeWaitTime(lockWaitElapsedUsec); - - const HifiSockAddr* nodeAddress = node->getActiveSocket(); - if (!nodeAddress) { - return packetsSent; // without sending... - } - // Here's where we check to see if this packet is a duplicate of the last packet. If it is, we will silently // obscure the packet and not send it. This allows the callers and upper level logic to not need to know about // this rate control savings. diff --git a/assignment-client/src/octree/OctreeSendThread.h b/assignment-client/src/octree/OctreeSendThread.h index 39c27911b0..ab88121ee8 100644 --- a/assignment-client/src/octree/OctreeSendThread.h +++ b/assignment-client/src/octree/OctreeSendThread.h @@ -23,6 +23,8 @@ class OctreeSendThread : public GenericThread { public: OctreeSendThread(const QUuid& nodeUUID, OctreeServer* myServer); virtual ~OctreeSendThread(); + + void setIsShuttingDown(); static quint64 _totalBytes; static quint64 _totalWastedBytes; @@ -45,6 +47,8 @@ private: OctreePacketData _packetData; int _nodeMissingCount; + QMutex _processLock; // don't allow us to have our nodeData, or our thread to be deleted while we're processing + bool _isShuttingDown; }; #endif // __octree_server__OctreeSendThread__ diff --git a/libraries/shared/src/NodeList.cpp b/libraries/shared/src/NodeList.cpp index 6d699322de..797e90fa51 100644 --- a/libraries/shared/src/NodeList.cpp +++ b/libraries/shared/src/NodeList.cpp @@ -308,9 +308,21 @@ int NodeList::findNodeAndUpdateWithDataFromPacket(const QByteArray& packet) { return 0; } -SharedNodePointer NodeList::nodeWithUUID(const QUuid& nodeUUID) { - QMutexLocker locker(&_nodeHashMutex); - return _nodeHash.value(nodeUUID); +SharedNodePointer NodeList::nodeWithUUID(const QUuid& nodeUUID, bool blockingLock) { + SharedNodePointer node; + // if caller wants us to block and guarantee the correct answer, then honor that request + if (blockingLock) { + // this will block till we can get access + QMutexLocker locker(&_nodeHashMutex); + node = _nodeHash.value(nodeUUID); + } else { + // some callers are willing to get wrong answers but not block + if (_nodeHashMutex.tryLock()) { + node = _nodeHash.value(nodeUUID); + _nodeHashMutex.unlock(); + } + } + return node; } SharedNodePointer NodeList::sendingNodeForPacket(const QByteArray& packet) { diff --git a/libraries/shared/src/NodeList.h b/libraries/shared/src/NodeList.h index 590a2ce83f..d6328a1303 100644 --- a/libraries/shared/src/NodeList.h +++ b/libraries/shared/src/NodeList.h @@ -101,7 +101,8 @@ public: QByteArray constructPingReplyPacket(const QByteArray& pingPacket); void pingPublicAndLocalSocketsForInactiveNode(const SharedNodePointer& node); - SharedNodePointer nodeWithUUID(const QUuid& nodeUUID); + /// passing false for blockingLock, will tryLock, and may return NULL when a node with the UUID actually does exist + SharedNodePointer nodeWithUUID(const QUuid& nodeUUID, bool blockingLock = true); SharedNodePointer sendingNodeForPacket(const QByteArray& packet); SharedNodePointer addOrUpdateNode(const QUuid& uuid, char nodeType,