Merge pull request #435 from HifiExperiments/model

Possible fix for entity server crash

assignment-client/src/entities/EntityServer.cpp

@@ -370,16 +370,18 @@ void EntityServer::entityFilterAdded(EntityItemID id, bool success) {
 
 void EntityServer::nodeAdded(SharedNodePointer node) {
     EntityTreePointer tree = std::static_pointer_cast<EntityTree>(_tree);
-    tree->knowAvatarID(node->getUUID());
+    if (tree) {
+        tree->knowAvatarID(node->getUUID());
+    }
     OctreeServer::nodeAdded(node);
 }
 
 void EntityServer::nodeKilled(SharedNodePointer node) {
     EntityTreePointer tree = std::static_pointer_cast<EntityTree>(_tree);
-    tree->withWriteLock([&] {
+    if (tree) {
         tree->deleteDescendantsOfAvatar(node->getUUID());
-    });
+        tree->forgetAvatarID(node->getUUID());
-    tree->forgetAvatarID(node->getUUID());
+    }
     OctreeServer::nodeKilled(node);
 }
 

libraries/entities/src/EntityTree.cpp

@@ -2233,15 +2233,24 @@ void EntityTree::fixupNeedsParentFixups() {
             }
 
             entity->postParentFixup();
-        } else if (getIsServer() || _avatarIDs.contains(entity->getParentID())) {
+        } else {
-            // this is a child of an avatar, which the entity server will never have
+            bool needsUpdate = getIsServer();
-            // a SpatiallyNestable object for.  Add it to a list for cleanup when the avatar leaves.
+            if (!needsUpdate) {
-            if (!_childrenOfAvatars.contains(entity->getParentID())) {
+                std::lock_guard<std::mutex> lock(_avatarIDsLock);
-                _childrenOfAvatars[entity->getParentID()] = QSet<EntityItemID>();
+                needsUpdate = _avatarIDs.contains(entity->getParentID());
+            }
+
+            if (needsUpdate) {
+                std::lock_guard<std::mutex> lock(_childrenOfAvatarsLock);
+                // this is a child of an avatar, which the entity server will never have
+                // a SpatiallyNestable object for.  Add it to a list for cleanup when the avatar leaves.
+                if (!_childrenOfAvatars.contains(entity->getParentID())) {
+                    _childrenOfAvatars[entity->getParentID()] = QSet<EntityItemID>();
+                }
+                _childrenOfAvatars[entity->getParentID()] += entity->getEntityItemID();
+                doMove = true;
+                iter.remove(); // and pull it out of the list
             }
-            _childrenOfAvatars[entity->getParentID()] += entity->getEntityItemID();
-            doMove = true;
-            iter.remove(); // and pull it out of the list
         }
 
         if (queryAACubeSuccess && doMove) {
@@ -2261,8 +2270,19 @@ void EntityTree::fixupNeedsParentFixups() {
     }
 }
 
-void EntityTree::deleteDescendantsOfAvatar(QUuid avatarID) {
+void EntityTree::knowAvatarID(const QUuid& avatarID) {
-    QHash<QUuid, QSet<EntityItemID>>::const_iterator itr = _childrenOfAvatars.constFind(avatarID);
+    std::lock_guard<std::mutex> lock(_avatarIDsLock);
+    _avatarIDs += avatarID;
+}
+
+void EntityTree::forgetAvatarID(const QUuid& avatarID) {
+    std::lock_guard<std::mutex> lock(_avatarIDsLock);
+    _avatarIDs -= avatarID;
+}
+
+void EntityTree::deleteDescendantsOfAvatar(const QUuid& avatarID) {
+    std::lock_guard<std::mutex> lock(_childrenOfAvatarsLock);
+    auto itr = _childrenOfAvatars.find(avatarID);
     if (itr != _childrenOfAvatars.end()) {
         if (!itr.value().empty()) {
             std::vector<EntityItemID> ids;
@@ -2280,8 +2300,10 @@ void EntityTree::deleteDescendantsOfAvatar(QUuid avatarID) {
 
 void EntityTree::removeFromChildrenOfAvatars(EntityItemPointer entity) {
     QUuid avatarID = entity->getParentID();
-    if (_childrenOfAvatars.contains(avatarID)) {
+    std::lock_guard<std::mutex> lock(_childrenOfAvatarsLock);
-        _childrenOfAvatars[avatarID].remove(entity->getID());
+    auto itr = _childrenOfAvatars.find(avatarID);
+    if (itr != _childrenOfAvatars.end()) {
+        itr.value().remove(entity->getID());
     }
 }
 

libraries/entities/src/EntityTree.h

@@ -240,9 +240,9 @@ public:
     Q_INVOKABLE int getJointIndex(const QUuid& entityID, const QString& name) const;
     Q_INVOKABLE QStringList getJointNames(const QUuid& entityID) const;
 
-    void knowAvatarID(QUuid avatarID) { _avatarIDs += avatarID; }
+    void knowAvatarID(const QUuid& avatarID);
-    void forgetAvatarID(QUuid avatarID) { _avatarIDs -= avatarID; }
+    void forgetAvatarID(const QUuid& avatarID);
-    void deleteDescendantsOfAvatar(QUuid avatarID);
+    void deleteDescendantsOfAvatar(const QUuid& avatarID);
     void removeFromChildrenOfAvatars(EntityItemPointer entity);
 
     void addToNeedsParentFixupList(EntityItemPointer entity);
@@ -365,8 +365,10 @@ protected:
     QVector<EntityItemWeakPointer> _needsParentFixup; // entites with a parentID but no (yet) known parent instance
     mutable QReadWriteLock _needsParentFixupLock;
 
+    std::mutex _avatarIDsLock;
     // we maintain a list of avatarIDs to notice when an entity is a child of one.
     QSet<QUuid> _avatarIDs; // IDs of avatars connected to entity server
+    std::mutex _childrenOfAvatarsLock;
     QHash<QUuid, QSet<EntityItemID>> _childrenOfAvatars;  // which entities are children of which avatars
 
     float _maxTmpEntityLifetime { DEFAULT_MAX_TMP_ENTITY_LIFETIME };