From 87078b1ea29931e9de641a92876382f1bc710aad Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Sat, 16 Apr 2016 11:09:23 -0700
Subject: [PATCH 01/21] first cut at resetting entities script engine on domain
 change

---
 interface/src/Application.cpp                 | 40 +++++++++++++++++--
 .../src/EntityTreeRenderer.cpp                | 38 +++++++++++++++++-
 libraries/script-engine/src/ScriptEngine.cpp  | 16 ++++++++
 3 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index 3a0b0998c5..11c68a8878 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -1156,6 +1156,9 @@ void Application::aboutToQuit() {
 }
 
 void Application::cleanupBeforeQuit() {
+    qDebug() << __FUNCTION__ << "----- START -----";
+
+
     // add a logline indicating if QTWEBENGINE_REMOTE_DEBUGGING is set or not
     QString webengineRemoteDebugging = QProcessEnvironment::systemEnvironment().value("QTWEBENGINE_REMOTE_DEBUGGING", "false");
     qCDebug(interfaceapp) << "QTWEBENGINE_REMOTE_DEBUGGING =" << webengineRemoteDebugging;
@@ -1178,8 +1181,6 @@ void Application::cleanupBeforeQuit() {
     }
     _keyboardFocusHighlight = nullptr;
 
-    getEntities()->clear(); // this will allow entity scripts to properly shutdown
-
     auto nodeList = DependencyManager::get<NodeList>();
 
     // send the domain a disconnect packet, force stoppage of domain-server check-ins
@@ -1189,10 +1190,18 @@ void Application::cleanupBeforeQuit() {
     // tell the packet receiver we're shutting down, so it can drop packets
     nodeList->getPacketReceiver().setShouldDropPackets(true);
 
+    qDebug() << __FUNCTION__ << "about to call getEntities()->shutdown();";
     getEntities()->shutdown(); // tell the entities system we're shutting down, so it will stop running scripts
+
+    getEntities()->clear(); // this will allow entity scripts to properly shutdown
+    //DependencyManager::destroy<EntityTreeRenderer>();
+    qDebug() << __FUNCTION__ << "AFTER call to getEntities()->shutdown();";
+
+    qDebug() << __FUNCTION__ << "about to shutdown ScriptEngines...";
     DependencyManager::get<ScriptEngines>()->saveScripts();
     DependencyManager::get<ScriptEngines>()->shutdownScripting(); // stop all currently running global scripts
     DependencyManager::destroy<ScriptEngines>();
+    qDebug() << __FUNCTION__ << "AFTER shutdown ScriptEngines...";
 
     // first stop all timers directly or by invokeMethod
     // depending on what thread they run in
@@ -1202,11 +1211,15 @@ void Application::cleanupBeforeQuit() {
     pingTimer.stop();
     QMetaObject::invokeMethod(&_settingsTimer, "stop", Qt::BlockingQueuedConnection);
 
+    qDebug() << __FUNCTION__ << " line:" << __LINE__;
+
     // save state
     _settingsThread.quit();
     saveSettings();
     _window->saveGeometry();
 
+    qDebug() << __FUNCTION__ << " line:" << __LINE__;
+
     // stop the AudioClient
     QMetaObject::invokeMethod(DependencyManager::get<AudioClient>().data(),
                               "stop", Qt::BlockingQueuedConnection);
@@ -1214,10 +1227,14 @@ void Application::cleanupBeforeQuit() {
     // destroy the AudioClient so it and its thread have a chance to go down safely
     DependencyManager::destroy<AudioClient>();
 
+    qDebug() << __FUNCTION__ << " line:" << __LINE__;
+
     // destroy the AudioInjectorManager so it and its thread have a chance to go down safely
     // this will also stop any ongoing network injectors
     DependencyManager::destroy<AudioInjectorManager>();
 
+    qDebug() << __FUNCTION__ << " line:" << __LINE__;
+
     // Destroy third party processes after scripts have finished using them.
 #ifdef HAVE_DDE
     DependencyManager::destroy<DdeFaceTracker>();
@@ -1226,7 +1243,11 @@ void Application::cleanupBeforeQuit() {
     DependencyManager::destroy<EyeTracker>();
 #endif
 
+    qDebug() << __FUNCTION__ << " line:" << __LINE__;
+
     DependencyManager::destroy<OffscreenUi>();
+
+    qDebug() << __FUNCTION__ << " ----- END -----";
 }
 
 Application::~Application() {
@@ -4174,6 +4195,14 @@ void Application::updateWindowTitle() const {
 }
 
 void Application::clearDomainOctreeDetails() {
+
+    // if we're about to quit, we really don't need to do any of these things...
+    if (_aboutToQuit) {
+        qCDebug(interfaceapp) << "No need to clear domain octree details we are shutting down...";
+        return;
+    }
+
+
     qCDebug(interfaceapp) << "Clearing domain octree details...";
     // reset the environment so that we don't erroneously end up with multiple
 
@@ -4189,7 +4218,9 @@ void Application::clearDomainOctreeDetails() {
     });
 
     // reset the model renderer
+    qCDebug(interfaceapp) << __FUNCTION__ << " --- BEFORE getEntities()->clear() --- _aboutToQuit:" << _aboutToQuit;
     getEntities()->clear();
+    qCDebug(interfaceapp) << __FUNCTION__ << " --- AFTER getEntities()->clear() ---";
 
     auto skyStage = DependencyManager::get<SceneScriptingInterface>()->getSkyStage();
     skyStage->setBackgroundMode(model::SunSkyStage::SKY_DOME);
@@ -4198,10 +4229,13 @@ void Application::clearDomainOctreeDetails() {
 }
 
 void Application::domainChanged(const QString& domainHostname) {
+    qCDebug(interfaceapp) << __FUNCTION__ << " --- BEGIN --- domainHostname:" << domainHostname;
+
     updateWindowTitle();
-    clearDomainOctreeDetails();
+    //clearDomainOctreeDetails();
     // disable physics until we have enough information about our new location to not cause craziness.
     resetPhysicsReadyInformation();
+    qCDebug(interfaceapp) << __FUNCTION__ << " --- END ---";
 }
 
 
diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
index eab28f500a..2544eb970d 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
@@ -71,13 +71,46 @@ EntityTreeRenderer::EntityTreeRenderer(bool wantScripts, AbstractViewStateInterf
 }
 
 EntityTreeRenderer::~EntityTreeRenderer() {
+    qDebug() << __FUNCTION__ << "---- BEGIN ----";
     // NOTE: We don't need to delete _entitiesScriptEngine because
     //       it is registered with ScriptEngines, which will call deleteLater for us.
+
+    /*
+    if (_entitiesScriptEngine) { 
+        qDebug() << __FUNCTION__ << "about to stop entity script engine.";
+        _entitiesScriptEngine->stop();
+        qDebug() << __FUNCTION__ << "about to delete entity script engine.";
+        delete _entitiesScriptEngine;
+        _entitiesScriptEngine = nullptr;
+    }
+    */
+    qDebug() << __FUNCTION__ << "---- END ----";
 }
 
+static int entititesScriptEngineCount = 0;
+
 void EntityTreeRenderer::clear() {
+    qDebug() << __FUNCTION__ << "---- BEGIN ----";
     leaveAllEntities();
-    _entitiesScriptEngine->unloadAllEntityScripts();
+    if (_entitiesScriptEngine) {
+        _entitiesScriptEngine->unloadAllEntityScripts();
+    }
+
+    // this would be a good place to actuall delete and recreate the _entitiesScriptEngine
+    qDebug() << __FUNCTION__ << "_shuttingDown:" << _shuttingDown;
+    if (_wantScripts && !_shuttingDown) {
+        qDebug() << __FUNCTION__ << " about to stop/delete current _entitiesScriptEngine";
+        _entitiesScriptEngine->stop();
+        _entitiesScriptEngine->deleteLater();
+        qDebug() << __FUNCTION__ << " AFTER stop/delete current _entitiesScriptEngine";
+
+        qDebug() << __FUNCTION__ << " about to create new _entitiesScriptEngine";
+        _entitiesScriptEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount));
+        _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine);
+        _entitiesScriptEngine->runInThread();
+        DependencyManager::get<EntityScriptingInterface>()->setEntitiesScriptEngine(_entitiesScriptEngine);
+        qDebug() << __FUNCTION__ << " AFTER create new _entitiesScriptEngine";
+    }
 
     auto scene = _viewState->getMain3DScene();
     render::PendingChanges pendingChanges;
@@ -88,6 +121,7 @@ void EntityTreeRenderer::clear() {
     _entitiesInScene.clear();
 
     OctreeRenderer::clear();
+    qDebug() << __FUNCTION__ << "---- END ----";
 }
 
 void EntityTreeRenderer::reloadEntityScripts() {
@@ -105,7 +139,7 @@ void EntityTreeRenderer::init() {
     entityTree->setFBXService(this);
 
     if (_wantScripts) {
-        _entitiesScriptEngine = new ScriptEngine(NO_SCRIPT, "Entities");
+        _entitiesScriptEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount));
         _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine);
         _entitiesScriptEngine->runInThread();
         DependencyManager::get<EntityScriptingInterface>()->setEntitiesScriptEngine(_entitiesScriptEngine);
diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index 506bdb9463..1b3ad09d78 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -142,14 +142,18 @@ ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNam
 }
 
 ScriptEngine::~ScriptEngine() {
+    qCDebug(scriptengine) << __FUNCTION__ << "--- BEGIN --- script:" << getFilename();
     qCDebug(scriptengine) << "Script Engine shutting down (destructor) for script:" << getFilename();
 
     auto scriptEngines = DependencyManager::get<ScriptEngines>();
     if (scriptEngines) {
+        qCDebug(scriptengine) << "About to remove ScriptEngine [" << getFilename() << "] from ScriptEngines!";
         scriptEngines->removeScriptEngine(this);
+        qCDebug(scriptengine) << "AFTER remove ScriptEngine [" << getFilename() << "] from ScriptEngines!";
     } else {
         qCWarning(scriptengine) << "Script destroyed after ScriptEngines!";
     }
+    qCDebug(scriptengine) << __FUNCTION__ << "--- END --- script:" << getFilename();
 }
 
 void ScriptEngine::disconnectNonEssentialSignals() {
@@ -1197,6 +1201,8 @@ void ScriptEngine::unloadEntityScript(const EntityItemID& entityID) {
     }
 }
 
+#include <QScriptValueIterator>
+
 void ScriptEngine::unloadAllEntityScripts() {
     if (QThread::currentThread() != thread()) {
 #ifdef THREAD_DEBUGGING
@@ -1213,6 +1219,16 @@ void ScriptEngine::unloadAllEntityScripts() {
         callEntityScriptMethod(entityID, "unload");
     }
     _entityScripts.clear();
+
+#ifdef DEBUG_ENGINE_STATE
+    qDebug() << "---- CURRENT STATE OF ENGINE: --------------------------";
+    QScriptValueIterator it(globalObject());
+    while (it.hasNext()) {
+        it.next();
+        qDebug() << it.name() << ":" << it.value().toString();
+    }
+    qDebug() << "--------------------------------------------------------";
+#endif // DEBUG_ENGINE_STATE
 }
 
 void ScriptEngine::refreshFileScript(const EntityItemID& entityID) {

From 71d57295eb968cd6f66364253b6d75893d77cc34 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Sat, 16 Apr 2016 19:39:45 -0700
Subject: [PATCH 02/21] handle possibly deleted ScriptEngine in
 getScriptContents

---
 libraries/script-engine/src/ScriptEngine.cpp | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index 1b3ad09d78..cff0c569bf 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -1077,13 +1077,21 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString&
     qDebug() << "ScriptEngine::loadEntityScript() calling scriptCache->getScriptContents() on thread ["
         << QThread::currentThread() << "] expected thread [" << thread() << "]";
 #endif
-    DependencyManager::get<ScriptCache>()->getScriptContents(entityScript, [=](const QString& scriptOrURL, const QString& contents, bool isURL, bool success) {
+
+    QPointer<ScriptEngine> theEngine(this);
+
+    DependencyManager::get<ScriptCache>()->getScriptContents(entityScript, [theEngine, entityID](const QString& scriptOrURL, const QString& contents, bool isURL, bool success) {
 #ifdef THREAD_DEBUGGING
         qDebug() << "ScriptEngine::entityScriptContentAvailable() IN LAMBDA contentAvailable on thread ["
             << QThread::currentThread() << "] expected thread [" << thread() << "]";
 #endif
 
-        this->entityScriptContentAvailable(entityID, scriptOrURL, contents, isURL, success);
+        if (!theEngine.isNull()) {
+            qDebug() << "ScriptCache::getScriptContents() returned ScriptEngine still active calling ... entityScriptContentAvailable()";
+            theEngine->entityScriptContentAvailable(entityID, scriptOrURL, contents, isURL, success);
+        } else {
+            qDebug() << "ScriptCache::getScriptContents() returned after our ScriptEngine was deleted...";
+        }
     }, forceRedownload);
 }
 

From 9db0fe0d11594ac658e6a4d8afa08c19a8b6885c Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 09:46:04 -0700
Subject: [PATCH 03/21] switch to using QSharedPointer for
 _entitiesScriptEngine

---
 .../src/EntityTreeRenderer.cpp                | 28 ++++++++++------
 .../src/EntityTreeRenderer.h                  |  2 +-
 libraries/script-engine/src/ScriptEngine.cpp  | 32 ++++++++++++-------
 libraries/script-engine/src/ScriptEngine.h    | 15 ++++++++-
 4 files changed, 54 insertions(+), 23 deletions(-)

diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
index 2544eb970d..3edc9ba75a 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
@@ -99,16 +99,24 @@ void EntityTreeRenderer::clear() {
     // this would be a good place to actuall delete and recreate the _entitiesScriptEngine
     qDebug() << __FUNCTION__ << "_shuttingDown:" << _shuttingDown;
     if (_wantScripts && !_shuttingDown) {
-        qDebug() << __FUNCTION__ << " about to stop/delete current _entitiesScriptEngine";
+        qDebug() << __FUNCTION__ << " about to _entitiesScriptEngine->stop()";
         _entitiesScriptEngine->stop();
+        qDebug() << __FUNCTION__ << " AFTER _entitiesScriptEngine->stop()";
+
+        qDebug() << __FUNCTION__ << " about to _entitiesScriptEngine->deleteLater()";
+        // NOTE: you can't actually delete it here we need to let Qt unwind and delete the object safely when ready
+        qDebug() << __FUNCTION__ << " about to call _entitiesScriptEngine->deleteLater() on thread[" << QThread::currentThread() << "], _entitiesScriptEngine's thread [" << _entitiesScriptEngine->thread() << "]";
         _entitiesScriptEngine->deleteLater();
-        qDebug() << __FUNCTION__ << " AFTER stop/delete current _entitiesScriptEngine";
+        //QMetaObject::invokeMethod(_entitiesScriptEngine, "deleteLater");
+        qDebug() << __FUNCTION__ << " AFTER _entitiesScriptEngine->deleteLater()";
 
         qDebug() << __FUNCTION__ << " about to create new _entitiesScriptEngine";
-        _entitiesScriptEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount));
-        _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine);
+        _entitiesScriptEngine = QSharedPointer<ScriptEngine>(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)), ScriptEngine::doDeleteLater);
+
+
+        _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data());
         _entitiesScriptEngine->runInThread();
-        DependencyManager::get<EntityScriptingInterface>()->setEntitiesScriptEngine(_entitiesScriptEngine);
+        DependencyManager::get<EntityScriptingInterface>()->setEntitiesScriptEngine(_entitiesScriptEngine.data());
         qDebug() << __FUNCTION__ << " AFTER create new _entitiesScriptEngine";
     }
 
@@ -128,7 +136,7 @@ void EntityTreeRenderer::reloadEntityScripts() {
     _entitiesScriptEngine->unloadAllEntityScripts();
     foreach(auto entity, _entitiesInScene) {
         if (!entity->getScript().isEmpty()) {
-            _entitiesScriptEngine->loadEntityScript(entity->getEntityItemID(), entity->getScript(), true);
+            ScriptEngine::loadEntityScript(_entitiesScriptEngine, entity->getEntityItemID(), entity->getScript(), true);
         }
     }
 }
@@ -139,10 +147,10 @@ void EntityTreeRenderer::init() {
     entityTree->setFBXService(this);
 
     if (_wantScripts) {
-        _entitiesScriptEngine = new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount));
-        _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine);
+        _entitiesScriptEngine = QSharedPointer<ScriptEngine>(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)), ScriptEngine::doDeleteLater);
+        _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data());
         _entitiesScriptEngine->runInThread();
-        DependencyManager::get<EntityScriptingInterface>()->setEntitiesScriptEngine(_entitiesScriptEngine);
+        DependencyManager::get<EntityScriptingInterface>()->setEntitiesScriptEngine(_entitiesScriptEngine.data());
     }
 
     forceRecheckEntities(); // setup our state to force checking our inside/outsideness of entities
@@ -797,7 +805,7 @@ void EntityTreeRenderer::checkAndCallPreload(const EntityItemID& entityID, const
         if (entity && entity->shouldPreloadScript()) {
             QString scriptUrl = entity->getScript();
             scriptUrl = ResourceManager::normalizeURL(scriptUrl);
-            _entitiesScriptEngine->loadEntityScript(entityID, scriptUrl, reload);
+            ScriptEngine::loadEntityScript(_entitiesScriptEngine, entityID, scriptUrl, reload);
             entity->scriptHasPreloaded();
         }
     }
diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.h b/libraries/entities-renderer/src/EntityTreeRenderer.h
index 0fd5adc3f0..42e8ddeddf 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.h
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.h
@@ -155,7 +155,7 @@ private:
     NetworkTexturePointer _ambientTexture;
 
     bool _wantScripts;
-    ScriptEngine* _entitiesScriptEngine;
+    QSharedPointer<ScriptEngine> _entitiesScriptEngine;
 
     bool isCollisionOwner(const QUuid& myNodeID, EntityTreePointer entityTree,
                           const EntityItemID& id, const Collision& collision);
diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index cff0c569bf..6b2d9e6854 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -143,6 +143,7 @@ ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNam
 
 ScriptEngine::~ScriptEngine() {
     qCDebug(scriptengine) << __FUNCTION__ << "--- BEGIN --- script:" << getFilename();
+    qCDebug(scriptengine) << __FUNCTION__ << "called on thread[" << QThread::currentThread() << "], object's thread [" << thread() << "] script:" << getFilename();
     qCDebug(scriptengine) << "Script Engine shutting down (destructor) for script:" << getFilename();
 
     auto scriptEngines = DependencyManager::get<ScriptEngines>();
@@ -1051,7 +1052,10 @@ void ScriptEngine::forwardHandlerCall(const EntityItemID& entityID, const QStrin
 
 // since all of these operations can be asynch we will always do the actual work in the response handler
 // for the download
-void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) {
+void ScriptEngine::loadEntityScript(QWeakPointer<ScriptEngine> theEngine, const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) {
+
+
+/*****
     if (QThread::currentThread() != thread()) {
 #ifdef THREAD_DEBUGGING
         qDebug() << "*** WARNING *** ScriptEngine::loadEntityScript() called on wrong thread ["
@@ -1070,25 +1074,30 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString&
         "entityID:" << entityID << "entityScript:" << entityScript << "forceRedownload:" << forceRedownload;
 #endif
 
-    // If we've been called our known entityScripts should not know about us..
-    assert(!_entityScripts.contains(entityID));
+        // If we've been called our known entityScripts should not know about us..
+        assert(!_entityScripts.contains(entityID));
 
 #ifdef THREAD_DEBUGGING
     qDebug() << "ScriptEngine::loadEntityScript() calling scriptCache->getScriptContents() on thread ["
         << QThread::currentThread() << "] expected thread [" << thread() << "]";
 #endif
+******/
 
-    QPointer<ScriptEngine> theEngine(this);
-
+    // NOTE: If the script content is not currently in the caceh, The LAMBDA here, will be called on the Main Thread
+    //       which means we're guarenteed that it's not the correct thread for the ScriptEngine. This means
+    //       when we get into entityScriptContentAvailable() we will likely invokeMethod() to get it over
+    //       to the "Entities" ScriptEngine thread.
     DependencyManager::get<ScriptCache>()->getScriptContents(entityScript, [theEngine, entityID](const QString& scriptOrURL, const QString& contents, bool isURL, bool success) {
-#ifdef THREAD_DEBUGGING
-        qDebug() << "ScriptEngine::entityScriptContentAvailable() IN LAMBDA contentAvailable on thread ["
-            << QThread::currentThread() << "] expected thread [" << thread() << "]";
+        QSharedPointer<ScriptEngine> strongEngine = theEngine.toStrongRef();
+        if (strongEngine) {
+            qDebug() << "ScriptCache::getScriptContents() returned ScriptEngine still active calling ... entityScriptContentAvailable()";
+#if 1//def THREAD_DEBUGGING
+            qDebug() << "ScriptEngine::entityScriptContentAvailable() IN LAMBDA contentAvailable on thread ["
+                << QThread::currentThread() << "] expected thread [" << strongEngine->thread() << "]";
 #endif
 
-        if (!theEngine.isNull()) {
-            qDebug() << "ScriptCache::getScriptContents() returned ScriptEngine still active calling ... entityScriptContentAvailable()";
-            theEngine->entityScriptContentAvailable(entityID, scriptOrURL, contents, isURL, success);
+
+            strongEngine->entityScriptContentAvailable(entityID, scriptOrURL, contents, isURL, success);
         } else {
             qDebug() << "ScriptCache::getScriptContents() returned after our ScriptEngine was deleted...";
         }
@@ -1098,6 +1107,7 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString&
 // since all of these operations can be asynch we will always do the actual work in the response handler
 // for the download
 void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, const QString& scriptOrURL, const QString& contents, bool isURL, bool success) {
+
     if (QThread::currentThread() != thread()) {
 #ifdef THREAD_DEBUGGING
         qDebug() << "*** WARNING *** ScriptEngine::entityScriptContentAvailable() called on wrong thread ["
diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h
index a6a623e751..c5ed5b56fe 100644
--- a/libraries/script-engine/src/ScriptEngine.h
+++ b/libraries/script-engine/src/ScriptEngine.h
@@ -70,6 +70,9 @@ public:
 
     ~ScriptEngine();
 
+    // Useful for QSharePointer<ScriptEngine>
+    static void doDeleteLater(ScriptEngine* obj) { obj->deleteLater(); }
+
     /// run the script in a dedicated thread. This will have the side effect of evalulating
     /// the current script contents and calling run(). Callers will likely want to register the script with external
     /// services before calling this.
@@ -124,7 +127,10 @@ public:
     Q_INVOKABLE QUrl resolvePath(const QString& path) const;
 
     // Entity Script Related methods
-    Q_INVOKABLE void loadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload = false); // will call the preload method once loaded
+    //Q_INVOKABLE void loadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload = false); // will call the preload method once loaded
+
+    static void loadEntityScript(QWeakPointer<ScriptEngine> theEngine, const EntityItemID& entityID, const QString& entityScript, bool forceRedownload);
+
     Q_INVOKABLE void unloadEntityScript(const EntityItemID& entityID); // will call unload method
     Q_INVOKABLE void unloadAllEntityScripts();
     Q_INVOKABLE void callEntityScriptMethod(const EntityItemID& entityID, const QString& methodName, const QStringList& params = QStringList());
@@ -224,4 +230,11 @@ protected:
     static std::atomic<bool> _stoppingAllScripts;
 };
 
+/*
+class ScriptEngine : public QObject {
+    Q_OBJECT
+};
+*/
+
+
 #endif // hifi_ScriptEngine_h

From a4f562672acf14f3b944dcdf9c0d0e3ee8908d99 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 10:08:33 -0700
Subject: [PATCH 04/21] cleanup

---
 .../src/EntityTreeRenderer.cpp                | 48 +++++++------------
 .../src/EntityTreeRenderer.h                  |  4 ++
 2 files changed, 20 insertions(+), 32 deletions(-)

diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
index 3edc9ba75a..131784cc94 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
@@ -71,23 +71,21 @@ EntityTreeRenderer::EntityTreeRenderer(bool wantScripts, AbstractViewStateInterf
 }
 
 EntityTreeRenderer::~EntityTreeRenderer() {
-    qDebug() << __FUNCTION__ << "---- BEGIN ----";
+    qDebug() << __FUNCTION__;
     // NOTE: We don't need to delete _entitiesScriptEngine because
     //       it is registered with ScriptEngines, which will call deleteLater for us.
-
-    /*
-    if (_entitiesScriptEngine) { 
-        qDebug() << __FUNCTION__ << "about to stop entity script engine.";
-        _entitiesScriptEngine->stop();
-        qDebug() << __FUNCTION__ << "about to delete entity script engine.";
-        delete _entitiesScriptEngine;
-        _entitiesScriptEngine = nullptr;
-    }
-    */
-    qDebug() << __FUNCTION__ << "---- END ----";
 }
 
-static int entititesScriptEngineCount = 0;
+int EntityTreeRenderer::_entititesScriptEngineCount = 0;
+
+void EntityTreeRenderer::setupEntitiesScriptEngine() {
+    qDebug() << __FUNCTION__ << "---- BEGIN ----";
+    _entitiesScriptEngine = QSharedPointer<ScriptEngine>(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entititesScriptEngineCount)), ScriptEngine::doDeleteLater);
+    _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data());
+    _entitiesScriptEngine->runInThread();
+    DependencyManager::get<EntityScriptingInterface>()->setEntitiesScriptEngine(_entitiesScriptEngine.data());
+    qDebug() << __FUNCTION__ << "---- END ----";
+}
 
 void EntityTreeRenderer::clear() {
     qDebug() << __FUNCTION__ << "---- BEGIN ----";
@@ -103,21 +101,10 @@ void EntityTreeRenderer::clear() {
         _entitiesScriptEngine->stop();
         qDebug() << __FUNCTION__ << " AFTER _entitiesScriptEngine->stop()";
 
-        qDebug() << __FUNCTION__ << " about to _entitiesScriptEngine->deleteLater()";
-        // NOTE: you can't actually delete it here we need to let Qt unwind and delete the object safely when ready
-        qDebug() << __FUNCTION__ << " about to call _entitiesScriptEngine->deleteLater() on thread[" << QThread::currentThread() << "], _entitiesScriptEngine's thread [" << _entitiesScriptEngine->thread() << "]";
-        _entitiesScriptEngine->deleteLater();
-        //QMetaObject::invokeMethod(_entitiesScriptEngine, "deleteLater");
-        qDebug() << __FUNCTION__ << " AFTER _entitiesScriptEngine->deleteLater()";
-
-        qDebug() << __FUNCTION__ << " about to create new _entitiesScriptEngine";
-        _entitiesScriptEngine = QSharedPointer<ScriptEngine>(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)), ScriptEngine::doDeleteLater);
-
-
-        _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data());
-        _entitiesScriptEngine->runInThread();
-        DependencyManager::get<EntityScriptingInterface>()->setEntitiesScriptEngine(_entitiesScriptEngine.data());
-        qDebug() << __FUNCTION__ << " AFTER create new _entitiesScriptEngine";
+        // NOTE: you can't actually need to delete it here because when we call setupEntitiesScriptEngine it will
+        //       assign a new instance to our shared pointer, which will deref the old instance and ultimately call
+        //       the custom deleter which calls deleteLater
+        setupEntitiesScriptEngine();
     }
 
     auto scene = _viewState->getMain3DScene();
@@ -147,10 +134,7 @@ void EntityTreeRenderer::init() {
     entityTree->setFBXService(this);
 
     if (_wantScripts) {
-        _entitiesScriptEngine = QSharedPointer<ScriptEngine>(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)), ScriptEngine::doDeleteLater);
-        _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data());
-        _entitiesScriptEngine->runInThread();
-        DependencyManager::get<EntityScriptingInterface>()->setEntitiesScriptEngine(_entitiesScriptEngine.data());
+        setupEntitiesScriptEngine();
     }
 
     forceRecheckEntities(); // setup our state to force checking our inside/outsideness of entities
diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.h b/libraries/entities-renderer/src/EntityTreeRenderer.h
index 42e8ddeddf..f1b581d8ef 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.h
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.h
@@ -126,6 +126,8 @@ protected:
     }
 
 private:
+    void setupEntitiesScriptEngine();
+
     void addEntityToScene(EntityItemPointer entity);
     bool findBestZoneAndMaybeContainingEntities(const glm::vec3& avatarPosition, QVector<EntityItemID>* entitiesContainingAvatar);
 
@@ -196,6 +198,8 @@ private:
     QHash<EntityItemID, EntityItemPointer> _entitiesInScene;
     // For Scene.shouldRenderEntities
     QList<EntityItemID> _entityIDsLastInScene;
+
+    static int _entititesScriptEngineCount;
 };
 
 

From d1f6b371c977fae41ebbcdd909db9f5e344bec20 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 10:27:12 -0700
Subject: [PATCH 05/21] cleanup

---
 interface/src/Application.cpp                 | 27 ------------
 .../src/EntityTreeRenderer.cpp                |  8 ----
 libraries/script-engine/src/ScriptEngine.cpp  | 42 ++-----------------
 3 files changed, 4 insertions(+), 73 deletions(-)

diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index 11c68a8878..d7702d0091 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -1156,9 +1156,6 @@ void Application::aboutToQuit() {
 }
 
 void Application::cleanupBeforeQuit() {
-    qDebug() << __FUNCTION__ << "----- START -----";
-
-
     // add a logline indicating if QTWEBENGINE_REMOTE_DEBUGGING is set or not
     QString webengineRemoteDebugging = QProcessEnvironment::systemEnvironment().value("QTWEBENGINE_REMOTE_DEBUGGING", "false");
     qCDebug(interfaceapp) << "QTWEBENGINE_REMOTE_DEBUGGING =" << webengineRemoteDebugging;
@@ -1190,18 +1187,12 @@ void Application::cleanupBeforeQuit() {
     // tell the packet receiver we're shutting down, so it can drop packets
     nodeList->getPacketReceiver().setShouldDropPackets(true);
 
-    qDebug() << __FUNCTION__ << "about to call getEntities()->shutdown();";
     getEntities()->shutdown(); // tell the entities system we're shutting down, so it will stop running scripts
-
     getEntities()->clear(); // this will allow entity scripts to properly shutdown
-    //DependencyManager::destroy<EntityTreeRenderer>();
-    qDebug() << __FUNCTION__ << "AFTER call to getEntities()->shutdown();";
 
-    qDebug() << __FUNCTION__ << "about to shutdown ScriptEngines...";
     DependencyManager::get<ScriptEngines>()->saveScripts();
     DependencyManager::get<ScriptEngines>()->shutdownScripting(); // stop all currently running global scripts
     DependencyManager::destroy<ScriptEngines>();
-    qDebug() << __FUNCTION__ << "AFTER shutdown ScriptEngines...";
 
     // first stop all timers directly or by invokeMethod
     // depending on what thread they run in
@@ -1211,15 +1202,11 @@ void Application::cleanupBeforeQuit() {
     pingTimer.stop();
     QMetaObject::invokeMethod(&_settingsTimer, "stop", Qt::BlockingQueuedConnection);
 
-    qDebug() << __FUNCTION__ << " line:" << __LINE__;
-
     // save state
     _settingsThread.quit();
     saveSettings();
     _window->saveGeometry();
 
-    qDebug() << __FUNCTION__ << " line:" << __LINE__;
-
     // stop the AudioClient
     QMetaObject::invokeMethod(DependencyManager::get<AudioClient>().data(),
                               "stop", Qt::BlockingQueuedConnection);
@@ -1227,14 +1214,10 @@ void Application::cleanupBeforeQuit() {
     // destroy the AudioClient so it and its thread have a chance to go down safely
     DependencyManager::destroy<AudioClient>();
 
-    qDebug() << __FUNCTION__ << " line:" << __LINE__;
-
     // destroy the AudioInjectorManager so it and its thread have a chance to go down safely
     // this will also stop any ongoing network injectors
     DependencyManager::destroy<AudioInjectorManager>();
 
-    qDebug() << __FUNCTION__ << " line:" << __LINE__;
-
     // Destroy third party processes after scripts have finished using them.
 #ifdef HAVE_DDE
     DependencyManager::destroy<DdeFaceTracker>();
@@ -1243,11 +1226,7 @@ void Application::cleanupBeforeQuit() {
     DependencyManager::destroy<EyeTracker>();
 #endif
 
-    qDebug() << __FUNCTION__ << " line:" << __LINE__;
-
     DependencyManager::destroy<OffscreenUi>();
-
-    qDebug() << __FUNCTION__ << " ----- END -----";
 }
 
 Application::~Application() {
@@ -4218,9 +4197,7 @@ void Application::clearDomainOctreeDetails() {
     });
 
     // reset the model renderer
-    qCDebug(interfaceapp) << __FUNCTION__ << " --- BEFORE getEntities()->clear() --- _aboutToQuit:" << _aboutToQuit;
     getEntities()->clear();
-    qCDebug(interfaceapp) << __FUNCTION__ << " --- AFTER getEntities()->clear() ---";
 
     auto skyStage = DependencyManager::get<SceneScriptingInterface>()->getSkyStage();
     skyStage->setBackgroundMode(model::SunSkyStage::SKY_DOME);
@@ -4229,13 +4206,9 @@ void Application::clearDomainOctreeDetails() {
 }
 
 void Application::domainChanged(const QString& domainHostname) {
-    qCDebug(interfaceapp) << __FUNCTION__ << " --- BEGIN --- domainHostname:" << domainHostname;
-
     updateWindowTitle();
-    //clearDomainOctreeDetails();
     // disable physics until we have enough information about our new location to not cause craziness.
     resetPhysicsReadyInformation();
-    qCDebug(interfaceapp) << __FUNCTION__ << " --- END ---";
 }
 
 
diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
index 131784cc94..f30261b179 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
@@ -71,7 +71,6 @@ EntityTreeRenderer::EntityTreeRenderer(bool wantScripts, AbstractViewStateInterf
 }
 
 EntityTreeRenderer::~EntityTreeRenderer() {
-    qDebug() << __FUNCTION__;
     // NOTE: We don't need to delete _entitiesScriptEngine because
     //       it is registered with ScriptEngines, which will call deleteLater for us.
 }
@@ -79,27 +78,21 @@ EntityTreeRenderer::~EntityTreeRenderer() {
 int EntityTreeRenderer::_entititesScriptEngineCount = 0;
 
 void EntityTreeRenderer::setupEntitiesScriptEngine() {
-    qDebug() << __FUNCTION__ << "---- BEGIN ----";
     _entitiesScriptEngine = QSharedPointer<ScriptEngine>(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entititesScriptEngineCount)), ScriptEngine::doDeleteLater);
     _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data());
     _entitiesScriptEngine->runInThread();
     DependencyManager::get<EntityScriptingInterface>()->setEntitiesScriptEngine(_entitiesScriptEngine.data());
-    qDebug() << __FUNCTION__ << "---- END ----";
 }
 
 void EntityTreeRenderer::clear() {
-    qDebug() << __FUNCTION__ << "---- BEGIN ----";
     leaveAllEntities();
     if (_entitiesScriptEngine) {
         _entitiesScriptEngine->unloadAllEntityScripts();
     }
 
     // this would be a good place to actuall delete and recreate the _entitiesScriptEngine
-    qDebug() << __FUNCTION__ << "_shuttingDown:" << _shuttingDown;
     if (_wantScripts && !_shuttingDown) {
-        qDebug() << __FUNCTION__ << " about to _entitiesScriptEngine->stop()";
         _entitiesScriptEngine->stop();
-        qDebug() << __FUNCTION__ << " AFTER _entitiesScriptEngine->stop()";
 
         // NOTE: you can't actually need to delete it here because when we call setupEntitiesScriptEngine it will
         //       assign a new instance to our shared pointer, which will deref the old instance and ultimately call
@@ -116,7 +109,6 @@ void EntityTreeRenderer::clear() {
     _entitiesInScene.clear();
 
     OctreeRenderer::clear();
-    qDebug() << __FUNCTION__ << "---- END ----";
 }
 
 void EntityTreeRenderer::reloadEntityScripts() {
diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index 6b2d9e6854..915eae84f8 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -142,10 +142,7 @@ ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNam
 }
 
 ScriptEngine::~ScriptEngine() {
-    qCDebug(scriptengine) << __FUNCTION__ << "--- BEGIN --- script:" << getFilename();
-    qCDebug(scriptengine) << __FUNCTION__ << "called on thread[" << QThread::currentThread() << "], object's thread [" << thread() << "] script:" << getFilename();
     qCDebug(scriptengine) << "Script Engine shutting down (destructor) for script:" << getFilename();
-
     auto scriptEngines = DependencyManager::get<ScriptEngines>();
     if (scriptEngines) {
         qCDebug(scriptengine) << "About to remove ScriptEngine [" << getFilename() << "] from ScriptEngines!";
@@ -154,7 +151,6 @@ ScriptEngine::~ScriptEngine() {
     } else {
         qCWarning(scriptengine) << "Script destroyed after ScriptEngines!";
     }
-    qCDebug(scriptengine) << __FUNCTION__ << "--- END --- script:" << getFilename();
 }
 
 void ScriptEngine::disconnectNonEssentialSignals() {
@@ -1053,36 +1049,6 @@ void ScriptEngine::forwardHandlerCall(const EntityItemID& entityID, const QStrin
 // since all of these operations can be asynch we will always do the actual work in the response handler
 // for the download
 void ScriptEngine::loadEntityScript(QWeakPointer<ScriptEngine> theEngine, const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) {
-
-
-/*****
-    if (QThread::currentThread() != thread()) {
-#ifdef THREAD_DEBUGGING
-        qDebug() << "*** WARNING *** ScriptEngine::loadEntityScript() called on wrong thread ["
-            << QThread::currentThread() << "], invoking on correct thread [" << thread() << "]  "
-            "entityID:" << entityID << "entityScript:" << entityScript <<"forceRedownload:" << forceRedownload;
-#endif
-
-        QMetaObject::invokeMethod(this, "loadEntityScript",
-                                  Q_ARG(const EntityItemID&, entityID),
-                                  Q_ARG(const QString&, entityScript),
-                                  Q_ARG(bool, forceRedownload));
-        return;
-    }
-#ifdef THREAD_DEBUGGING
-    qDebug() << "ScriptEngine::loadEntityScript() called on correct thread [" << thread() << "]  "
-        "entityID:" << entityID << "entityScript:" << entityScript << "forceRedownload:" << forceRedownload;
-#endif
-
-        // If we've been called our known entityScripts should not know about us..
-        assert(!_entityScripts.contains(entityID));
-
-#ifdef THREAD_DEBUGGING
-    qDebug() << "ScriptEngine::loadEntityScript() calling scriptCache->getScriptContents() on thread ["
-        << QThread::currentThread() << "] expected thread [" << thread() << "]";
-#endif
-******/
-
     // NOTE: If the script content is not currently in the caceh, The LAMBDA here, will be called on the Main Thread
     //       which means we're guarenteed that it's not the correct thread for the ScriptEngine. This means
     //       when we get into entityScriptContentAvailable() we will likely invokeMethod() to get it over
@@ -1090,15 +1056,15 @@ void ScriptEngine::loadEntityScript(QWeakPointer<ScriptEngine> theEngine, const
     DependencyManager::get<ScriptCache>()->getScriptContents(entityScript, [theEngine, entityID](const QString& scriptOrURL, const QString& contents, bool isURL, bool success) {
         QSharedPointer<ScriptEngine> strongEngine = theEngine.toStrongRef();
         if (strongEngine) {
-            qDebug() << "ScriptCache::getScriptContents() returned ScriptEngine still active calling ... entityScriptContentAvailable()";
-#if 1//def THREAD_DEBUGGING
+#ifdef THREAD_DEBUGGING
             qDebug() << "ScriptEngine::entityScriptContentAvailable() IN LAMBDA contentAvailable on thread ["
                 << QThread::currentThread() << "] expected thread [" << strongEngine->thread() << "]";
 #endif
-
-
             strongEngine->entityScriptContentAvailable(entityID, scriptOrURL, contents, isURL, success);
         } else {
+            // FIXME - I'm leaving this in for testing, so that QA can confirm that sometimes the script contents
+            //         returns after the ScriptEngine has been deleted, we can remove this after QA verifies the
+            //         repro case.
             qDebug() << "ScriptCache::getScriptContents() returned after our ScriptEngine was deleted...";
         }
     }, forceRedownload);

From 0520363da85c8dfd298b52038388f5474e3e2f05 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 10:37:15 -0700
Subject: [PATCH 06/21] cleanup

---
 interface/src/Application.cpp                |  4 +---
 libraries/script-engine/src/ScriptEngine.cpp |  3 +--
 libraries/script-engine/src/ScriptEngine.h   | 10 ----------
 3 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index d7702d0091..32b8af6ce8 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -4177,14 +4177,12 @@ void Application::clearDomainOctreeDetails() {
 
     // if we're about to quit, we really don't need to do any of these things...
     if (_aboutToQuit) {
-        qCDebug(interfaceapp) << "No need to clear domain octree details we are shutting down...";
         return;
     }
 
-
     qCDebug(interfaceapp) << "Clearing domain octree details...";
-    // reset the environment so that we don't erroneously end up with multiple
 
+    // reset the environment so that we don't erroneously end up with multiple
     resetPhysicsReadyInformation();
 
     // reset our node to stats and node to jurisdiction maps... since these must be changing...
diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index 915eae84f8..2e9ca88908 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -145,9 +145,7 @@ ScriptEngine::~ScriptEngine() {
     qCDebug(scriptengine) << "Script Engine shutting down (destructor) for script:" << getFilename();
     auto scriptEngines = DependencyManager::get<ScriptEngines>();
     if (scriptEngines) {
-        qCDebug(scriptengine) << "About to remove ScriptEngine [" << getFilename() << "] from ScriptEngines!";
         scriptEngines->removeScriptEngine(this);
-        qCDebug(scriptengine) << "AFTER remove ScriptEngine [" << getFilename() << "] from ScriptEngines!";
     } else {
         qCWarning(scriptengine) << "Script destroyed after ScriptEngines!";
     }
@@ -759,6 +757,7 @@ void ScriptEngine::stop() {
             QMetaObject::invokeMethod(this, "stop");
             return;
         }
+        abortEvaluation(); // abort any script evaluation that may be active
         _isFinished = true;
         if (_wantSignals) {
             emit runningStateChanged();
diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h
index c5ed5b56fe..2af4975a8e 100644
--- a/libraries/script-engine/src/ScriptEngine.h
+++ b/libraries/script-engine/src/ScriptEngine.h
@@ -127,10 +127,7 @@ public:
     Q_INVOKABLE QUrl resolvePath(const QString& path) const;
 
     // Entity Script Related methods
-    //Q_INVOKABLE void loadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload = false); // will call the preload method once loaded
-
     static void loadEntityScript(QWeakPointer<ScriptEngine> theEngine, const EntityItemID& entityID, const QString& entityScript, bool forceRedownload);
-
     Q_INVOKABLE void unloadEntityScript(const EntityItemID& entityID); // will call unload method
     Q_INVOKABLE void unloadAllEntityScripts();
     Q_INVOKABLE void callEntityScriptMethod(const EntityItemID& entityID, const QString& methodName, const QStringList& params = QStringList());
@@ -230,11 +227,4 @@ protected:
     static std::atomic<bool> _stoppingAllScripts;
 };
 
-/*
-class ScriptEngine : public QObject {
-    Q_OBJECT
-};
-*/
-
-
 #endif // hifi_ScriptEngine_h

From f6c75d0530c3b41dfc9af287ac4318ec421dadcf Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 10:41:54 -0700
Subject: [PATCH 07/21] cleanup

---
 libraries/entities-renderer/src/EntityTreeRenderer.cpp | 3 +--
 libraries/script-engine/src/ScriptEngine.cpp           | 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
index f30261b179..da860cd834 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
@@ -88,12 +88,11 @@ void EntityTreeRenderer::clear() {
     leaveAllEntities();
     if (_entitiesScriptEngine) {
         _entitiesScriptEngine->unloadAllEntityScripts();
+        _entitiesScriptEngine->stop();
     }
 
     // this would be a good place to actuall delete and recreate the _entitiesScriptEngine
     if (_wantScripts && !_shuttingDown) {
-        _entitiesScriptEngine->stop();
-
         // NOTE: you can't actually need to delete it here because when we call setupEntitiesScriptEngine it will
         //       assign a new instance to our shared pointer, which will deref the old instance and ultimately call
         //       the custom deleter which calls deleteLater
diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index 2e9ca88908..2aab250ef4 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -18,6 +18,7 @@
 #include <QtNetwork/QNetworkReply>
 #include <QtScript/QScriptEngine>
 #include <QtScript/QScriptValue>
+#include <QtScript/QScriptValueIterator>
 #include <QtCore/QStringList>
 
 #include <AudioConstants.h>
@@ -1072,7 +1073,6 @@ void ScriptEngine::loadEntityScript(QWeakPointer<ScriptEngine> theEngine, const
 // since all of these operations can be asynch we will always do the actual work in the response handler
 // for the download
 void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, const QString& scriptOrURL, const QString& contents, bool isURL, bool success) {
-
     if (QThread::currentThread() != thread()) {
 #ifdef THREAD_DEBUGGING
         qDebug() << "*** WARNING *** ScriptEngine::entityScriptContentAvailable() called on wrong thread ["
@@ -1184,8 +1184,6 @@ void ScriptEngine::unloadEntityScript(const EntityItemID& entityID) {
     }
 }
 
-#include <QScriptValueIterator>
-
 void ScriptEngine::unloadAllEntityScripts() {
     if (QThread::currentThread() != thread()) {
 #ifdef THREAD_DEBUGGING

From 47900df3e776bbb7b0dd05da3b4d8a0b2a890cd9 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 10:43:08 -0700
Subject: [PATCH 08/21] cleanup

---
 libraries/script-engine/src/ScriptEngine.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h
index 2af4975a8e..7a021dde4f 100644
--- a/libraries/script-engine/src/ScriptEngine.h
+++ b/libraries/script-engine/src/ScriptEngine.h
@@ -71,7 +71,7 @@ public:
     ~ScriptEngine();
 
     // Useful for QSharePointer<ScriptEngine>
-    static void doDeleteLater(ScriptEngine* obj) { obj->deleteLater(); }
+    static void doDeleteLater(ScriptEngine* engine) { engine->deleteLater(); }
 
     /// run the script in a dedicated thread. This will have the side effect of evalulating
     /// the current script contents and calling run(). Callers will likely want to register the script with external

From 1c111e20bd6be7bdd201d5a4052111e076a2f920 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 11:20:26 -0700
Subject: [PATCH 09/21] fix possible thread issue with
 Entities.callEntityMethod()

---
 libraries/entities/src/EntityScriptingInterface.cpp | 6 ++++++
 libraries/entities/src/EntityScriptingInterface.h   | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp
index 540cc68689..697119cf9c 100644
--- a/libraries/entities/src/EntityScriptingInterface.cpp
+++ b/libraries/entities/src/EntityScriptingInterface.cpp
@@ -414,7 +414,13 @@ void EntityScriptingInterface::deleteEntity(QUuid id) {
     }
 }
 
+void EntityScriptingInterface::setEntitiesScriptEngine(EntitiesScriptEngineProvider* engine) {
+    std::unique_lock<std::mutex> lock(_entitiesScriptEngineLock);
+    _entitiesScriptEngine = engine;
+}
+
 void EntityScriptingInterface::callEntityMethod(QUuid id, const QString& method, const QStringList& params) {
+    std::unique_lock<std::mutex> lock(_entitiesScriptEngineLock);
     if (_entitiesScriptEngine) {
         EntityItemID entityID{ id };
         _entitiesScriptEngine->callEntityScriptMethod(entityID, method, params);
diff --git a/libraries/entities/src/EntityScriptingInterface.h b/libraries/entities/src/EntityScriptingInterface.h
index b65bc55dbb..ca6e6fd9ac 100644
--- a/libraries/entities/src/EntityScriptingInterface.h
+++ b/libraries/entities/src/EntityScriptingInterface.h
@@ -71,7 +71,7 @@ public:
 
     void setEntityTree(EntityTreePointer modelTree);
     EntityTreePointer getEntityTree() { return _entityTree; }
-    void setEntitiesScriptEngine(EntitiesScriptEngineProvider* engine) { _entitiesScriptEngine = engine; }
+    void setEntitiesScriptEngine(EntitiesScriptEngineProvider* engine);
     float calculateCost(float mass, float oldVelocity, float newVelocity);
 public slots:
 
@@ -214,6 +214,8 @@ private:
         bool precisionPicking, const QVector<EntityItemID>& entityIdsToInclude, const QVector<EntityItemID>& entityIdsToDiscard);
 
     EntityTreePointer _entityTree;
+
+    std::mutex _entitiesScriptEngineLock;
     EntitiesScriptEngineProvider* _entitiesScriptEngine { nullptr };
     
     bool _bidOnSimulationOwnership { false };

From c26115cf2ef72698921b5eeab8a49d88ced2f2f6 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 11:40:17 -0700
Subject: [PATCH 10/21] one small tweak

---
 libraries/entities-renderer/src/EntityTreeRenderer.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
index da860cd834..d0b2c67c95 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
@@ -78,6 +78,7 @@ EntityTreeRenderer::~EntityTreeRenderer() {
 int EntityTreeRenderer::_entititesScriptEngineCount = 0;
 
 void EntityTreeRenderer::setupEntitiesScriptEngine() {
+    QSharedPointer<ScriptEngine> oldEngine = _entitiesScriptEngine; // save the old engine through this function, so the EntityScriptingInterface doesn't have problems with it.
     _entitiesScriptEngine = QSharedPointer<ScriptEngine>(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entititesScriptEngineCount)), ScriptEngine::doDeleteLater);
     _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data());
     _entitiesScriptEngine->runInThread();

From 0d20f2468e84caf51735dacaeace03dd719eec26 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 12:13:21 -0700
Subject: [PATCH 11/21] CR feedback

---
 interface/src/Application.cpp                          | 1 -
 libraries/entities-renderer/src/EntityTreeRenderer.cpp | 4 ++--
 libraries/entities-renderer/src/EntityTreeRenderer.h   | 2 +-
 libraries/script-engine/src/ScriptEngine.h             | 3 ---
 4 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index 32b8af6ce8..eeed11deee 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -4182,7 +4182,6 @@ void Application::clearDomainOctreeDetails() {
 
     qCDebug(interfaceapp) << "Clearing domain octree details...";
 
-    // reset the environment so that we don't erroneously end up with multiple
     resetPhysicsReadyInformation();
 
     // reset our node to stats and node to jurisdiction maps... since these must be changing...
diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
index d0b2c67c95..9de2fc1525 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
@@ -75,11 +75,11 @@ EntityTreeRenderer::~EntityTreeRenderer() {
     //       it is registered with ScriptEngines, which will call deleteLater for us.
 }
 
-int EntityTreeRenderer::_entititesScriptEngineCount = 0;
+int EntityTreeRenderer::_entitiesScriptEngineCount = 0;
 
 void EntityTreeRenderer::setupEntitiesScriptEngine() {
     QSharedPointer<ScriptEngine> oldEngine = _entitiesScriptEngine; // save the old engine through this function, so the EntityScriptingInterface doesn't have problems with it.
-    _entitiesScriptEngine = QSharedPointer<ScriptEngine>(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entititesScriptEngineCount)), ScriptEngine::doDeleteLater);
+    _entitiesScriptEngine = QSharedPointer<ScriptEngine>(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++_entitiesScriptEngineCount)), &QObject::deleteLater);
     _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data());
     _entitiesScriptEngine->runInThread();
     DependencyManager::get<EntityScriptingInterface>()->setEntitiesScriptEngine(_entitiesScriptEngine.data());
diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.h b/libraries/entities-renderer/src/EntityTreeRenderer.h
index f1b581d8ef..a6fc58e5f1 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.h
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.h
@@ -199,7 +199,7 @@ private:
     // For Scene.shouldRenderEntities
     QList<EntityItemID> _entityIDsLastInScene;
 
-    static int _entititesScriptEngineCount;
+    static int _entitiesScriptEngineCount;
 };
 
 
diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h
index 7a021dde4f..e8ce00c66c 100644
--- a/libraries/script-engine/src/ScriptEngine.h
+++ b/libraries/script-engine/src/ScriptEngine.h
@@ -70,9 +70,6 @@ public:
 
     ~ScriptEngine();
 
-    // Useful for QSharePointer<ScriptEngine>
-    static void doDeleteLater(ScriptEngine* engine) { engine->deleteLater(); }
-
     /// run the script in a dedicated thread. This will have the side effect of evalulating
     /// the current script contents and calling run(). Callers will likely want to register the script with external
     /// services before calling this.

From 374ba105249284c6e7a66e2882a6542041e4ef49 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 12:23:36 -0700
Subject: [PATCH 12/21] more CR feedback

---
 interface/src/Application.cpp                          | 1 -
 libraries/entities-renderer/src/EntityTreeRenderer.cpp | 2 ++
 libraries/entities/src/EntityScriptingInterface.cpp    | 4 ++--
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index eeed11deee..ce673f444f 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -1188,7 +1188,6 @@ void Application::cleanupBeforeQuit() {
     nodeList->getPacketReceiver().setShouldDropPackets(true);
 
     getEntities()->shutdown(); // tell the entities system we're shutting down, so it will stop running scripts
-    getEntities()->clear(); // this will allow entity scripts to properly shutdown
 
     DependencyManager::get<ScriptEngines>()->saveScripts();
     DependencyManager::get<ScriptEngines>()->shutdownScripting(); // stop all currently running global scripts
diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
index 9de2fc1525..abba0fe7fe 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
@@ -140,6 +140,8 @@ void EntityTreeRenderer::init() {
 void EntityTreeRenderer::shutdown() {
     _entitiesScriptEngine->disconnectNonEssentialSignals(); // disconnect all slots/signals from the script engine, except essential
     _shuttingDown = true;
+
+    clear(); // always clear() on shutdown
 }
 
 void EntityTreeRenderer::setTree(OctreePointer newTree) {
diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp
index 697119cf9c..8213316b7b 100644
--- a/libraries/entities/src/EntityScriptingInterface.cpp
+++ b/libraries/entities/src/EntityScriptingInterface.cpp
@@ -415,12 +415,12 @@ void EntityScriptingInterface::deleteEntity(QUuid id) {
 }
 
 void EntityScriptingInterface::setEntitiesScriptEngine(EntitiesScriptEngineProvider* engine) {
-    std::unique_lock<std::mutex> lock(_entitiesScriptEngineLock);
+    std::lock_guard<std::mutex> lock(_entitiesScriptEngineLock);
     _entitiesScriptEngine = engine;
 }
 
 void EntityScriptingInterface::callEntityMethod(QUuid id, const QString& method, const QStringList& params) {
-    std::unique_lock<std::mutex> lock(_entitiesScriptEngineLock);
+    std::lock_guard<std::mutex> lock(_entitiesScriptEngineLock);
     if (_entitiesScriptEngine) {
         EntityItemID entityID{ id };
         _entitiesScriptEngine->callEntityScriptMethod(entityID, method, params);

From 9e13b1bbaef7fd3cbb06d62cfcc01a7396c9cf10 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 12:25:46 -0700
Subject: [PATCH 13/21] more CR feedback

---
 libraries/script-engine/src/ScriptEngine.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index 2aab250ef4..610f542397 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -1049,8 +1049,8 @@ void ScriptEngine::forwardHandlerCall(const EntityItemID& entityID, const QStrin
 // since all of these operations can be asynch we will always do the actual work in the response handler
 // for the download
 void ScriptEngine::loadEntityScript(QWeakPointer<ScriptEngine> theEngine, const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) {
-    // NOTE: If the script content is not currently in the caceh, The LAMBDA here, will be called on the Main Thread
-    //       which means we're guarenteed that it's not the correct thread for the ScriptEngine. This means
+    // NOTE: If the script content is not currently in the cache, the LAMBDA here will be called on the Main Thread
+    //       which means we're guaranteed that it's not the correct thread for the ScriptEngine. This means
     //       when we get into entityScriptContentAvailable() we will likely invokeMethod() to get it over
     //       to the "Entities" ScriptEngine thread.
     DependencyManager::get<ScriptCache>()->getScriptContents(entityScript, [theEngine, entityID](const QString& scriptOrURL, const QString& contents, bool isURL, bool success) {

From 55010cebb70a0898b5552dd1346aa558acd582c8 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 12:43:32 -0700
Subject: [PATCH 14/21] remove unhelpful comment

---
 libraries/entities-renderer/src/EntityTreeRenderer.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
index abba0fe7fe..09f50c5de3 100644
--- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp
+++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp
@@ -92,7 +92,6 @@ void EntityTreeRenderer::clear() {
         _entitiesScriptEngine->stop();
     }
 
-    // this would be a good place to actuall delete and recreate the _entitiesScriptEngine
     if (_wantScripts && !_shuttingDown) {
         // NOTE: you can't actually need to delete it here because when we call setupEntitiesScriptEngine it will
         //       assign a new instance to our shared pointer, which will deref the old instance and ultimately call

From bf8cdabd1bf965e80f35728ece99c58a006dacb0 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 12:50:29 -0700
Subject: [PATCH 15/21] CR feedback

---
 libraries/script-engine/src/ScriptEngine.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index 610f542397..cbbc8832b6 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -1065,7 +1065,7 @@ void ScriptEngine::loadEntityScript(QWeakPointer<ScriptEngine> theEngine, const
             // FIXME - I'm leaving this in for testing, so that QA can confirm that sometimes the script contents
             //         returns after the ScriptEngine has been deleted, we can remove this after QA verifies the
             //         repro case.
-            qDebug() << "ScriptCache::getScriptContents() returned after our ScriptEngine was deleted...";
+            qDebug() << "ScriptCache::getScriptContents() returned after our ScriptEngine was deleted... script:" << scriptOrURL;
         }
     }, forceRedownload);
 }

From 6ac173d19701fa5087db1e5e4988950c04d5e257 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 13:27:05 -0700
Subject: [PATCH 16/21] add an example script

---
 examples/entityScripts/exampleUpdate.js | 54 +++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 examples/entityScripts/exampleUpdate.js

diff --git a/examples/entityScripts/exampleUpdate.js b/examples/entityScripts/exampleUpdate.js
new file mode 100644
index 0000000000..c8a18ac774
--- /dev/null
+++ b/examples/entityScripts/exampleUpdate.js
@@ -0,0 +1,54 @@
+//
+//  exampleUpdate.js
+//  examples/entityScripts
+//
+//  Created by Brad Hefta-Gaub on 4/18/16.
+//  Copyright 2016 High Fidelity, Inc.
+//
+//  This is an example of an entity script which hooks the update signal
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+(function() {
+    var _this;
+
+    // this is the "constructor" for the entity as a JS object we don't do much here, but we do want to remember
+    // our this object, so we can access it in cases where we're called without a this (like in the case of various global signals)
+    ExampleUpdate = function() { 
+         _this = this;
+    };
+
+    ExampleUpdate.prototype = {
+
+        // update() will be called regulary, because we've hooked the update signal in our preload() function.
+        // we will check the avatars hand positions and if either hand is in our bounding box, we will notice that
+        update: function() {
+            // because the update() signal doesn't have a valid this, we need to use our memorized _this to access our entityID
+            var entityID = _this.entityID;
+            print("update in entityID:" + entityID);
+        },
+
+        // preload() will be called when the entity has become visible (or known) to the interface
+        // it gives us a chance to set our local JavaScript object up. In this case it means:
+        //   * remembering our entityID, so we can access it in cases where we're called without an entityID
+        //   * connecting to the update signal so we can check our grabbed state
+        preload: function(entityID) {
+            print("preload - entityID:" + entityID);
+            this.entityID = entityID;
+            Script.update.connect(this.update);
+        },
+
+        // unload() will be called when our entity is no longer available. It may be because we were deleted,
+        // or because we've left the domain or quit the application. In all cases we want to unhook our connection
+        // to the update signal
+        unload: function(entityID) {
+            print("unload - entityID:" + entityID);
+            Script.update.disconnect(this.update);
+        },
+    };
+
+    // entity scripts always need to return a newly constructed object of our type
+    return new ExampleUpdate();
+})

From 24003840df35ceb70fd9cc90062789c704a5dab3 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 13:33:08 -0700
Subject: [PATCH 17/21] add an example script

---
 .../exampleUpdateNoDisconnect.js              | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 examples/entityScripts/exampleUpdateNoDisconnect.js

diff --git a/examples/entityScripts/exampleUpdateNoDisconnect.js b/examples/entityScripts/exampleUpdateNoDisconnect.js
new file mode 100644
index 0000000000..81bb742ca2
--- /dev/null
+++ b/examples/entityScripts/exampleUpdateNoDisconnect.js
@@ -0,0 +1,54 @@
+//
+//  exampleUpdateNoDisconnect.js
+//  examples/entityScripts
+//
+//  Created by Brad Hefta-Gaub on 4/18/16.
+//  Copyright 2016 High Fidelity, Inc.
+//
+//  This is an example of an entity script which hooks the update signal
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+(function() {
+    var _this;
+
+    // this is the "constructor" for the entity as a JS object we don't do much here, but we do want to remember
+    // our this object, so we can access it in cases where we're called without a this (like in the case of various global signals)
+    ExampleUpdate = function() { 
+         _this = this;
+    };
+
+    ExampleUpdate.prototype = {
+
+        // update() will be called regulary, because we've hooked the update signal in our preload() function.
+        // we will check the avatars hand positions and if either hand is in our bounding box, we will notice that
+        update: function() {
+            // because the update() signal doesn't have a valid this, we need to use our memorized _this to access our entityID
+            var entityID = _this.entityID;
+            print("update in entityID:" + entityID);
+        },
+
+        // preload() will be called when the entity has become visible (or known) to the interface
+        // it gives us a chance to set our local JavaScript object up. In this case it means:
+        //   * remembering our entityID, so we can access it in cases where we're called without an entityID
+        //   * connecting to the update signal so we can check our grabbed state
+        preload: function(entityID) {
+            print("preload - entityID:" + entityID);
+            this.entityID = entityID;
+            Script.update.connect(this.update);
+        },
+
+        // unload() will be called when our entity is no longer available. It may be because we were deleted,
+        // or because we've left the domain or quit the application. In all cases we want to unhook our connection
+        // to the update signal
+        unload: function(entityID) {
+            print("unload - entityID:" + entityID);
+            print("NOTE --- WE DID NOT CALL Script.update.disconnect()");
+        },
+    };
+
+    // entity scripts always need to return a newly constructed object of our type
+    return new ExampleUpdate();
+})

From d6fa58c39c9feeb4b4a201f8dd9324472bb3e3d7 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 13:42:58 -0700
Subject: [PATCH 18/21] add an example script

---
 .../entityScripts/exampleTimeoutNoCleanup.js  | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 examples/entityScripts/exampleTimeoutNoCleanup.js

diff --git a/examples/entityScripts/exampleTimeoutNoCleanup.js b/examples/entityScripts/exampleTimeoutNoCleanup.js
new file mode 100644
index 0000000000..c05a7223ec
--- /dev/null
+++ b/examples/entityScripts/exampleTimeoutNoCleanup.js
@@ -0,0 +1,50 @@
+//
+//  exampleTimeoutNoCleanup.js
+//  examples/entityScripts
+//
+//  Created by Brad Hefta-Gaub on 4/18/16.
+//  Copyright 2016 High Fidelity, Inc.
+//
+//  This is an example of an entity script which hooks the update signal
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+(function() {
+    var _this;
+
+    // this is the "constructor" for the entity as a JS object we don't do much here, but we do want to remember
+    // our this object, so we can access it in cases where we're called without a this (like in the case of various global signals)
+    ExampleUpdate = function() { 
+         _this = this;
+    };
+
+    ExampleUpdate.prototype = {
+
+        // preload() will be called when the entity has become visible (or known) to the interface
+        // it gives us a chance to set our local JavaScript object up. In this case it means:
+        //   * remembering our entityID, so we can access it in cases where we're called without an entityID
+        //   * connecting to the update signal so we can check our grabbed state
+        preload: function(entityID) {
+            print("preload - entityID:" + entityID);
+            this.entityID = entityID;
+
+            Script.setTimeout(function() {
+                    var entityID = _this.entityID;
+                    print("timer timeout in entityID:" + entityID);
+                }, 3000);
+        },
+
+        // unload() will be called when our entity is no longer available. It may be because we were deleted,
+        // or because we've left the domain or quit the application. In all cases we want to unhook our connection
+        // to the update signal
+        unload: function(entityID) {
+            print("unload - entityID:" + entityID);
+            print("NOTE --- WE DID NOT CALL clear our timeout");
+        },
+    };
+
+    // entity scripts always need to return a newly constructed object of our type
+    return new ExampleUpdate();
+})

From ab7f3d573984ae41f1b772c4fa3f1b392a036e4f Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 13:58:40 -0700
Subject: [PATCH 19/21] change the timeout to an interval so it constantly runs

---
 examples/entityScripts/exampleTimeoutNoCleanup.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/entityScripts/exampleTimeoutNoCleanup.js b/examples/entityScripts/exampleTimeoutNoCleanup.js
index c05a7223ec..99b8816388 100644
--- a/examples/entityScripts/exampleTimeoutNoCleanup.js
+++ b/examples/entityScripts/exampleTimeoutNoCleanup.js
@@ -30,9 +30,9 @@
             print("preload - entityID:" + entityID);
             this.entityID = entityID;
 
-            Script.setTimeout(function() {
+            Script.setInterval(function() {
                     var entityID = _this.entityID;
-                    print("timer timeout in entityID:" + entityID);
+                    print("timer interval in entityID:" + entityID);
                 }, 3000);
         },
 

From c3a86f46d07f1bee738f19b8f8cbc5b1e2d362de Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 14:55:23 -0700
Subject: [PATCH 20/21] change the timeout to an interval so it constantly runs

---
 .../exampleSelfCallingTimeoutNoCleanup.js     | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 examples/entityScripts/exampleSelfCallingTimeoutNoCleanup.js

diff --git a/examples/entityScripts/exampleSelfCallingTimeoutNoCleanup.js b/examples/entityScripts/exampleSelfCallingTimeoutNoCleanup.js
new file mode 100644
index 0000000000..a4a66fc785
--- /dev/null
+++ b/examples/entityScripts/exampleSelfCallingTimeoutNoCleanup.js
@@ -0,0 +1,56 @@
+//
+//  exampleSelfCallingTimeoutNoCleanup.js
+//  examples/entityScripts
+//
+//  Created by Brad Hefta-Gaub on 4/18/16.
+//  Copyright 2016 High Fidelity, Inc.
+//
+//  This is an example of an entity script which hooks the update signal
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+(function() {
+    var _this;
+
+    // this is the "constructor" for the entity as a JS object we don't do much here, but we do want to remember
+    // our this object, so we can access it in cases where we're called without a this (like in the case of various global signals)
+    ExampleUpdate = function() { 
+         _this = this;
+    };
+
+    ExampleUpdate.prototype = {
+
+        timeOutFunction: function() {
+            var entityID = _this.entityID;
+            print("timeOutFunction in entityID:" + entityID);
+            Script.setTimeout(function() {
+                _this.timeOutFunction();
+            }, 3000);
+        },
+
+        // preload() will be called when the entity has become visible (or known) to the interface
+        // it gives us a chance to set our local JavaScript object up. In this case it means:
+        //   * remembering our entityID, so we can access it in cases where we're called without an entityID
+        //   * connecting to the update signal so we can check our grabbed state
+        preload: function(entityID) {
+            print("preload - entityID:" + entityID);
+            this.entityID = entityID;
+
+            print("preload - entityID:" + entityID + "-- calling timeOutFunction()....");
+            _this.timeOutFunction();
+        },
+
+        // unload() will be called when our entity is no longer available. It may be because we were deleted,
+        // or because we've left the domain or quit the application. In all cases we want to unhook our connection
+        // to the update signal
+        unload: function(entityID) {
+            print("unload - entityID:" + entityID);
+            print("NOTE --- WE DID NOT CALL clear our timeout");
+        },
+    };
+
+    // entity scripts always need to return a newly constructed object of our type
+    return new ExampleUpdate();
+})

From d5891e9073ec21abeae0448de448e312a2dc5f86 Mon Sep 17 00:00:00 2001
From: Brad Hefta-Gaub <brad@highfidelity.io>
Date: Mon, 18 Apr 2016 15:28:33 -0700
Subject: [PATCH 21/21] remove abortEvaluation() since it has no effect for the
 test cases and might cause unexpected side effects

---
 libraries/script-engine/src/ScriptEngine.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp
index cbbc8832b6..15f3ebb985 100644
--- a/libraries/script-engine/src/ScriptEngine.cpp
+++ b/libraries/script-engine/src/ScriptEngine.cpp
@@ -758,7 +758,6 @@ void ScriptEngine::stop() {
             QMetaObject::invokeMethod(this, "stop");
             return;
         }
-        abortEvaluation(); // abort any script evaluation that may be active
         _isFinished = true;
         if (_wantSignals) {
             emit runningStateChanged();