From fa491a5c4f013c0278d03199f5ff2339cb09919e Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Mon, 1 Jun 2015 13:59:56 -0700
Subject: [PATCH] fix theoretical crash bug in editEntity()

---
 .../entities/src/EntityScriptingInterface.cpp | 43 ++++++++++---------
 .../entities/src/EntityScriptingInterface.h   |  2 +-
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp
index d684fbf2fc..36cc8dc07a 100644
--- a/libraries/entities/src/EntityScriptingInterface.cpp
+++ b/libraries/entities/src/EntityScriptingInterface.cpp
@@ -129,37 +129,38 @@ EntityItemProperties EntityScriptingInterface::getEntityProperties(QUuid identit
     return results;
 }
 
-QUuid EntityScriptingInterface::editEntity(QUuid id, const EntityItemProperties& properties) {
+QUuid EntityScriptingInterface::editEntity(QUuid id, EntityItemProperties properties) {
     EntityItemID entityID(id);
     // If we have a local entity tree set, then also update it.
     if (_entityTree) {
         _entityTree->lockForWrite();
-        _entityTree->updateEntity(entityID, properties);
+        bool updatedEntity = _entityTree->updateEntity(entityID, properties);
         _entityTree->unlock();
-    }
 
-    // make sure the properties has a type, so that the encode can know which properties to include
-    if (properties.getType() == EntityTypes::Unknown) {
-        EntityItemPointer entity = _entityTree->findEntityByEntityItemID(entityID);
-        if (entity) {
-            // we need to change the outgoing properties, so we make a copy, modify, and send.
-            EntityItemProperties modifiedProperties = properties;
-            entity->setLastBroadcast(usecTimestampNow());
-            modifiedProperties.setType(entity->getType());
-            if (modifiedProperties.hasTerseUpdateChanges()) {
-                // we make a bid for (or assert) our simulation ownership
-                auto nodeList = DependencyManager::get<NodeList>();
-                const QUuid myNodeID = nodeList->getSessionUUID();
-                modifiedProperties.setSimulatorID(myNodeID);
-
-                if (entity->getSimulatorID() == myNodeID) {
-                    // we think we already own simulation, so make sure we send ALL TerseUpdate properties
-                    entity->getAllTerseUpdateProperties(modifiedProperties);
+        if (updatedEntity) {
+            _entityTree->lockForRead();
+            EntityItemPointer entity = _entityTree->findEntityByEntityItemID(entityID);
+            if (entity) {
+                // make sure the properties has a type, so that the encode can know which properties to include
+                properties.setType(entity->getType());
+                if (properties.hasTerseUpdateChanges()) {
+                    // we make a bid for (or assert) our simulation ownership
+                    auto nodeList = DependencyManager::get<NodeList>();
+                    const QUuid myNodeID = nodeList->getSessionUUID();
+                    properties.setSimulatorID(myNodeID);
+    
+                    if (entity->getSimulatorID() == myNodeID) {
+                        // we think we already own the simulation, so make sure to send ALL TerseUpdate properties
+                        entity->getAllTerseUpdateProperties(properties);
+                    }
                 }
+                entity->setLastBroadcast(usecTimestampNow());
             }
-            queueEntityMessage(PacketTypeEntityEdit, entityID, modifiedProperties);
+            _entityTree->unlock();
+            queueEntityMessage(PacketTypeEntityEdit, entityID, properties);
             return id;
         }
+        return QUuid();
     }
 
     queueEntityMessage(PacketTypeEntityEdit, entityID, properties);
diff --git a/libraries/entities/src/EntityScriptingInterface.h b/libraries/entities/src/EntityScriptingInterface.h
index 6c2dc06579..0e2b27baad 100644
--- a/libraries/entities/src/EntityScriptingInterface.h
+++ b/libraries/entities/src/EntityScriptingInterface.h
@@ -78,7 +78,7 @@ public slots:
 
     /// edits a model updating only the included properties, will return the identified EntityItemID in case of
     /// successful edit, if the input entityID is for an unknown model this function will have no effect
-    Q_INVOKABLE QUuid editEntity(QUuid entityID, const EntityItemProperties& properties);
+    Q_INVOKABLE QUuid editEntity(QUuid entityID, EntityItemProperties properties);
 
     /// deletes a model
     Q_INVOKABLE void deleteEntity(QUuid entityID);