From 6088612ce06fc4d12543dba0b4dafd8c3888d4fc Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 8 Apr 2016 13:16:30 -0700 Subject: [PATCH] guard access to _missingParent vector with a lock to allow safe access from multiple threads --- libraries/entities/src/EntityTree.cpp | 33 +++++++++++++++++++++------ libraries/entities/src/EntityTree.h | 2 ++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 92c3ce216f..248ae48f6e 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -90,6 +90,7 @@ void EntityTree::postAddEntity(EntityItemPointer entity) { } if (!entity->isParentIDValid()) { + QWriteLocker locker(&_missingParentLock); _missingParent.append(entity); } @@ -256,10 +257,12 @@ bool EntityTree::updateEntityWithElement(EntityItemPointer entity, const EntityI bool success; AACube queryCube = childEntity->getQueryAACube(success); if (!success) { + QWriteLocker locker(&_missingParentLock); _missingParent.append(childEntity); continue; } if (!childEntity->isParentIDValid()) { + QWriteLocker locker(&_missingParentLock); _missingParent.append(childEntity); } @@ -346,6 +349,7 @@ EntityItemPointer EntityTree::addEntity(const EntityItemID& entityID, const Enti if (result->getAncestorMissing()) { // we added the entity, but didn't know about all its ancestors, so it went into the wrong place. // add it to a list of entities needing to be fixed once their parents are known. + QWriteLocker locker(&_missingParentLock); _missingParent.append(result); } @@ -1008,10 +1012,29 @@ void EntityTree::entityChanged(EntityItemPointer entity) { void EntityTree::fixupMissingParents() { MovingEntitiesOperator moveOperator(getThisPointer()); - QMutableVectorIterator iter(_missingParent); + QList missingParents; + { + QWriteLocker locker(&_missingParentLock); + QMutableVectorIterator iter(_missingParent); + while (iter.hasNext()) { + EntityItemWeakPointer entityWP = iter.next(); + EntityItemPointer entity = entityWP.lock(); + if (entity) { + if (entity->isParentIDValid()) { + iter.remove(); + } else { + missingParents.append(entity); + } + } else { + // entity was deleted before we found its parent. + iter.remove(); + } + } + } + + QListIterator iter(missingParents); while (iter.hasNext()) { - EntityItemWeakPointer entityWP = iter.next(); - EntityItemPointer entity = entityWP.lock(); + EntityItemPointer entity = iter.next(); if (entity) { bool queryAACubeSuccess; AACube newCube = entity->getQueryAACube(queryAACubeSuccess); @@ -1040,12 +1063,8 @@ void EntityTree::fixupMissingParents() { if (queryAACubeSuccess && doMove) { moveOperator.addEntityToMoveList(entity, newCube); - iter.remove(); entity->markAncestorMissing(false); } - } else { - // entity was deleted before we found its parent. - iter.remove(); } } diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 5c5f5b4eb9..1937bf6daf 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -324,6 +324,8 @@ protected: void fixupMissingParents(); // try to hook members of _missingParent to parent instances QVector _missingParent; // entites with a parentID but no (yet) known parent instance + mutable QReadWriteLock _missingParentLock; + // we maintain a list of avatarIDs to notice when an entity is a child of one. QSet _avatarIDs; // IDs of avatars connected to entity server QHash> _childrenOfAvatars; // which entities are children of which avatars