fixes to voxel server crashes on client shutdown

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...

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;

assignment-client/src/octree/OctreeSendThread.cpp

@@ -5,6 +5,8 @@
 //  Copyright (c) 2013 High Fidelity, Inc. All rights reserved.
 //
 
+#include <QMutexLocker>
+
 #include <NodeList.h>
 #include <PacketHeaders.h>
 #include <PerfStat.h>
@@ -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.

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__

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) {

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,