From 68fd9eafa8c3fd7409d410750be1cae162ec89b1 Mon Sep 17 00:00:00 2001 From: Dante Ruiz Date: Mon, 29 Jan 2018 10:23:24 -0800 Subject: [PATCH 1/3] fix entity parenting crash --- libraries/entities/src/EntityTree.cpp | 15 +++++++++++++-- libraries/entities/src/EntityTree.h | 2 ++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 15fc3256bd..91694c86aa 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1678,14 +1678,23 @@ void EntityTree::entityChanged(EntityItemPointer entity) { } } +QVector EntityTree::getEntitiesParentFixup() const { + QReadLocker locker(&_needsParentFixupLock); + return _needsParentFixup; +} + +void EntityTree::setNeedsParentFixup(QVector entitiesFixup) { + QWriteLocker locker(&_needsParentFixupLock); + _needsParentFixup = entitiesFixup; +} void EntityTree::fixupNeedsParentFixups() { PROFILE_RANGE(simulation_physics, "FixupParents"); MovingEntitiesOperator moveOperator; - QWriteLocker locker(&_needsParentFixupLock); + QVector entitiesParentFixup = getEntitiesParentFixup(); - QMutableVectorIterator iter(_needsParentFixup); + QMutableVectorIterator iter(entitiesParentFixup); while (iter.hasNext()) { EntityItemWeakPointer entityWP = iter.next(); EntityItemPointer entity = entityWP.lock(); @@ -1748,6 +1757,8 @@ void EntityTree::fixupNeedsParentFixups() { PerformanceTimer perfTimer("recurseTreeWithOperator"); recurseTreeWithOperator(&moveOperator); } + + setNeedsParentFixup(entitiesParentFixup); } void EntityTree::deleteDescendantsOfAvatar(QUuid avatarID) { diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 11a747d624..fa44fd37b8 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -385,6 +385,8 @@ private: void sendChallengeOwnershipPacket(const QString& certID, const QString& ownerKey, const EntityItemID& entityItemID, const SharedNodePointer& senderNode); void sendChallengeOwnershipRequestPacket(const QByteArray& certID, const QByteArray& text, const QByteArray& nodeToChallenge, const SharedNodePointer& senderNode); void validatePop(const QString& certID, const EntityItemID& entityItemID, const SharedNodePointer& senderNode, bool isRetryingValidation); + QVector getEntitiesParentFixup() const; + void setNeedsParentFixup(QVector entitiesFixup); std::shared_ptr _myAvatar{ nullptr }; }; From bed2ea052df8dbe4f08b501dd0646b0eeb905d7a Mon Sep 17 00:00:00 2001 From: Dante Ruiz Date: Tue, 30 Jan 2018 09:44:49 -0800 Subject: [PATCH 2/3] better solution --- libraries/entities/src/EntityTree.cpp | 18 +++--------------- libraries/entities/src/EntityTree.h | 2 -- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 8dad126d30..f485f4121e 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -65,7 +65,7 @@ public: EntityTree::EntityTree(bool shouldReaverage) : - Octree(shouldReaverage) + Octree(shouldReaverage), _needsParentFixupLock(QReadWriteLock::Recursive) { resetClientEditStats(); @@ -1679,23 +1679,13 @@ void EntityTree::entityChanged(EntityItemPointer entity) { } } -QVector EntityTree::getEntitiesParentFixup() const { - QReadLocker locker(&_needsParentFixupLock); - return _needsParentFixup; -} - -void EntityTree::setNeedsParentFixup(QVector entitiesFixup) { - QWriteLocker locker(&_needsParentFixupLock); - _needsParentFixup = entitiesFixup; -} - void EntityTree::fixupNeedsParentFixups() { PROFILE_RANGE(simulation_physics, "FixupParents"); MovingEntitiesOperator moveOperator; - QVector entitiesParentFixup = getEntitiesParentFixup(); + QWriteLocker locker(&_needsParentFixupLock); - QMutableVectorIterator iter(entitiesParentFixup); + QMutableVectorIterator iter(_needsParentFixup); while (iter.hasNext()) { EntityItemWeakPointer entityWP = iter.next(); EntityItemPointer entity = entityWP.lock(); @@ -1758,8 +1748,6 @@ void EntityTree::fixupNeedsParentFixups() { PerformanceTimer perfTimer("recurseTreeWithOperator"); recurseTreeWithOperator(&moveOperator); } - - setNeedsParentFixup(entitiesParentFixup); } void EntityTree::deleteDescendantsOfAvatar(QUuid avatarID) { diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index f4ff7ad118..8cb89d6493 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -385,8 +385,6 @@ private: void sendChallengeOwnershipPacket(const QString& certID, const QString& ownerKey, const EntityItemID& entityItemID, const SharedNodePointer& senderNode); void sendChallengeOwnershipRequestPacket(const QByteArray& certID, const QByteArray& text, const QByteArray& nodeToChallenge, const SharedNodePointer& senderNode); void validatePop(const QString& certID, const EntityItemID& entityItemID, const SharedNodePointer& senderNode, bool isRetryingValidation); - QVector getEntitiesParentFixup() const; - void setNeedsParentFixup(QVector entitiesFixup); std::shared_ptr _myAvatar{ nullptr }; }; From 4c0a1732874c205d3235f1ae28536c8b648f31d7 Mon Sep 17 00:00:00 2001 From: Dante Ruiz Date: Tue, 30 Jan 2018 10:23:19 -0800 Subject: [PATCH 3/3] trying another solution --- libraries/entities/src/EntityTree.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index f485f4121e..bf29f3bec9 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -65,7 +65,7 @@ public: EntityTree::EntityTree(bool shouldReaverage) : - Octree(shouldReaverage), _needsParentFixupLock(QReadWriteLock::Recursive) + Octree(shouldReaverage) { resetClientEditStats(); @@ -1682,10 +1682,14 @@ void EntityTree::entityChanged(EntityItemPointer entity) { void EntityTree::fixupNeedsParentFixups() { PROFILE_RANGE(simulation_physics, "FixupParents"); MovingEntitiesOperator moveOperator; + QVector entitiesToFixup; + { + QWriteLocker locker(&_needsParentFixupLock); + entitiesToFixup = _needsParentFixup; + _needsParentFixup.clear(); + } - QWriteLocker locker(&_needsParentFixupLock); - - QMutableVectorIterator iter(_needsParentFixup); + QMutableVectorIterator iter(entitiesToFixup); while (iter.hasNext()) { EntityItemWeakPointer entityWP = iter.next(); EntityItemPointer entity = entityWP.lock(); @@ -1748,6 +1752,12 @@ void EntityTree::fixupNeedsParentFixups() { PerformanceTimer perfTimer("recurseTreeWithOperator"); recurseTreeWithOperator(&moveOperator); } + + { + QWriteLocker locker(&_needsParentFixupLock); + // add back the entities that did not get fixup + _needsParentFixup.append(entitiesToFixup); + } } void EntityTree::deleteDescendantsOfAvatar(QUuid avatarID) {