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(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)), ScriptEngine::doDeleteLater); + + + _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); _entitiesScriptEngine->runInThread(); - DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine); + DependencyManager::get()->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(new ScriptEngine(NO_SCRIPT, QString("Entities %1").arg(++entititesScriptEngineCount)), ScriptEngine::doDeleteLater); + _scriptingServices->registerScriptEngineWithApplicationServices(_entitiesScriptEngine.data()); _entitiesScriptEngine->runInThread(); - DependencyManager::get()->setEntitiesScriptEngine(_entitiesScriptEngine); + DependencyManager::get()->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 _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(); @@ -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 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 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()->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 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 + 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 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 _stoppingAllScripts; }; +/* +class ScriptEngine : public QObject { + Q_OBJECT +}; +*/ + + #endif // hifi_ScriptEngine_h