Merge pull request #1272 from ZappoMan/stats_crash_bug_fix

fix to crash in client side VoxelSceneStats crash

interface/src/Application.cpp

@@ -4188,9 +4188,11 @@ void Application::nodeKilled(Node* node) {
         }
         
         // also clean up scene stats for that server
+        _voxelSceneStatsLock.lockForWrite();
         if (_voxelServerSceneStats.find(nodeUUID) != _voxelServerSceneStats.end()) {
             _voxelServerSceneStats.erase(nodeUUID);
         }
+        _voxelSceneStatsLock.unlock();
     } else if (node->getLinkedData() == _lookatTargetAvatar) {
         _lookatTargetAvatar = NULL;
     }
@@ -4211,27 +4213,13 @@ int Application::parseVoxelStats(unsigned char* messageData, ssize_t messageLeng
         QUuid nodeUUID = voxelServer->getUUID();
         
         // now that we know the node ID, let's add these stats to the stats for that node...
+        _voxelSceneStatsLock.lockForWrite();
         if (_voxelServerSceneStats.find(nodeUUID) != _voxelServerSceneStats.end()) {
-            VoxelSceneStats& oldStats = _voxelServerSceneStats[nodeUUID];
-            
-            // this if construct is a little strange because we aren't quite using it yet. But
-            // we want to keep this logic in here for now because we plan to use it soon to determine
-            // additional network optimization states and better rate control
-            if (!oldStats.isMoving() && temp.isMoving()) {
-                // we think we are starting to move
-                _voxelServerSceneStats[nodeUUID].unpackFromMessage(messageData, messageLength);
-            } else if (oldStats.isMoving() && !temp.isMoving()) {
-                // we think we are done moving
-                _voxelServerSceneStats[nodeUUID].unpackFromMessage(messageData, messageLength);
-            } else if (!oldStats.isMoving() && !temp.isMoving()) {
-                // we think we are still not moving
-            } else {
-                // we think we are still moving
-            }
-        
+            _voxelServerSceneStats[nodeUUID].unpackFromMessage(messageData, messageLength);
         } else {
             _voxelServerSceneStats[nodeUUID] = temp;
         }
+        _voxelSceneStatsLock.unlock();
         
         VoxelPositionSize rootDetails;
         voxelDetailsForCode(temp.getJurisdictionRoot(), rootDetails);

interface/src/Application.h

@@ -136,6 +136,8 @@ public:
     Swatch*  getSwatch() { return &_swatch; }
     QMainWindow* getWindow() { return _window; }
     NodeToVoxelSceneStats* getVoxelSceneStats() { return &_voxelServerSceneStats; }
+    void lockVoxelSceneStats() { _voxelSceneStatsLock.lockForRead(); }
+    void unlockVoxelSceneStats() { _voxelSceneStatsLock.unlock(); }
     
     QNetworkAccessManager* getNetworkAccessManager() { return _networkAccessManager; }
     GeometryCache* getGeometryCache() { return &_geometryCache; }
@@ -454,6 +456,7 @@ private:
     
     NodeToJurisdictionMap _voxelServerJurisdictions;
     NodeToVoxelSceneStats _voxelServerSceneStats;
+    QReadWriteLock _voxelSceneStatsLock;
     
     std::vector<VoxelFade> _voxelFades;
 };

interface/src/ui/VoxelStatsDialog.cpp

@@ -154,6 +154,8 @@ void VoxelStatsDialog::paintEvent(QPaintEvent* event) {
     unsigned long totalNodes = 0;
     unsigned long totalInternal = 0;
     unsigned long totalLeaves = 0;
+
+    Application::getInstance()->lockVoxelSceneStats();
     NodeToVoxelSceneStats* sceneStats = Application::getInstance()->getVoxelSceneStats();
     for(NodeToVoxelSceneStatsIterator i = sceneStats->begin(); i != sceneStats->end(); i++) {
         //const QUuid& uuid = i->first;
@@ -176,6 +178,7 @@ void VoxelStatsDialog::paintEvent(QPaintEvent* event) {
             sendingMode << "S";
         }
     }
+    Application::getInstance()->unlockVoxelSceneStats();
     sendingMode << " - " << serverCount << " servers";
     if (movingServerCount > 0) {
         sendingMode << " <SCENE NOT STABLE>";

libraries/voxels/src/VoxelSceneStats.cpp

@@ -28,6 +28,97 @@ VoxelSceneStats::VoxelSceneStats() :
     _isStarted = false;
 }
 
+// copy constructor
+VoxelSceneStats::VoxelSceneStats(const VoxelSceneStats& other) :
+_jurisdictionRoot(NULL) {
+    copyFromOther(other);
+}
+
+// copy assignment
+VoxelSceneStats& VoxelSceneStats::operator=(const VoxelSceneStats& other) {
+    copyFromOther(other);
+    return *this;
+}
+
+void VoxelSceneStats::copyFromOther(const VoxelSceneStats& other) {
+    _totalEncodeTime = other._totalEncodeTime;
+    _encodeStart = other._encodeStart;
+
+    _packets = other._packets;
+    _bytes = other._bytes;
+    _passes = other._passes;
+
+    _totalVoxels = other._totalVoxels;
+    _totalInternal = other._totalInternal;
+    _totalLeaves = other._totalLeaves;
+
+    _traversed = other._traversed;
+    _internal = other._internal;
+    _leaves = other._leaves;
+
+    _skippedDistance = other._skippedDistance;
+    _internalSkippedDistance = other._internalSkippedDistance;
+    _leavesSkippedDistance = other._leavesSkippedDistance;
+
+    _skippedOutOfView = other._skippedOutOfView;
+    _internalSkippedOutOfView = other._internalSkippedOutOfView;
+    _leavesSkippedOutOfView = other._leavesSkippedOutOfView;
+
+    _skippedWasInView = other._skippedWasInView;
+    _internalSkippedWasInView = other._internalSkippedWasInView;
+    _leavesSkippedWasInView = other._leavesSkippedWasInView;
+
+    _skippedNoChange = other._skippedNoChange;
+    _internalSkippedNoChange = other._internalSkippedNoChange;
+    _leavesSkippedNoChange = other._leavesSkippedNoChange;
+
+    _skippedOccluded = other._skippedOccluded;
+    _internalSkippedOccluded = other._internalSkippedOccluded;
+    _leavesSkippedOccluded = other._leavesSkippedOccluded;
+
+    _colorSent = other._colorSent;
+    _internalColorSent = other._internalColorSent;
+    _leavesColorSent = other._leavesColorSent;
+
+    _didntFit = other._didntFit;
+    _internalDidntFit = other._internalDidntFit;
+    _leavesDidntFit = other._leavesDidntFit;
+
+    _colorBitsWritten = other._colorBitsWritten;
+    _existsBitsWritten = other._existsBitsWritten;
+    _existsInPacketBitsWritten = other._existsInPacketBitsWritten;
+    _treesRemoved = other._treesRemoved;
+
+    // before copying the jurisdictions, delete any current values...
+    if (_jurisdictionRoot) {
+        delete[] _jurisdictionRoot;
+        _jurisdictionRoot = NULL;
+    }
+    for (int i=0; i < _jurisdictionEndNodes.size(); i++) {
+        if (_jurisdictionEndNodes[i]) {
+            delete[] _jurisdictionEndNodes[i];
+        }
+    }
+    _jurisdictionEndNodes.clear();
+    
+    // Now copy the values from the other    
+    if (other._jurisdictionRoot) {
+        int bytes = bytesRequiredForCodeLength(numberOfThreeBitSectionsInCode(other._jurisdictionRoot));
+        _jurisdictionRoot = new unsigned char[bytes];
+        memcpy(_jurisdictionRoot, other._jurisdictionRoot, bytes);
+    }
+    for (int i=0; i < other._jurisdictionEndNodes.size(); i++) {
+        unsigned char* endNodeCode = other._jurisdictionEndNodes[i];
+        if (endNodeCode) {
+            int bytes = bytesRequiredForCodeLength(numberOfThreeBitSectionsInCode(endNodeCode));
+            unsigned char* endNodeCodeCopy = new unsigned char[bytes];
+            memcpy(endNodeCodeCopy, endNodeCode, bytes);
+            _jurisdictionEndNodes.push_back(endNodeCodeCopy);
+        }
+    }
+}
+
+
 VoxelSceneStats::~VoxelSceneStats() {
     reset();
 }
@@ -48,6 +139,15 @@ void VoxelSceneStats::sceneStarted(bool isFullScene, bool isMoving, VoxelNode* r
         delete[] _jurisdictionRoot;
         _jurisdictionRoot = NULL;
     }
+    // clear existing endNodes before copying new ones...
+    for (int i=0; i < _jurisdictionEndNodes.size(); i++) {
+        if (_jurisdictionEndNodes[i]) {
+            delete[] _jurisdictionEndNodes[i];
+        }
+    }
+    _jurisdictionEndNodes.clear();
+
+    // setup jurisdictions
     if (jurisdictionMap) {
         unsigned char* jurisdictionRoot = jurisdictionMap->getRootOctalCode();
         if (jurisdictionRoot) {
@@ -56,6 +156,7 @@ void VoxelSceneStats::sceneStarted(bool isFullScene, bool isMoving, VoxelNode* r
             memcpy(_jurisdictionRoot, jurisdictionRoot, bytes);
         }
 
+        // copy new endNodes...
         for (int i=0; i < jurisdictionMap->getEndNodeCount(); i++) {
             unsigned char* endNodeCode = jurisdictionMap->getEndNodeOctalCode(i);
             if (endNodeCode) {
@@ -436,6 +537,20 @@ int VoxelSceneStats::unpackFromMessage(unsigned char* sourceBuffer, int availabl
     memcpy(&_treesRemoved, sourceBuffer, sizeof(_treesRemoved));
     sourceBuffer += sizeof(_treesRemoved);
 
+    // before allocating new juridiction, clean up existing ones
+    if (_jurisdictionRoot) {
+        delete[] _jurisdictionRoot;
+        _jurisdictionRoot = NULL;
+    }
+
+    // clear existing endNodes before copying new ones...
+    for (int i=0; i < _jurisdictionEndNodes.size(); i++) {
+        if (_jurisdictionEndNodes[i]) {
+            delete[] _jurisdictionEndNodes[i];
+        }
+    }
+    _jurisdictionEndNodes.clear();
+
     // read the root jurisdiction
     int bytes = 0;
     memcpy(&bytes, sourceBuffer, sizeof(bytes));

libraries/voxels/src/VoxelSceneStats.h

@@ -27,6 +27,9 @@ public:
     ~VoxelSceneStats();
     void reset();
     
+    VoxelSceneStats(const VoxelSceneStats& other); // copy constructor
+    VoxelSceneStats& operator= (const VoxelSceneStats& other); // copy assignment
+    
     /// Call when beginning the computation of a scene. Initializes internal structures
     void sceneStarted(bool fullScene, bool moving, VoxelNode* root, JurisdictionMap* jurisdictionMap);
 
@@ -144,6 +147,9 @@ public:
     unsigned long getTotalLeaves() const { return _totalLeaves; }
     
 private:
+
+    void copyFromOther(const VoxelSceneStats& other);
+
     bool _isReadyToSend;
     unsigned char _statsMessage[MAX_PACKET_SIZE];
     int _statsMessageLength;