fix crash when deleting entities quickly

interface/src/Application.cpp

@@ -3089,13 +3089,14 @@ void Application::update(float deltaTime) {
             static VectorOfMotionStates motionStates;
             _entitySimulation.getObjectsToRemoveFromPhysics(motionStates);
             _physicsEngine->removeObjects(motionStates);
+            _entitySimulation.deleteObjectsRemovedFromPhysics();
 
-            getEntities()->getTree()->withWriteLock([&] {
+            getEntities()->getTree()->withReadLock([&] {
                 _entitySimulation.getObjectsToAddToPhysics(motionStates);
                 _physicsEngine->addObjects(motionStates);
 
             });
-            getEntities()->getTree()->withWriteLock([&] {
+            getEntities()->getTree()->withReadLock([&] {
                 _entitySimulation.getObjectsToChange(motionStates);
                 VectorOfMotionStates stillNeedChange = _physicsEngine->changeObjects(motionStates);
                 _entitySimulation.setObjectsToChange(stillNeedChange);

libraries/entities/src/EntitySimulation.cpp

@@ -48,6 +48,7 @@ void EntitySimulation::takeEntitiesToDelete(VectorOfEntities& entitiesToDelete)
 }
 
 void EntitySimulation::removeEntityInternal(EntityItemPointer entity) {
+    QMutexLocker lock(&_mutex);
     // remove from all internal lists except _entitiesToDelete
     _mortalEntities.remove(entity);
     _entitiesToUpdate.remove(entity);
@@ -61,6 +62,7 @@ void EntitySimulation::prepareEntityForDelete(EntityItemPointer entity) {
     assert(entity);
     assert(entity->isDead());
     if (entity->isSimulated()) {
+        QMutexLocker lock(&_mutex);
         entity->clearActions(this);
         removeEntityInternal(entity);
         _entitiesToDelete.insert(entity);
@@ -69,11 +71,13 @@ void EntitySimulation::prepareEntityForDelete(EntityItemPointer entity) {
 
 void EntitySimulation::addEntityInternal(EntityItemPointer entity) {
     if (entity->isMoving() && !entity->getPhysicsInfo()) {
+        QMutexLocker lock(&_mutex);
         _simpleKinematicEntities.insert(entity);
     }
 }
 
 void EntitySimulation::changeEntityInternal(EntityItemPointer entity) {
+    QMutexLocker lock(&_mutex);
     if (entity->isMoving() && !entity->getPhysicsInfo()) {
         _simpleKinematicEntities.insert(entity);
     } else {
@@ -86,6 +90,7 @@ void EntitySimulation::expireMortalEntities(const quint64& now) {
     if (now > _nextExpiry) {
         // only search for expired entities if we expect to find one
         _nextExpiry = quint64(-1);
+        QMutexLocker lock(&_mutex);
         SetOfEntities::iterator itemItr = _mortalEntities.begin();
         while (itemItr != _mortalEntities.end()) {
             EntityItemPointer entity = *itemItr;
@@ -108,6 +113,7 @@ void EntitySimulation::expireMortalEntities(const quint64& now) {
 // protected
 void EntitySimulation::callUpdateOnEntitiesThatNeedIt(const quint64& now) {
     PerformanceTimer perfTimer("updatingEntities");
+    QMutexLocker lock(&_mutex);
     SetOfEntities::iterator itemItr = _entitiesToUpdate.begin();
     while (itemItr != _entitiesToUpdate.end()) {
         EntityItemPointer entity = *itemItr;
@@ -124,6 +130,7 @@ void EntitySimulation::callUpdateOnEntitiesThatNeedIt(const quint64& now) {
 
 // protected
 void EntitySimulation::sortEntitiesThatMoved() {
+    QMutexLocker lock(&_mutex);
     // NOTE: this is only for entities that have been moved by THIS EntitySimulation.
     // External changes to entity position/shape are expected to be sorted outside of the EntitySimulation.
     PerformanceTimer perfTimer("sortingEntities");

libraries/entities/src/SimpleEntitySimulation.cpp

@@ -23,6 +23,7 @@ void SimpleEntitySimulation::updateEntitiesInternal(const quint64& now) {
     // has finished simulating it.
     auto nodeList = DependencyManager::get<LimitedNodeList>();
 
+    QMutexLocker lock(&_mutex);
     SetOfEntities::iterator itemItr = _entitiesWithSimulator.begin();
     while (itemItr != _entitiesWithSimulator.end()) {
         EntityItemPointer entity = *itemItr;
@@ -48,18 +49,21 @@ void SimpleEntitySimulation::updateEntitiesInternal(const quint64& now) {
 void SimpleEntitySimulation::addEntityInternal(EntityItemPointer entity) {
     EntitySimulation::addEntityInternal(entity);
     if (!entity->getSimulatorID().isNull()) {
+        QMutexLocker lock(&_mutex);
         _entitiesWithSimulator.insert(entity);
     }
 }
 
 void SimpleEntitySimulation::removeEntityInternal(EntityItemPointer entity) {
     EntitySimulation::removeEntityInternal(entity);
+    QMutexLocker lock(&_mutex);
     _entitiesWithSimulator.remove(entity);
 }
 
 void SimpleEntitySimulation::changeEntityInternal(EntityItemPointer entity) {
     EntitySimulation::changeEntityInternal(entity);
     if (!entity->getSimulatorID().isNull()) {
+        QMutexLocker lock(&_mutex);
         _entitiesWithSimulator.insert(entity);
     }
     entity->clearDirtyFlags();

libraries/physics/src/PhysicalEntitySimulation.cpp

@@ -43,6 +43,7 @@ void PhysicalEntitySimulation::updateEntitiesInternal(const quint64& now) {
 }
 
 void PhysicalEntitySimulation::addEntityInternal(EntityItemPointer entity) {
+    QMutexLocker lock(&_mutex);
     assert(entity);
     assert(!entity->isDead());
     if (entity->shouldBePhysical()) {
@@ -57,6 +58,7 @@ void PhysicalEntitySimulation::addEntityInternal(EntityItemPointer entity) {
 
 void PhysicalEntitySimulation::removeEntityInternal(EntityItemPointer entity) {
     EntitySimulation::removeEntityInternal(entity);
+    QMutexLocker lock(&_mutex);
     _entitiesToAddToPhysics.remove(entity);
 
     EntityMotionState* motionState = static_cast<EntityMotionState*>(entity->getPhysicsInfo());
@@ -78,8 +80,7 @@ void PhysicalEntitySimulation::takeEntitiesToDelete(VectorOfEntities& entitiesTo
         // rather than do it here
         EntityMotionState* motionState = static_cast<EntityMotionState*>(entity->getPhysicsInfo());
         if (motionState) {
-            delete motionState;
-            entity->setPhysicsInfo(nullptr);
+            _entitiesToRemoveFromPhysics.insert(entity);
         }
     }
     _entitiesToDelete.clear();
@@ -87,6 +88,7 @@ void PhysicalEntitySimulation::takeEntitiesToDelete(VectorOfEntities& entitiesTo
 
 void PhysicalEntitySimulation::changeEntityInternal(EntityItemPointer entity) {
     // queue incoming changes: from external sources (script, EntityServer, etc) to physics engine
+    QMutexLocker lock(&_mutex);
     assert(entity);
     EntityMotionState* motionState = static_cast<EntityMotionState*>(entity->getPhysicsInfo());
     if (motionState) {
@@ -119,7 +121,7 @@ void PhysicalEntitySimulation::clearEntitiesInternal() {
     // while it is in the middle of a simulation step.  As it is, we're probably in shutdown mode
     // anyway, so maybe the simulation was already properly shutdown?  Cross our fingers...
 
-    // first disconnect each MotionStates from its Entity
+    // copy everything into _entitiesToDelete
     for (auto stateItr : _physicalObjects) {
         EntityMotionState* motionState = static_cast<EntityMotionState*>(&(*stateItr));
         _entitiesToDelete.insert(motionState->getEntity());
@@ -134,8 +136,8 @@ void PhysicalEntitySimulation::clearEntitiesInternal() {
     for (auto entity : _entitiesToDelete) {
         EntityMotionState* motionState = static_cast<EntityMotionState*>(entity->getPhysicsInfo());
         if (motionState) {
-            delete motionState;
             entity->setPhysicsInfo(nullptr);
+            delete motionState;
         }
     }
 
@@ -158,22 +160,39 @@ void PhysicalEntitySimulation::prepareEntityForDelete(EntityItemPointer entity)
 // end EntitySimulation overrides
 
 void PhysicalEntitySimulation::getObjectsToRemoveFromPhysics(VectorOfMotionStates& result) {
-    _entitiesToRelease.clear();
     result.clear();
     QMutexLocker lock(&_mutex);
     for (auto entity: _entitiesToRemoveFromPhysics) {
-        EntityMotionState* motionState = static_cast<EntityMotionState*>(entity->getPhysicsInfo());
-        if (motionState) {
-            _pendingChanges.remove(motionState);
-            _physicalObjects.remove(motionState);
-            result.push_back(motionState);
-        }
+        // make sure it isn't on any side lists
         _entitiesToAddToPhysics.remove(entity);
-        if (entity->isDead()) {
+
+        EntityMotionState* motionState = static_cast<EntityMotionState*>(entity->getPhysicsInfo());
+        assert(motionState);
+        _pendingChanges.remove(motionState);
+        _physicalObjects.remove(motionState);
+        result.push_back(motionState);
+        _entitiesToRelease.insert(entity);
+
+        if (entity->isSimulated() && entity->isDead()) {
             _entitiesToDelete.insert(entity);
         }
     }
-    _entitiesToRemoveFromPhysics.swap(_entitiesToRelease);
+    _entitiesToRemoveFromPhysics.clear();
+}
+
+void PhysicalEntitySimulation::deleteObjectsRemovedFromPhysics() {
+    QMutexLocker lock(&_mutex);
+    for (auto entity: _entitiesToRelease) {
+        EntityMotionState* motionState = static_cast<EntityMotionState*>(entity->getPhysicsInfo());
+        assert(motionState);
+        entity->setPhysicsInfo(nullptr);
+        delete motionState;
+
+        if (entity->isSimulated() && entity->isDead()) {
+            _entitiesToDelete.insert(entity);
+        }
+    }
+    _entitiesToRelease.clear();
 }
 
 void PhysicalEntitySimulation::getObjectsToAddToPhysics(VectorOfMotionStates& result) {

libraries/physics/src/PhysicalEntitySimulation.h

@@ -49,6 +49,7 @@ public:
     virtual void prepareEntityForDelete(EntityItemPointer entity) override;
 
     void getObjectsToRemoveFromPhysics(VectorOfMotionStates& result);
+    void deleteObjectsRemovedFromPhysics();
     void getObjectsToAddToPhysics(VectorOfMotionStates& result);
     void setObjectsToChange(const VectorOfMotionStates& objectsToChange);
     void getObjectsToChange(VectorOfMotionStates& result);