Merge pull request #2407 from ZappoMan/voxel_server_crash_fixes

fix voxel server crash/pure virtual call on shutdown of client

assignment-client/src/octree/OctreeQueryNode.cpp

@@ -36,18 +36,34 @@ OctreeQueryNode::OctreeQueryNode() :
     _lodChanged(false),
     _lodInitialized(false),
     _sequenceNumber(0),
-    _lastRootTimestamp(0)
+    _lastRootTimestamp(0),
+    _myPacketType(PacketTypeUnknown),
+    _isShuttingDown(false)
 {
 }
 
 OctreeQueryNode::~OctreeQueryNode() {
+    _isShuttingDown = true;
+    const bool extraDebugging = false;
+    if (extraDebugging) {
+        qDebug() << "OctreeQueryNode::~OctreeQueryNode()";
+    }
     if (_octreeSendThread) {
+        if (extraDebugging) {
+            qDebug() << "OctreeQueryNode::~OctreeQueryNode()... calling _octreeSendThread->terminate()";
+        }
         _octreeSendThread->terminate();
+        if (extraDebugging) {
+            qDebug() << "OctreeQueryNode::~OctreeQueryNode()... calling delete _octreeSendThread";
+        }
         delete _octreeSendThread;
     }
 
     delete[] _octreePacket;
     delete[] _lastOctreePacket;
+    if (extraDebugging) {
+        qDebug() << "OctreeQueryNode::~OctreeQueryNode()... DONE...";
+    }
 }
 
 
@@ -59,9 +75,13 @@ void OctreeQueryNode::initializeOctreeSendThread(OctreeServer* octreeServer, con
 }
 
 bool OctreeQueryNode::packetIsDuplicate() const {
+    // if shutting down, return immediately
+    if (_isShuttingDown) {
+        return false;
+    }
     // since our packets now include header information, like sequence number, and createTime, we can't just do a memcmp
     // of the entire packet, we need to compare only the packet content...
-    int numBytesPacketHeader = numBytesForPacketHeaderGivenPacketType(getMyPacketType());
+    int numBytesPacketHeader = numBytesForPacketHeaderGivenPacketType(_myPacketType);
     
     if (_lastOctreePacketLength == getPacketLength()) {
         if (memcmp(_lastOctreePacket + (numBytesPacketHeader + OCTREE_PACKET_EXTRA_HEADERS_SIZE),
@@ -74,6 +94,11 @@ bool OctreeQueryNode::packetIsDuplicate() const {
 }
 
 bool OctreeQueryNode::shouldSuppressDuplicatePacket() {
+    // if shutting down, return immediately
+    if (_isShuttingDown) {
+        return true;
+    }
+
     bool shouldSuppress = false; // assume we won't suppress
 
     // only consider duplicate packets
@@ -107,7 +132,17 @@ bool OctreeQueryNode::shouldSuppressDuplicatePacket() {
     return shouldSuppress;
 }
 
+void OctreeQueryNode::init() {
+    _myPacketType = getMyPacketType();
+    resetOctreePacket(true); // don't bump sequence
+}
+
 void OctreeQueryNode::resetOctreePacket(bool lastWasSurpressed) {
+    // if shutting down, return immediately
+    if (_isShuttingDown) {
+        return;
+    }
+
     // Whenever we call this, we will keep a copy of the last packet, so we can determine if the last packet has
     // changed since we last reset it. Since we know that no two packets can ever be identical without being the same
     // scene information, (e.g. the root node packet of a static scene), we can use this as a strategy for reducing
@@ -128,7 +163,7 @@ void OctreeQueryNode::resetOctreePacket(bool lastWasSurpressed) {
     }
 
     _octreePacketAvailableBytes = MAX_PACKET_SIZE;
-    int numBytesPacketHeader = populatePacketHeader(reinterpret_cast<char*>(_octreePacket), getMyPacketType());
+    int numBytesPacketHeader = populatePacketHeader(reinterpret_cast<char*>(_octreePacket), _myPacketType);
     _octreePacketAt = _octreePacket + numBytesPacketHeader;
     _octreePacketAvailableBytes -= numBytesPacketHeader;
 
@@ -158,6 +193,11 @@ void OctreeQueryNode::resetOctreePacket(bool lastWasSurpressed) {
 }
 
 void OctreeQueryNode::writeToPacket(const unsigned char* buffer, unsigned int bytes) {
+    // if shutting down, return immediately
+    if (_isShuttingDown) {
+        return;
+    }
+
     // compressed packets include lead bytes which contain compressed size, this allows packing of
     // multiple compressed portions together
     if (_currentPacketIsCompressed) {
@@ -174,6 +214,11 @@ void OctreeQueryNode::writeToPacket(const unsigned char* buffer, unsigned int by
 }
 
 bool OctreeQueryNode::updateCurrentViewFrustum() {
+    // if shutting down, return immediately
+    if (_isShuttingDown) {
+        return false;
+    }
+
     bool currentViewFrustumChanged = false;
     ViewFrustum newestViewFrustum;
     // get position and orientation details from the camera
@@ -233,6 +278,11 @@ void OctreeQueryNode::setViewSent(bool viewSent) {
 }
 
 void OctreeQueryNode::updateLastKnownViewFrustum() {
+    // if shutting down, return immediately
+    if (_isShuttingDown) {
+        return;
+    }
+
     bool frustumChanges = !_lastKnownViewFrustum.isVerySimilar(_currentViewFrustum);
 
     if (frustumChanges) {
@@ -247,6 +297,11 @@ void OctreeQueryNode::updateLastKnownViewFrustum() {
 
 
 bool OctreeQueryNode::moveShouldDump() const {
+    // if shutting down, return immediately
+    if (_isShuttingDown) {
+        return false;
+    }
+
     glm::vec3 oldPosition = _lastKnownViewFrustum.getPosition();
     glm::vec3 newPosition = _currentViewFrustum.getPosition();
 
@@ -259,6 +314,11 @@ bool OctreeQueryNode::moveShouldDump() const {
 }
 
 void OctreeQueryNode::dumpOutOfView() {
+    // if shutting down, return immediately
+    if (_isShuttingDown) {
+        return;
+    }
+    
     int stillInView = 0;
     int outOfView = 0;
     OctreeElementBag tempBag;

assignment-client/src/octree/OctreeQueryNode.h

@@ -28,6 +28,7 @@ public:
     OctreeQueryNode();
     virtual ~OctreeQueryNode();
     
+    void init(); // called after creation to set up some virtual items
     virtual PacketType getMyPacketType() const = 0;
 
     void resetOctreePacket(bool lastWasSurpressed = false);  // resets octree packet to after "V" header
@@ -91,6 +92,7 @@ public:
     unsigned int getlastOctreePacketLength() const { return _lastOctreePacketLength; }
     int getDuplicatePacketCount() const { return _duplicatePacketCount; }
     
+    bool isShuttingDown() const { return _isShuttingDown; }
     
 private:
     OctreeQueryNode(const OctreeQueryNode &);
@@ -127,6 +129,9 @@ private:
     
     OCTREE_PACKET_SEQUENCE _sequenceNumber;
     quint64 _lastRootTimestamp;
+    
+    PacketType _myPacketType;
+    bool _isShuttingDown;
 };
 
 #endif /* defined(__hifi__OctreeQueryNode__) */

assignment-client/src/octree/OctreeSendThread.cpp

@@ -65,7 +65,7 @@ bool OctreeSendThread::process() {
             nodeData = (OctreeQueryNode*) node->getLinkedData();
 
             // Sometimes the node data has not yet been linked, in which case we can't really do anything
-            if (nodeData) {
+            if (nodeData && !nodeData->isShuttingDown()) {
                 bool viewFrustumChanged = nodeData->updateCurrentViewFrustum();
                 packetDistributor(node, nodeData, viewFrustumChanged);
             }
@@ -99,7 +99,14 @@ quint64 OctreeSendThread::_totalBytes = 0;
 quint64 OctreeSendThread::_totalWastedBytes = 0;
 quint64 OctreeSendThread::_totalPackets = 0;
 
-int OctreeSendThread::handlePacketSend(const SharedNodePointer& node, OctreeQueryNode* nodeData, int& trueBytesSent, int& truePacketsSent) {
+int OctreeSendThread::handlePacketSend(const SharedNodePointer& node, 
+                        OctreeQueryNode* nodeData, int& trueBytesSent, int& truePacketsSent) {
+                 
+    // if we're shutting down, then exit early       
+    if (nodeData->isShuttingDown()) {
+        return 0;
+    }
+    
     bool debug = _myServer->wantsDebugSending();
     quint64 now = usecTimestampNow();
 
@@ -136,7 +143,7 @@ int OctreeSendThread::handlePacketSend(const SharedNodePointer& node, OctreeQuer
 
 
     // If we've got a stats message ready to send, then see if we can piggyback them together
-    if (nodeData->stats.isReadyToSend()) {
+    if (nodeData->stats.isReadyToSend() && !nodeData->isShuttingDown()) {
         // Send the stats message to the client
         unsigned char* statsMessage = nodeData->stats.getStatsMessage();
         int statsMessageLength = nodeData->stats.getStatsMessageLength();
@@ -203,7 +210,7 @@ int OctreeSendThread::handlePacketSend(const SharedNodePointer& node, OctreeQuer
         nodeData->stats.markAsSent();
     } else {
         // If there's actually a packet waiting, then send it.
-        if (nodeData->isPacketWaiting()) {
+        if (nodeData->isPacketWaiting() && !nodeData->isShuttingDown()) {
             // just send the voxel packet
             NodeList::getInstance()->writeDatagram((char*) nodeData->getPacket(), nodeData->getPacketLength(),
                                                    SharedNodePointer(node));
@@ -234,6 +241,12 @@ int OctreeSendThread::handlePacketSend(const SharedNodePointer& node, OctreeQuer
 
 /// Version of voxel distributor that sends the deepest LOD level at once
 int OctreeSendThread::packetDistributor(const SharedNodePointer& node, OctreeQueryNode* nodeData, bool viewFrustumChanged) {
+
+    // if shutting down, exit early
+    if (nodeData->isShuttingDown()) {
+        return 0;
+    }
+    
     int truePacketsSent = 0;
     int trueBytesSent = 0;
     int packetsSentThisInterval = 0;
@@ -336,7 +349,7 @@ int OctreeSendThread::packetDistributor(const SharedNodePointer& node, OctreeQue
 
         int extraPackingAttempts = 0;
         bool completedScene = false;
-        while (somethingToSend && packetsSentThisInterval < maxPacketsPerInterval) {
+        while (somethingToSend && packetsSentThisInterval < maxPacketsPerInterval && !nodeData->isShuttingDown()) {
             float lockWaitElapsedUsec = OctreeServer::SKIP_TIME;
             float encodeElapsedUsec = OctreeServer::SKIP_TIME;
             float compressAndWriteElapsedUsec = OctreeServer::SKIP_TIME;
@@ -503,7 +516,7 @@ int OctreeSendThread::packetDistributor(const SharedNodePointer& node, OctreeQue
         // Here's where we can/should allow the server to send other data...
         // send the environment packet
         // TODO: should we turn this into a while loop to better handle sending multiple special packets
-        if (_myServer->hasSpecialPacketToSend(node)) {
+        if (_myServer->hasSpecialPacketToSend(node) && !nodeData->isShuttingDown()) {
             trueBytesSent += _myServer->sendSpecialPacket(node);
             truePacketsSent++;
             packetsSentThisInterval++;

assignment-client/src/octree/OctreeServer.cpp

@@ -168,7 +168,7 @@ void OctreeServer::trackPacketSendingTime(float time) {
 void OctreeServer::attachQueryNodeToNode(Node* newNode) {
     if (!newNode->getLinkedData()) {
         OctreeQueryNode* newQueryNodeData = _instance->createOctreeQueryNode();
-        newQueryNodeData->resetOctreePacket(true); // don't bump sequence
+        newQueryNodeData->init();
         newNode->setLinkedData(newQueryNodeData);
     }
 }
@@ -784,16 +784,26 @@ void OctreeServer::readPendingDatagrams() {
             if (packetType == getMyQueryMessageType()) {
                 bool debug = false;
                 if (debug) {
-                    qDebug() << "Got PacketTypeVoxelQuery at" << usecTimestampNow();
+                    if (matchingNode) {
+                        qDebug() << "Got PacketTypeVoxelQuery at" << usecTimestampNow() << "node:" << *matchingNode;
+                    } else {
+                        qDebug() << "Got PacketTypeVoxelQuery at" << usecTimestampNow() << "node: ??????";
+                    }
                 }
                 
                 // If we got a PacketType_VOXEL_QUERY, then we're talking to an NodeType_t_AVATAR, and we
                 // need to make sure we have it in our nodeList.
                 if (matchingNode) {
+                    if (debug) {
+                        qDebug() << "calling updateNodeWithDataFromPacket()... node:" << *matchingNode;
+                    }
                     nodeList->updateNodeWithDataFromPacket(matchingNode, receivedPacket);
                     
                     OctreeQueryNode* nodeData = (OctreeQueryNode*) matchingNode->getLinkedData();
                     if (nodeData && !nodeData->isOctreeSendThreadInitalized()) {
+                        if (debug) {
+                            qDebug() << "calling initializeOctreeSendThread()... node:" << *matchingNode;
+                        }
                         nodeData->initializeOctreeSendThread(this, matchingNode->getUUID());
                     }
                 }
@@ -999,6 +1009,8 @@ void OctreeServer::nodeKilled(SharedNodePointer node) {
         node->setLinkedData(NULL); // set this first in case another thread comes through and tryes to acces this
         qDebug() << qPrintable(_safeServerName) << "server deleting Linked Data for node:" << *node;
         nodeData->deleteLater();
+    } else {
+        qDebug() << qPrintable(_safeServerName) << "server node missing linked data node:" << *node;
     }
 }