switch to using QSharedPointer for _entitiesScriptEngine

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();
         }
     }

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);

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 ["

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