avoid mem-leaked entities when clearing tree

This commit is contained in:
Andrew Meadows 2019-02-28 14:21:35 -08:00
parent ed9ebf84c9
commit 33d6e2ce3b

View file

@ -52,7 +52,15 @@ EntityTree::EntityTree(bool shouldReaverage) :
} }
EntityTree::~EntityTree() { EntityTree::~EntityTree() {
eraseAllOctreeElements(false); // NOTE: to eraseAllOctreeElements() in this context is useless because
// any OctreeElements in the tree still have shared backpointers to this Tree
// which means the dtor never would have been called in the first place!
//
// I'm keeping this useless commented-out line to remind us:
// we don't need shared pointer overhead for EntityTrees.
// TODO: EntityTreeElement::_tree should be raw back pointer.
// AND: EntityItem::_element should be a raw back pointer.
//eraseAllOctreeElements(false); // KEEP THIS
} }
void EntityTree::setEntityScriptSourceWhitelist(const QString& entityScriptSourceWhitelist) { void EntityTree::setEntityScriptSourceWhitelist(const QString& entityScriptSourceWhitelist) {
@ -74,16 +82,15 @@ void EntityTree::eraseNonLocalEntities() {
emit clearingEntities(); emit clearingEntities();
if (_simulation) { if (_simulation) {
// This will clear all entities host types including local entities, because local entities // local entities are not in the simulation, so we clear ALL
// are not in the physics simulation
_simulation->clearEntities(); _simulation->clearEntities();
} }
_staleProxies.clear();
QHash<EntityItemID, EntityItemPointer> localMap;
localMap.swap(_entityMap);
QHash<EntityItemID, EntityItemPointer> savedEntities;
this->withWriteLock([&] { this->withWriteLock([&] {
foreach(EntityItemPointer entity, localMap) { QHash<EntityItemID, EntityItemPointer> savedEntities;
// NOTE: lock the Tree first, then lock the _entityMap.
// It should never be done the other way around.
QReadLocker locker(&_entityMapLock);
foreach(EntityItemPointer entity, _entityMap) {
EntityTreeElementPointer element = entity->getElement(); EntityTreeElementPointer element = entity->getElement();
if (element) { if (element) {
element->cleanupNonLocalEntities(); element->cleanupNonLocalEntities();
@ -91,11 +98,16 @@ void EntityTree::eraseNonLocalEntities() {
if (entity->isLocalEntity()) { if (entity->isLocalEntity()) {
savedEntities[entity->getEntityItemID()] = entity; savedEntities[entity->getEntityItemID()] = entity;
} else {
int32_t spaceIndex = entity->getSpaceIndex();
if (spaceIndex != -1) {
// stale spaceIndices will be freed later
_staleProxies.push_back(spaceIndex);
}
} }
} }
_entityMap.swap(savedEntities);
}); });
localMap.clear();
_entityMap = savedEntities;
resetClientEditStats(); resetClientEditStats();
clearDeletedEntities(); clearDeletedEntities();
@ -113,13 +125,13 @@ void EntityTree::eraseNonLocalEntities() {
_needsParentFixup = localEntitiesNeedsParentFixup; _needsParentFixup = localEntitiesNeedsParentFixup;
} }
} }
void EntityTree::eraseAllOctreeElements(bool createNewRoot) { void EntityTree::eraseAllOctreeElements(bool createNewRoot) {
emit clearingEntities(); emit clearingEntities();
if (_simulation) { if (_simulation) {
_simulation->clearEntities(); _simulation->clearEntities();
} }
_staleProxies.clear();
QHash<EntityItemID, EntityItemPointer> localMap; QHash<EntityItemID, EntityItemPointer> localMap;
localMap.swap(_entityMap); localMap.swap(_entityMap);
this->withWriteLock([&] { this->withWriteLock([&] {
@ -128,6 +140,11 @@ void EntityTree::eraseAllOctreeElements(bool createNewRoot) {
if (element) { if (element) {
element->cleanupEntities(); element->cleanupEntities();
} }
int32_t spaceIndex = entity->getSpaceIndex();
if (spaceIndex != -1) {
// assume stale spaceIndices will be freed later
_staleProxies.push_back(spaceIndex);
}
} }
}); });
localMap.clear(); localMap.clear();
@ -767,6 +784,7 @@ void EntityTree::processRemovedEntities(const DeleteEntityOperator& theOperator)
// keep a record of valid stale spaceIndices so they can be removed from the Space // keep a record of valid stale spaceIndices so they can be removed from the Space
int32_t spaceIndex = theEntity->getSpaceIndex(); int32_t spaceIndex = theEntity->getSpaceIndex();
if (spaceIndex != -1) { if (spaceIndex != -1) {
// stale spaceIndices will be freed later
_staleProxies.push_back(spaceIndex); _staleProxies.push_back(spaceIndex);
} }
} }