From 6881d9fb363457cd60a243e8f808b93942bf24b8 Mon Sep 17 00:00:00 2001 From: ZappoMan Date: Tue, 19 Nov 2013 22:57:42 -0800 Subject: [PATCH] fix to crash in client side VoxelSceneStats crash --- interface/src/Application.cpp | 22 +---- interface/src/Application.h | 3 + interface/src/ui/VoxelStatsDialog.cpp | 3 + libraries/voxels/src/VoxelSceneStats.cpp | 115 +++++++++++++++++++++++ libraries/voxels/src/VoxelSceneStats.h | 6 ++ 5 files changed, 132 insertions(+), 17 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 628a91c69b..385e7bc8af 100644 --- a/interface/src/Application.cpp +++ b/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); diff --git a/interface/src/Application.h b/interface/src/Application.h index 78530ecea2..21e3479288 100644 --- a/interface/src/Application.h +++ b/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 _voxelFades; }; diff --git a/interface/src/ui/VoxelStatsDialog.cpp b/interface/src/ui/VoxelStatsDialog.cpp index ab1371f53f..ff082e4b02 100644 --- a/interface/src/ui/VoxelStatsDialog.cpp +++ b/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 << " "; diff --git a/libraries/voxels/src/VoxelSceneStats.cpp b/libraries/voxels/src/VoxelSceneStats.cpp index cce7d7d7b3..fa5968a6e3 100644 --- a/libraries/voxels/src/VoxelSceneStats.cpp +++ b/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)); diff --git a/libraries/voxels/src/VoxelSceneStats.h b/libraries/voxels/src/VoxelSceneStats.h index c93633fea7..245fe430dd 100644 --- a/libraries/voxels/src/VoxelSceneStats.h +++ b/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;