From c5b8dd51d8df9520cc1c12a608bb9cf360029961 Mon Sep 17 00:00:00 2001
From: Howard Stearns <howard@highfidelity.io>
Date: Wed, 20 May 2015 11:07:03 -0700
Subject: [PATCH] Fix https://app.asana.com/0/32622044445063/34195351184789

Handle collision events when we do other updates (and their entity scripts) rather than when we do the physic updates while the tree is locked.
Given that, remove the check that kept sound from playing (or scripts from running) when we would have deadlocked, because now we don't.
---
 interface/src/Application.cpp                          |  7 ++++---
 libraries/entities-renderer/src/EntityTreeRenderer.cpp | 10 +---------
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index 37ee116f40..747b765ff7 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -2461,7 +2461,6 @@ void Application::update(float deltaTime) {
         if (_physicsEngine.hasOutgoingChanges()) {
             _entitySimulation.lock();
             _entitySimulation.handleOutgoingChanges(_physicsEngine.getOutgoingChanges(), _physicsEngine.getSessionID());
-            _entitySimulation.handleCollisionEvents(_physicsEngine.getCollisionEvents());
             _entitySimulation.unlock();
             _physicsEngine.dumpStatsIfNecessary();
         }
@@ -2469,9 +2468,11 @@ void Application::update(float deltaTime) {
 
     if (!_aboutToQuit) {
         PerformanceTimer perfTimer("entities");
-        // NOTE: the _entities.update() call below will wait for lock 
+        // Collision events (and their scripts) must not be handled when we're locked, above. (That would risk deadlock.)
+        _entitySimulation.handleCollisionEvents(_physicsEngine.getCollisionEvents());
+        // NOTE: the _entities.update() call below will wait for lock
         // and will simulate entity motion (the EntityTree has been given an EntitySimulation).  
-        _entities.update(); // update the models...
+       _entities.update(); // update the models...
     }
 
     {
diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
index bacd6eb56e..e8627e655a 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
@@ -1174,8 +1174,7 @@ void EntityTreeRenderer::entityCollisionWithEntity(const EntityItemID& idA, cons
     if (!_tree || _shuttingDown) {
         return;
     }
-    // Don't respond to small continuous contacts. It causes deadlocks when locking the entityTree.
-    // Note that any entity script is likely to Entities.getEntityProperties(), which locks the tree.
+    // Don't respond to small continuous contacts.
     const float COLLISION_MINUMUM_PENETRATION = 0.005;
     if ((collision.type != CONTACT_EVENT_TYPE_START) && (glm::length(collision.penetration) < COLLISION_MINUMUM_PENETRATION)) {
         return;
@@ -1183,16 +1182,9 @@ void EntityTreeRenderer::entityCollisionWithEntity(const EntityItemID& idA, cons
 
     // See if we should play sounds
     EntityTree* entityTree = static_cast<EntityTree*>(_tree);
-    if (!entityTree->tryLockForRead()) {
-        // I don't know why this can happen, but if it does,
-        // the consequences are a deadlock, so bail.
-        qCDebug(entitiesrenderer) << "NOTICE: skipping collision type " << collision.type << " penetration " << glm::length(collision.penetration);
-        return;
-    }
     const QUuid& myNodeID = DependencyManager::get<NodeList>()->getSessionUUID();
     playEntityCollisionSound(myNodeID, entityTree, idA, collision);
     playEntityCollisionSound(myNodeID, entityTree, idB, collision);
-    entityTree->unlock();
 
     // And now the entity scripts
     QScriptValue entityScriptA = loadEntityScript(idA);