use local copy of _element for thread safety

This commit is contained in:
Andrew Meadows 2017-07-03 10:59:41 -07:00
parent 8fc4d1f43e
commit c7ec82f98a
2 changed files with 10 additions and 5 deletions

View file

@ -89,7 +89,8 @@ EntityItem::EntityItem(const EntityItemID& entityItemID) :
EntityItem::~EntityItem() { EntityItem::~EntityItem() {
// clear out any left-over actions // clear out any left-over actions
EntityTreePointer entityTree = _element ? _element->getTree() : nullptr; EntityTreeElementPointer element = _element; // use local copy of _element for logic below
EntityTreePointer entityTree = element ? element->getTree() : nullptr;
EntitySimulationPointer simulation = entityTree ? entityTree->getSimulation() : nullptr; EntitySimulationPointer simulation = entityTree ? entityTree->getSimulation() : nullptr;
if (simulation) { if (simulation) {
clearActions(simulation); clearActions(simulation);
@ -880,8 +881,9 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
// Tracking for editing roundtrips here. We will tell our EntityTree that we just got incoming data about // Tracking for editing roundtrips here. We will tell our EntityTree that we just got incoming data about
// and entity that was edited at some time in the past. The tree will determine how it wants to track this // and entity that was edited at some time in the past. The tree will determine how it wants to track this
// information. // information.
if (_element && _element->getTree()) { EntityTreeElementPointer element = _element; // use local copy of _element for logic below
_element->getTree()->trackIncomingEntityLastEdited(lastEditedFromBufferAdjusted, bytesRead); if (element && element->getTree()) {
element->getTree()->trackIncomingEntityLastEdited(lastEditedFromBufferAdjusted, bytesRead);
} }
@ -2056,7 +2058,8 @@ bool EntityItem::removeActionInternal(const QUuid& actionID, EntitySimulationPoi
_previouslyDeletedActions.insert(actionID, usecTimestampNow()); _previouslyDeletedActions.insert(actionID, usecTimestampNow());
if (_objectActions.contains(actionID)) { if (_objectActions.contains(actionID)) {
if (!simulation) { if (!simulation) {
EntityTreePointer entityTree = _element ? _element->getTree() : nullptr; EntityTreeElementPointer element = _element; // use local copy of _element for logic below
EntityTreePointer entityTree = element ? element->getTree() : nullptr;
simulation = entityTree ? entityTree->getSimulation() : nullptr; simulation = entityTree ? entityTree->getSimulation() : nullptr;
} }

View file

@ -885,10 +885,10 @@ EntityItemPointer EntityTreeElement::getEntityWithEntityItemID(const EntityItemI
void EntityTreeElement::cleanupEntities() { void EntityTreeElement::cleanupEntities() {
withWriteLock([&] { withWriteLock([&] {
foreach(EntityItemPointer entity, _entityItems) { foreach(EntityItemPointer entity, _entityItems) {
// NOTE: only EntityTreeElement should ever be changing the value of entity->_element
// NOTE: We explicitly don't delete the EntityItem here because since we only // NOTE: We explicitly don't delete the EntityItem here because since we only
// access it by smart pointers, when we remove it from the _entityItems // access it by smart pointers, when we remove it from the _entityItems
// we know that it will be deleted. // we know that it will be deleted.
//delete entity;
entity->_element = NULL; entity->_element = NULL;
} }
_entityItems.clear(); _entityItems.clear();
@ -903,6 +903,7 @@ bool EntityTreeElement::removeEntityWithEntityItemID(const EntityItemID& id) {
EntityItemPointer& entity = _entityItems[i]; EntityItemPointer& entity = _entityItems[i];
if (entity->getEntityItemID() == id) { if (entity->getEntityItemID() == id) {
foundEntity = true; foundEntity = true;
// NOTE: only EntityTreeElement should ever be changing the value of entity->_element
entity->_element = NULL; entity->_element = NULL;
_entityItems.removeAt(i); _entityItems.removeAt(i);
break; break;
@ -918,6 +919,7 @@ bool EntityTreeElement::removeEntityItem(EntityItemPointer entity) {
numEntries = _entityItems.removeAll(entity); numEntries = _entityItems.removeAll(entity);
}); });
if (numEntries > 0) { if (numEntries > 0) {
// NOTE: only EntityTreeElement should ever be changing the value of entity->_element
assert(entity->_element.get() == this); assert(entity->_element.get() == this);
entity->_element = NULL; entity->_element = NULL;
return true; return true;