Merge pull request #2175 from ZappoMan/bugfixes

Fixes to particle deadlock, and some other crashes

interface/src/ParticleTreeRenderer.cpp

@@ -33,10 +33,7 @@ void ParticleTreeRenderer::init() {
 void ParticleTreeRenderer::update() {
     if (_tree) {
         ParticleTree* tree = static_cast<ParticleTree*>(_tree);
-        if (tree->tryLockForWrite()) {
-            tree->update();
-            tree->unlock();
-        }
+        tree->update();
     }
 }
 

libraries/octree/src/Octree.cpp

@@ -41,7 +41,8 @@ float boundaryDistanceForRenderLevel(unsigned int renderLevel, float voxelSizeSc
 Octree::Octree(bool shouldReaverage) :
     _isDirty(true),
     _shouldReaverage(shouldReaverage),
-    _stopImport(false) {
+    _stopImport(false),
+    _lock(QReadWriteLock::Recursive) {
     _rootNode = NULL;
     _isViewing = false;
 }
@@ -551,7 +552,10 @@ OctreeElement* Octree::getOctreeElementAt(float x, float y, float z, float s) co
 
 
 OctreeElement* Octree::getOrCreateChildElementAt(float x, float y, float z, float s) {
-    return getRoot()->getOrCreateChildElementAt(x, y, z, s);
+    lockForWrite();
+    OctreeElement* result = getRoot()->getOrCreateChildElementAt(x, y, z, s);
+    unlock();
+    return result;
 }
 
 

libraries/octree/src/Octree.h

@@ -240,11 +240,11 @@ public:
     bool readFromSVOFile(const char* filename);
 
     // Octree does not currently handle its own locking, caller must use these to lock/unlock
-    void lockForRead() { lock.lockForRead(); }
-    bool tryLockForRead() { return lock.tryLockForRead(); }
-    void lockForWrite() { lock.lockForWrite(); }
-    bool tryLockForWrite() { return lock.tryLockForWrite(); }
-    void unlock() { lock.unlock(); }
+    void lockForRead() { _lock.lockForRead(); }
+    bool tryLockForRead() { return _lock.tryLockForRead(); }
+    void lockForWrite() { _lock.lockForWrite(); }
+    bool tryLockForWrite() { return _lock.tryLockForWrite(); }
+    void unlock() { _lock.unlock(); }
 
     unsigned long getOctreeElementsCount();
 
@@ -329,7 +329,7 @@ protected:
     /// flushes out any Octal Codes that had to be queued
     void emptyDeleteQueue();
 
-    QReadWriteLock lock;
+    QReadWriteLock _lock;
     
     /// This tree is receiving inbound viewer datagrams.
     bool _isViewing;

libraries/particles/src/Particle.cpp

@@ -385,7 +385,7 @@ Particle Particle::fromEditPacket(const unsigned char* data, int length, int& pr
 
     } else {
         // look up the existing particle
-        const Particle* existingParticle = tree->findParticleByID(editID, true);
+        const Particle* existingParticle = tree->findParticleByID(editID);
 
         // copy existing properties before over-writing with new properties
         if (existingParticle) {

libraries/particles/src/ParticleTree.cpp

@@ -99,8 +99,8 @@ void ParticleTree::storeParticle(const Particle& particle, const SharedNodePoint
     if (!args.found) {
         glm::vec3 position = particle.getPosition();
         float size = std::max(MINIMUM_PARTICLE_ELEMENT_SIZE, particle.getRadius());
-        ParticleTreeElement* element = (ParticleTreeElement*)getOrCreateChildElementAt(position.x, position.y, position.z, size);
 
+        ParticleTreeElement* element = (ParticleTreeElement*)getOrCreateChildElementAt(position.x, position.y, position.z, size);
         element->storeParticle(particle);
     }
     // what else do we need to do here to get reaveraging to work
@@ -149,8 +149,8 @@ void ParticleTree::addParticle(const ParticleID& particleID, const ParticlePrope
     Particle particle(particleID, properties);
     glm::vec3 position = particle.getPosition();
     float size = std::max(MINIMUM_PARTICLE_ELEMENT_SIZE, particle.getRadius());
+    
     ParticleTreeElement* element = (ParticleTreeElement*)getOrCreateChildElementAt(position.x, position.y, position.z, size);
-
     element->storeParticle(particle);
     
     _isDirty = true;
@@ -370,17 +370,12 @@ bool ParticleTree::findByIDOperation(OctreeElement* element, void* extraData) {
 }
 
 
-const Particle* ParticleTree::findParticleByID(uint32_t id, bool alreadyLocked) {
+const Particle* ParticleTree::findParticleByID(uint32_t id) {
     FindByIDArgs args = { id, false, NULL };
-    if (!alreadyLocked) {
-        //qDebug() << "ParticleTree::findParticleByID().... about to call lockForRead()....";
-        lockForRead();
-        //qDebug() << "ParticleTree::findParticleByID().... after call lockForRead()....";
-    }
+
+    lockForRead();
     recurseTreeWithOperation(findByIDOperation, &args);
-    if (!alreadyLocked) {
-        unlock();
-    }
+    unlock();
     return args.foundParticle;
 }
 
@@ -487,7 +482,9 @@ void ParticleTree::update() {
     }
 
     // prune the tree...
+    lockForWrite();
     recurseTreeWithOperation(pruneOperation, NULL);
+    unlock();
 }
 
 

libraries/particles/src/ParticleTree.h

@@ -44,7 +44,7 @@ public:
     void addParticle(const ParticleID& particleID, const ParticleProperties& properties);
     void deleteParticle(const ParticleID& particleID);
     const Particle* findClosestParticle(glm::vec3 position, float targetRadius);
-    const Particle* findParticleByID(uint32_t id, bool alreadyLocked = false);
+    const Particle* findParticleByID(uint32_t id);
 
     /// finds all particles that touch a sphere
     /// \param center the center of the sphere

libraries/particles/src/ParticlesScriptingInterface.cpp

@@ -33,9 +33,7 @@ ParticleID ParticlesScriptingInterface::addParticle(const ParticleProperties& pr
 
     // If we have a local particle tree set, then also update it.
     if (_particleTree) {
-        _particleTree->lockForWrite();
         _particleTree->addParticle(id, properties);
-        _particleTree->unlock();
     }
 
     return id;
@@ -66,7 +64,7 @@ ParticleProperties ParticlesScriptingInterface::getParticleProperties(ParticleID
     }
     if (_particleTree) {
         _particleTree->lockForRead();
-        const Particle* particle = _particleTree->findParticleByID(identity.id, true);
+        const Particle* particle = _particleTree->findParticleByID(identity.id);
         if (particle) {
             results.copyFromParticle(*particle);
         } else {

libraries/particles/src/ParticlesScriptingInterface.h

@@ -30,12 +30,10 @@ public:
 private slots:
     /// inbound slots for external collision systems
     void forwardParticleCollisionWithVoxel(const ParticleID& particleID, const VoxelDetail& voxel) {
-        qDebug() << "forwardParticleCollisionWithVoxel()";
         emit particleCollisionWithVoxel(particleID, voxel);
     }
 
     void forwardParticleCollisionWithParticle(const ParticleID& idA, const ParticleID& idB) {
-        qDebug() << "forwardParticleCollisionWithParticle()";
         emit particleCollisionWithParticle(idA, idB);
     }