Working on thread safety for the entity tree

libraries/entities-renderer/src/EntityTreeRenderer.cpp

@@ -642,22 +642,14 @@ void EntityTreeRenderer::renderElement(OctreeElement* element, RenderArgs* args)
     // we need to iterate the actual entityItems of the element
     EntityTreeElement* entityTreeElement = static_cast<EntityTreeElement*>(element);
 
-    EntityItems& entityItems = entityTreeElement->getEntities();
-    
-
-    uint16_t numberOfEntities = entityItems.size();
-
     bool isShadowMode = args->_renderMode == RenderArgs::SHADOW_RENDER_MODE;
 
-    if (!isShadowMode && _displayModelElementProxy && numberOfEntities > 0) {
+    if (!isShadowMode && _displayModelElementProxy && entityTreeElement->size() > 0) {
         renderElementProxy(entityTreeElement, args);
     }
 
-    for (uint16_t i = 0; i < numberOfEntities; i++) {
+    entityTreeElement->forEachEntity([&](EntityItemPointer entityItem) {
-        EntityItemPointer entityItem = entityItems[i];
-        
         if (entityItem->isVisible()) {
-
             // NOTE: Zone Entities are a special case we handle here...
             if (entityItem->getType() == EntityTypes::Zone) {
                 if (entityItem->contains(_viewState->getAvatarPosition())) {
@@ -681,7 +673,8 @@ void EntityTreeRenderer::renderElement(OctreeElement* element, RenderArgs* args)
                 }
             }
         }
-    }
+    });
+
 }
 
 float EntityTreeRenderer::getSizeScale() const {

libraries/entities/src/EntityItem.cpp

@@ -1512,7 +1512,7 @@ bool EntityItem::addAction(EntitySimulation* simulation, EntityActionPointer act
     withWriteLock([&] {
         checkWaitingToRemove(simulation);
 
-        bool result = addActionInternal(simulation, action);
+        result = addActionInternal(simulation, action);
         if (!result) {
             removeActionInternal(action->getID());
         }

libraries/entities/src/EntityTree.cpp

@@ -1012,12 +1012,10 @@ QVector<EntityItemID> EntityTree::sendEntities(EntityEditPacketSender* packetSen
 bool EntityTree::sendEntitiesOperation(OctreeElement* element, void* extraData) {
     SendEntitiesOperationArgs* args = static_cast<SendEntitiesOperationArgs*>(extraData);
     EntityTreeElement* entityTreeElement = static_cast<EntityTreeElement*>(element);
-
+    entityTreeElement->forEachEntity([&](EntityItemPointer entityItem) {
-    const EntityItems&  entities = entityTreeElement->getEntities();
-    for (int i = 0; i < entities.size(); i++) {
         EntityItemID newID(QUuid::createUuid());
         args->newEntityIDs->append(newID);
-        EntityItemProperties properties = entities[i]->getProperties();
+        EntityItemProperties properties = entityItem->getProperties();
         properties.setPosition(properties.getPosition() + args->root);
         properties.markAllChanged(); // so the entire property set is considered new, since we're making a new entity
 
@@ -1030,8 +1028,7 @@ bool EntityTree::sendEntitiesOperation(OctreeElement* element, void* extraData)
                 args->localTree->addEntity(newID, properties);
             });
         }
-    }
+    });
-
     return true;
 }
 

libraries/entities/src/EntityTreeElement.cpp

@@ -24,8 +24,6 @@ EntityTreeElement::EntityTreeElement(unsigned char* octalCode) : OctreeElement()
 
 EntityTreeElement::~EntityTreeElement() {
     _octreeMemoryUsage -= sizeof(EntityTreeElement);
-    delete _entityItems;
-    _entityItems = NULL;
 }
 
 // This will be called primarily on addChildAt(), which means we're adding a child of our
@@ -39,7 +37,6 @@ OctreeElement* EntityTreeElement::createNewElement(unsigned char* octalCode) {
 
 void EntityTreeElement::init(unsigned char* octalCode) {
     OctreeElement::init(octalCode);
-    _entityItems = new EntityItems;
     _octreeMemoryUsage += sizeof(EntityTreeElement);
 }
 
@@ -71,7 +68,7 @@ void EntityTreeElement::initializeExtraEncodeData(EncodeBitstreamParams& params)
     // Check to see if this element yet has encode data... if it doesn't create it
     if (!extraEncodeData->contains(this)) {
         EntityTreeElementExtraEncodeData* entityTreeElementExtraEncodeData = new EntityTreeElementExtraEncodeData();
-        entityTreeElementExtraEncodeData->elementCompleted = (_entityItems->size() == 0);
+        entityTreeElementExtraEncodeData->elementCompleted = (_entityItems.size() == 0);
         for (int i = 0; i < NUMBER_OF_CHILDREN; i++) {
             EntityTreeElement* child = getChildAtIndex(i);
             if (!child) {
@@ -84,10 +81,9 @@ void EntityTreeElement::initializeExtraEncodeData(EncodeBitstreamParams& params)
                 }
             }
         }
-        for (uint16_t i = 0; i < _entityItems->size(); i++) {
+        forEachEntity([&](EntityItemPointer entity) {
-            EntityItemPointer entity = (*_entityItems)[i];
             entityTreeElementExtraEncodeData->entities.insert(entity->getEntityItemID(), entity->getEntityProperties(params));
-        }
+        });
         
         // TODO: some of these inserts might be redundant!!!
         extraEncodeData->insert(this, entityTreeElementExtraEncodeData);
@@ -248,7 +244,7 @@ OctreeElement::AppendState EntityTreeElement::appendElementData(OctreePacketData
     } else {
         // if there wasn't one already, then create one
         entityTreeElementExtraEncodeData = new EntityTreeElementExtraEncodeData();
-        entityTreeElementExtraEncodeData->elementCompleted = (_entityItems->size() == 0);
+        entityTreeElementExtraEncodeData->elementCompleted = !hasContent();
 
         for (int i = 0; i < NUMBER_OF_CHILDREN; i++) {
             EntityTreeElement* child = getChildAtIndex(i);
@@ -262,10 +258,9 @@ OctreeElement::AppendState EntityTreeElement::appendElementData(OctreePacketData
                 }
             }
         }
-        for (uint16_t i = 0; i < _entityItems->size(); i++) {
+        forEachEntity([&](EntityItemPointer entity) {
-            EntityItemPointer entity = (*_entityItems)[i];
             entityTreeElementExtraEncodeData->entities.insert(entity->getEntityItemID(), entity->getEntityProperties(params));
-        }
+        });
     }
 
     //assert(extraEncodeData);
@@ -277,81 +272,84 @@ OctreeElement::AppendState EntityTreeElement::appendElementData(OctreePacketData
     // write our entities out... first determine which of the entities are in view based on our params
     uint16_t numberOfEntities = 0;
     uint16_t actualNumberOfEntities = 0;
-    QVector<uint16_t> indexesOfEntitiesToInclude;
+    int numberOfEntitiesOffset = 0;
+    withReadLock([&] {
+        QVector<uint16_t> indexesOfEntitiesToInclude;
 
-    // It's possible that our element has been previous completed. In this case we'll simply not include any of our
+        // It's possible that our element has been previous completed. In this case we'll simply not include any of our
-    // entities for encoding. This is needed because we encode the element data at the "parent" level, and so we 
+        // entities for encoding. This is needed because we encode the element data at the "parent" level, and so we 
-    // need to handle the case where our sibling elements need encoding but we don't.
+        // need to handle the case where our sibling elements need encoding but we don't.
-    if (!entityTreeElementExtraEncodeData->elementCompleted) {
+        if (!entityTreeElementExtraEncodeData->elementCompleted) {
-        for (uint16_t i = 0; i < _entityItems->size(); i++) {
+            for (uint16_t i = 0; i < _entityItems.size(); i++) {
-            EntityItemPointer entity = (*_entityItems)[i];
+                EntityItemPointer entity = _entityItems[i];
-            bool includeThisEntity = true;
+                bool includeThisEntity = true;
 
-            if (!params.forceSendScene && entity->getLastChangedOnServer() < params.lastViewFrustumSent) {
+                if (!params.forceSendScene && entity->getLastChangedOnServer() < params.lastViewFrustumSent) {
-                includeThisEntity = false;
+                    includeThisEntity = false;
-            }
+                }
 
-            if (hadElementExtraData) {
+                if (hadElementExtraData) {
-                includeThisEntity = includeThisEntity && 
+                    includeThisEntity = includeThisEntity &&
-                                        entityTreeElementExtraEncodeData->entities.contains(entity->getEntityItemID());
+                        entityTreeElementExtraEncodeData->entities.contains(entity->getEntityItemID());
-            }
+                }
 
-            if (includeThisEntity && params.viewFrustum) {
+                if (includeThisEntity && params.viewFrustum) {
 
-                // we want to use the maximum possible box for this, so that we don't have to worry about the nuance of
+                    // we want to use the maximum possible box for this, so that we don't have to worry about the nuance of
-                // simulation changing what's visible. consider the case where the entity contains an angular velocity
+                    // simulation changing what's visible. consider the case where the entity contains an angular velocity
-                // the entity may not be in view and then in view a frame later, let the client side handle it's view
+                    // the entity may not be in view and then in view a frame later, let the client side handle it's view
-                // frustum culling on rendering.
+                    // frustum culling on rendering.
-                AACube entityCube = entity->getMaximumAACube();
+                    AACube entityCube = entity->getMaximumAACube();
-                if (params.viewFrustum->cubeInFrustum(entityCube) == ViewFrustum::OUTSIDE) {
+                    if (params.viewFrustum->cubeInFrustum(entityCube) == ViewFrustum::OUTSIDE) {
-                    includeThisEntity = false; // out of view, don't include it
+                        includeThisEntity = false; // out of view, don't include it
+                    }
+                }
+
+                if (includeThisEntity) {
+                    indexesOfEntitiesToInclude << i;
+                    numberOfEntities++;
                 }
             }
-        
-            if (includeThisEntity) {
-                indexesOfEntitiesToInclude << i;
-                numberOfEntities++;
-            }
         }
-    }
 
-    int numberOfEntitiesOffset = packetData->getUncompressedByteOffset();
+        numberOfEntitiesOffset = packetData->getUncompressedByteOffset();
-    bool successAppendEntityCount = packetData->appendValue(numberOfEntities);
+        bool successAppendEntityCount = packetData->appendValue(numberOfEntities);
 
-    if (successAppendEntityCount) {
+        if (successAppendEntityCount) {
-        foreach (uint16_t i, indexesOfEntitiesToInclude) {
+            foreach(uint16_t i, indexesOfEntitiesToInclude) {
-            EntityItemPointer entity = (*_entityItems)[i];
+                EntityItemPointer entity = _entityItems[i];
-            LevelDetails entityLevel = packetData->startLevel();
+                LevelDetails entityLevel = packetData->startLevel();
-            OctreeElement::AppendState appendEntityState = entity->appendEntityData(packetData, 
+                OctreeElement::AppendState appendEntityState = entity->appendEntityData(packetData,
-                                                                        params, entityTreeElementExtraEncodeData);
+                    params, entityTreeElementExtraEncodeData);
 
-            // If none of this entity data was able to be appended, then discard it
+                // If none of this entity data was able to be appended, then discard it
-            // and don't include it in our entity count
+                // and don't include it in our entity count
-            if (appendEntityState == OctreeElement::NONE) {
+                if (appendEntityState == OctreeElement::NONE) {
-                packetData->discardLevel(entityLevel);
+                    packetData->discardLevel(entityLevel);
-            } else {
+                } else {
-                // If either ALL or some of it got appended, then end the level (commit it)
+                    // If either ALL or some of it got appended, then end the level (commit it)
-                // and include the entity in our final count of entities
+                    // and include the entity in our final count of entities
-                packetData->endLevel(entityLevel);
+                    packetData->endLevel(entityLevel);
-                actualNumberOfEntities++;
+                    actualNumberOfEntities++;
-            }
+                }
-            
+
-            // If the entity item got completely appended, then we can remove it from the extra encode data
+                // If the entity item got completely appended, then we can remove it from the extra encode data
-            if (appendEntityState == OctreeElement::COMPLETED) {
+                if (appendEntityState == OctreeElement::COMPLETED) {
-                entityTreeElementExtraEncodeData->entities.remove(entity->getEntityItemID());
+                    entityTreeElementExtraEncodeData->entities.remove(entity->getEntityItemID());
-            }
+                }
-
+
-            // If any part of the entity items didn't fit, then the element is considered partial
+                // If any part of the entity items didn't fit, then the element is considered partial
-            // NOTE: if the entity item didn't fit or only partially fit, then the entity item should have
+                // NOTE: if the entity item didn't fit or only partially fit, then the entity item should have
-            // added itself to the extra encode data.
+                // added itself to the extra encode data.
-            if (appendEntityState != OctreeElement::COMPLETED) {
+                if (appendEntityState != OctreeElement::COMPLETED) {
-                appendElementState = OctreeElement::PARTIAL;
+                    appendElementState = OctreeElement::PARTIAL;
+                }
             }
+        } else {
+            // we we couldn't add the entity count, then we couldn't add anything for this element and we're in a NONE state
+            appendElementState = OctreeElement::NONE;
         }
-    } else {
+    });
-        // we we couldn't add the entity count, then we couldn't add anything for this element and we're in a NONE state
-        appendElementState = OctreeElement::NONE;
-    }
 
     // If we were provided with extraEncodeData, and we allocated and/or got entityTreeElementExtraEncodeData
     // then we need to do some additional processing, namely make sure our extraEncodeData is up to date for
@@ -478,56 +476,41 @@ bool EntityTreeElement::findDetailedRayIntersection(const glm::vec3& origin, con
 
     // only called if we do intersect our bounding cube, but find if we actually intersect with entities...
     int entityNumber = 0;
-    
-    EntityItems::iterator entityItr = _entityItems->begin();
-    EntityItems::const_iterator entityEnd = _entityItems->end();
     bool somethingIntersected = false;
-    
+    forEachEntity([&](EntityItemPointer entity) {
-    //float bestEntityDistance = distance;
-    
-    while(entityItr != entityEnd) {
-        EntityItemPointer entity = (*entityItr);
-        
         AABox entityBox = entity->getAABox();
         float localDistance;
         BoxFace localFace;
 
         // if the ray doesn't intersect with our cube, we can stop searching!
-        if (entityBox.findRayIntersection(origin, direction, localDistance, localFace)) {
+        if (!entityBox.findRayIntersection(origin, direction, localDistance, localFace)) {
+            return;
+        }
 
-            // extents is the entity relative, scaled, centered extents of the entity
+        // extents is the entity relative, scaled, centered extents of the entity
-            glm::mat4 rotation = glm::mat4_cast(entity->getRotation());
+        glm::mat4 rotation = glm::mat4_cast(entity->getRotation());
-            glm::mat4 translation = glm::translate(entity->getPosition());
+        glm::mat4 translation = glm::translate(entity->getPosition());
-            glm::mat4 entityToWorldMatrix = translation * rotation;
+        glm::mat4 entityToWorldMatrix = translation * rotation;
-            glm::mat4 worldToEntityMatrix = glm::inverse(entityToWorldMatrix);
+        glm::mat4 worldToEntityMatrix = glm::inverse(entityToWorldMatrix);
 
-            glm::vec3 dimensions = entity->getDimensions();
+        glm::vec3 dimensions = entity->getDimensions();
-            glm::vec3 registrationPoint = entity->getRegistrationPoint();
+        glm::vec3 registrationPoint = entity->getRegistrationPoint();
-            glm::vec3 corner = -(dimensions * registrationPoint);
+        glm::vec3 corner = -(dimensions * registrationPoint);
 
-            AABox entityFrameBox(corner, dimensions);
+        AABox entityFrameBox(corner, dimensions);
 
-            glm::vec3 entityFrameOrigin = glm::vec3(worldToEntityMatrix * glm::vec4(origin, 1.0f));
+        glm::vec3 entityFrameOrigin = glm::vec3(worldToEntityMatrix * glm::vec4(origin, 1.0f));
-            glm::vec3 entityFrameDirection = glm::vec3(worldToEntityMatrix * glm::vec4(direction, 0.0f));
+        glm::vec3 entityFrameDirection = glm::vec3(worldToEntityMatrix * glm::vec4(direction, 0.0f));
 
-            // we can use the AABox's ray intersection by mapping our origin and direction into the entity frame
+        // we can use the AABox's ray intersection by mapping our origin and direction into the entity frame
-            // and testing intersection there.
+        // and testing intersection there.
-            if (entityFrameBox.findRayIntersection(entityFrameOrigin, entityFrameDirection, localDistance, localFace)) {
+        if (entityFrameBox.findRayIntersection(entityFrameOrigin, entityFrameDirection, localDistance, localFace)) {
-                if (localDistance < distance) {
+            if (localDistance < distance) {
-                    // now ask the entity if we actually intersect
+                // now ask the entity if we actually intersect
-                    if (entity->supportsDetailedRayIntersection()) {
+                if (entity->supportsDetailedRayIntersection()) {
-                        if (entity->findDetailedRayIntersection(origin, direction, keepSearching, element, localDistance, 
+                    if (entity->findDetailedRayIntersection(origin, direction, keepSearching, element, localDistance,
-                                                                    localFace, intersectedObject, precisionPicking)) {
+                        localFace, intersectedObject, precisionPicking)) {
 
-                            if (localDistance < distance) {
-                                distance = localDistance;
-                                face = localFace;
-                                *intersectedObject = (void*)entity.get();
-                                somethingIntersected = true;
-                            }
-                        }
-                    } else {
-                        // if the entity type doesn't support a detailed intersection, then just return the non-AABox results
                         if (localDistance < distance) {
                             distance = localDistance;
                             face = localFace;
@@ -535,75 +518,79 @@ bool EntityTreeElement::findDetailedRayIntersection(const glm::vec3& origin, con
                             somethingIntersected = true;
                         }
                     }
+                } else {
+                    // if the entity type doesn't support a detailed intersection, then just return the non-AABox results
+                    if (localDistance < distance) {
+                        distance = localDistance;
+                        face = localFace;
+                        *intersectedObject = (void*)entity.get();
+                        somethingIntersected = true;
+                    }
                 }
             }
         }
-        
-        ++entityItr;
         entityNumber++;
-    }
+    });
     return somethingIntersected;
 }
 
 // TODO: change this to use better bounding shape for entity than sphere
 bool EntityTreeElement::findSpherePenetration(const glm::vec3& center, float radius,
                                     glm::vec3& penetration, void** penetratedObject) const {
-    EntityItems::iterator entityItr = _entityItems->begin();
+    bool result = false;
-    EntityItems::const_iterator entityEnd = _entityItems->end();
+    withReadLock([&] {
-    while(entityItr != entityEnd) {
+        foreach(EntityItemPointer entity, _entityItems) {
-        EntityItemPointer entity = (*entityItr);
+            glm::vec3 entityCenter = entity->getPosition();
-        glm::vec3 entityCenter = entity->getPosition();
+            float entityRadius = entity->getRadius();
-        float entityRadius = entity->getRadius();
 
-        // don't penetrate yourself
+            // don't penetrate yourself
-        if (entityCenter == center && entityRadius == radius) {
+            if (entityCenter == center && entityRadius == radius) {
-            return false;
+                return;
+            }
+
+            if (findSphereSpherePenetration(center, radius, entityCenter, entityRadius, penetration)) {
+                // return true on first valid entity penetration
+                *penetratedObject = (void*)(entity.get());
+
+                result = true;
+                return;
+            }
         }
-
+    });
-        if (findSphereSpherePenetration(center, radius, entityCenter, entityRadius, penetration)) {
+    return result;
-            // return true on first valid entity penetration
-            
-            *penetratedObject = (void*)(entity.get());
-            
-            return true;
-        }
-        ++entityItr;
-    }
-    return false;
 }
 
 EntityItemPointer EntityTreeElement::getClosestEntity(glm::vec3 position) const {
     EntityItemPointer closestEntity = NULL;
     float closestEntityDistance = FLT_MAX;
-    uint16_t numberOfEntities = _entityItems->size();
+    withReadLock([&] {
-    for (uint16_t i = 0; i < numberOfEntities; i++) {
+        foreach(EntityItemPointer entity, _entityItems) {
-        float distanceToEntity = glm::distance(position, (*_entityItems)[i]->getPosition());
+            float distanceToEntity = glm::distance2(position, entity->getPosition());
-        if (distanceToEntity < closestEntityDistance) {
+            if (distanceToEntity < closestEntityDistance) {
-            closestEntity = (*_entityItems)[i];
+                closestEntity = entity;
+            }
         }
-    }
+    });
     return closestEntity;
 }
 
 // TODO: change this to use better bounding shape for entity than sphere
 void EntityTreeElement::getEntities(const glm::vec3& searchPosition, float searchRadius, QVector<EntityItemPointer>& foundEntities) const {
-    uint16_t numberOfEntities = _entityItems->size();
+    float compareRadius = searchRadius * searchRadius;
-    for (uint16_t i = 0; i < numberOfEntities; i++) {
+    forEachEntity([&](EntityItemPointer entity) {
-        EntityItemPointer entity = (*_entityItems)[i];
+        // For iteration like this, avoid the use of square roots by comparing distances squared
-        float distance = glm::length(entity->getPosition() - searchPosition);
+        float distanceSquared = glm::length2(entity->getPosition() - searchPosition);
-        if (distance < searchRadius + entity->getRadius()) {
+        float otherRadius = entity->getRadius();
+        if (distanceSquared < (compareRadius + (otherRadius * otherRadius))) {
             foundEntities.push_back(entity);
         }
-    }
+    });
 }
 
 // TODO: change this to use better bounding shape for entity than sphere
 void EntityTreeElement::getEntities(const AACube& box, QVector<EntityItemPointer>& foundEntities) {
-    EntityItems::iterator entityItr = _entityItems->begin();
-    EntityItems::iterator entityEnd = _entityItems->end();
     AACube entityCube;
-    while(entityItr != entityEnd) {
+    forEachEntity([&](EntityItemPointer entity) {
-        EntityItemPointer entity = (*entityItr);
         float radius = entity->getRadius();
         // NOTE: we actually do cube-cube collision queries here, which is sloppy but good enough for now
         // TODO: decide whether to replace entityCube-cube query with sphere-cube (requires a square root
@@ -612,64 +599,57 @@ void EntityTreeElement::getEntities(const AACube& box, QVector<EntityItemPointer
         if (entityCube.touches(box)) {
             foundEntities.push_back(entity);
         }
-        ++entityItr;
+    });
-    }
 }
 
 EntityItemPointer EntityTreeElement::getEntityWithEntityItemID(const EntityItemID& id) const {
     EntityItemPointer foundEntity = NULL;
-    uint16_t numberOfEntities = _entityItems->size();
+    withReadLock([&] {
-    for (uint16_t i = 0; i < numberOfEntities; i++) {
+        foreach(EntityItemPointer entity, _entityItems) {
-        if ((*_entityItems)[i]->getEntityItemID() == id) {
+            if (entity->getEntityItemID() == id) {
-            foundEntity = (*_entityItems)[i];
+                foundEntity = entity;
-            break;
+                break;
+            }
         }
-    }
+    });
-    return foundEntity;
-}
-   
-EntityItemPointer EntityTreeElement::getEntityWithEntityItemID(const EntityItemID& id) {
-    EntityItemPointer foundEntity = NULL;
-    uint16_t numberOfEntities = _entityItems->size();
-    for (uint16_t i = 0; i < numberOfEntities; i++) {
-        if ((*_entityItems)[i]->getEntityItemID() == id) {
-            foundEntity = (*_entityItems)[i];
-            break;
-        }
-    }
     return foundEntity;
 }
 
 void EntityTreeElement::cleanupEntities() {
-    uint16_t numberOfEntities = _entityItems->size();
+    withWriteLock([&] {
-    for (uint16_t i = 0; i < numberOfEntities; i++) {
+        foreach(EntityItemPointer entity, _entityItems) {
-        EntityItemPointer entity = (*_entityItems)[i];
+            // NOTE: We explicitly don't delete the EntityItem here because since we only
-        entity->_element = NULL;
+            // access it by smart pointers, when we remove it from the _entityItems
-        
+            // we know that it will be deleted.
-        // NOTE: We explicitly don't delete the EntityItem here because since we only
+            //delete entity;
-        // access it by smart pointers, when we remove it from the _entityItems
+            entity->_element = NULL;
-        // we know that it will be deleted.
+        }
-        //delete entity;
+        _entityItems.clear();
-    }
+    });
-    _entityItems->clear();
 }
 
 bool EntityTreeElement::removeEntityWithEntityItemID(const EntityItemID& id) {
     bool foundEntity = false;
-    uint16_t numberOfEntities = _entityItems->size();
+    withWriteLock([&] {
-    for (uint16_t i = 0; i < numberOfEntities; i++) {
+        uint16_t numberOfEntities = _entityItems.size();
-        if ((*_entityItems)[i]->getEntityItemID() == id) {
+        for (uint16_t i = 0; i < numberOfEntities; i++) {
-            foundEntity = true;
+            EntityItemPointer& entity = _entityItems[i];
-            (*_entityItems)[i]->_element = NULL;
+            if (entity->getEntityItemID() == id) {
-            _entityItems->removeAt(i);
+                foundEntity = true;
-            break;
+                entity->_element = NULL;
+                _entityItems.removeAt(i);
+                break;
+            }
         }
-    }
+    });
     return foundEntity;
 }
 
 bool EntityTreeElement::removeEntityItem(EntityItemPointer entity) {
-    int numEntries = _entityItems->removeAll(entity);
+    int numEntries = 0;
+    withWriteLock([&] {
+        numEntries = _entityItems.removeAll(entity);
+    });
     if (numEntries > 0) {
         assert(entity->_element == this);
         entity->_element = NULL;
@@ -791,7 +771,9 @@ int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int
 void EntityTreeElement::addEntityItem(EntityItemPointer entity) {
     assert(entity);
     assert(entity->_element == NULL);
-    _entityItems->push_back(entity);
+    withWriteLock([&] {
+        _entityItems.push_back(entity);
+    });
     entity->_element = this;
 }
 
@@ -824,30 +806,39 @@ bool EntityTreeElement::pruneChildren() {
 }
 
 void EntityTreeElement::expandExtentsToContents(Extents& extents) {
-    if (_entityItems->size()) {
+    withReadLock([&] {
-        for (uint16_t i = 0; i < _entityItems->size(); i++) {
+        foreach(EntityItemPointer entity, _entityItems) {
-            EntityItemPointer entity = (*_entityItems)[i];
             extents.add(entity->getAABox());
         }
-    }
+    });
 }
 
+uint16_t EntityTreeElement::size() const {
+    uint16_t result = 0;
+    withReadLock([&] {
+        result = _entityItems.size();
+    });
+    return result;
+}
 
 
 void EntityTreeElement::debugDump() {
     qCDebug(entities) << "EntityTreeElement...";
     qCDebug(entities) << "    cube:" << _cube;
     qCDebug(entities) << "    has child elements:" << getChildCount();
-    if (_entityItems->size()) {
+
-        qCDebug(entities) << "    has entities:" << _entityItems->size();
+    withReadLock([&] {
-        qCDebug(entities) << "--------------------------------------------------";
+        if (_entityItems.size()) {
-        for (uint16_t i = 0; i < _entityItems->size(); i++) {
+            qCDebug(entities) << "    has entities:" << _entityItems.size();
-            EntityItemPointer entity = (*_entityItems)[i];
+            qCDebug(entities) << "--------------------------------------------------";
-            entity->debugDump();
+            for (uint16_t i = 0; i < _entityItems.size(); i++) {
+                EntityItemPointer entity = _entityItems[i];
+                entity->debugDump();
+            }
+            qCDebug(entities) << "--------------------------------------------------";
+        } else {
+            qCDebug(entities) << "    NO entities!";
         }
-        qCDebug(entities) << "--------------------------------------------------";
+    });
-    } else {
-        qCDebug(entities) << "    NO entities!";
-    }
 }
     

libraries/entities/src/EntityTreeElement.h

@@ -74,7 +74,7 @@ public:
 };
 
 
-class EntityTreeElement : public OctreeElement {
+class EntityTreeElement : public OctreeElement, ReadWriteLockable {
     friend class EntityTree; // to allow createElement to new us...
 
     EntityTreeElement(unsigned char* octalCode = NULL);
@@ -146,10 +146,18 @@ public:
     virtual bool findSpherePenetration(const glm::vec3& center, float radius,
                         glm::vec3& penetration, void** penetratedObject) const;
 
-    const EntityItems& getEntities() const { return *_entityItems; }
-    EntityItems& getEntities() { return *_entityItems; }
 
-    bool hasEntities() const { return _entityItems ? _entityItems->size() > 0 : false; }
+    template <typename F>
+    void forEachEntity(F f) const {
+        withReadLock([&] {
+            foreach(EntityItemPointer entityItem, _entityItems) {
+                f(entityItem);
+            }
+        });
+    }
+
+    virtual uint16_t size() const;
+    bool hasEntities() const { return size() > 0; }
 
     void setTree(EntityTree* tree) { _myTree = tree; }
     EntityTree* getTree() const { return _myTree; }
@@ -174,8 +182,6 @@ public:
     EntityItemPointer getEntityWithEntityItemID(const EntityItemID& id) const;
     void getEntitiesInside(const AACube& box, QVector<EntityItemPointer>& foundEntities);
 
-    EntityItemPointer getEntityWithEntityItemID(const EntityItemID& id);
-
     void cleanupEntities(); /// called by EntityTree on cleanup this will free all entities
     bool removeEntityWithEntityItemID(const EntityItemID& id);
     bool removeEntityItem(EntityItemPointer entity);
@@ -204,7 +210,7 @@ public:
 protected:
     virtual void init(unsigned char * octalCode);
     EntityTree* _myTree;
-    EntityItems* _entityItems;
+    EntityItems _entityItems;
 };
 
 #endif // hifi_EntityTreeElement_h

libraries/entities/src/RecurseOctreeToMapOperator.cpp

@@ -43,11 +43,9 @@ bool RecurseOctreeToMapOperator::postRecursion(OctreeElement* element) {
     EntityItemProperties defaultProperties;
 
     EntityTreeElement* entityTreeElement = static_cast<EntityTreeElement*>(element);
-    const EntityItems& entities = entityTreeElement->getEntities();
-
     QVariantList entitiesQList = qvariant_cast<QVariantList>(_map["Entities"]);
 
-    foreach (EntityItemPointer entityItem, entities) {
+    entityTreeElement->forEachEntity([&](EntityItemPointer entityItem) {
         EntityItemProperties properties = entityItem->getProperties();
         QScriptValue qScriptValues;
         if (_skipDefaultValues) {
@@ -56,7 +54,8 @@ bool RecurseOctreeToMapOperator::postRecursion(OctreeElement* element) {
             qScriptValues = EntityItemPropertiesToScriptValue(_engine, properties);
         }
         entitiesQList << qScriptValues.toVariant();
-    }
+    });
+
     _map["Entities"] = entitiesQList;
     if (element == _top) {
         _withinTop = false;