diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 7f2a992f68..cfd155e14b 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -1974,8 +1974,9 @@ void ScriptEngine::forwardHandlerCall(const EntityItemID& entityID, const QStrin } int ScriptEngine::getNumRunningEntityScripts() const { + QReadLocker locker { &_entityScriptsLock }; int sum = 0; - for (auto& st : _entityScripts) { + for (const auto& st : _entityScripts) { if (st.status == EntityScriptStatus::RUNNING) { ++sum; } @@ -1984,14 +1985,20 @@ int ScriptEngine::getNumRunningEntityScripts() const { } void ScriptEngine::setEntityScriptDetails(const EntityItemID& entityID, const EntityScriptDetails& details) { - _entityScripts[entityID] = details; + { + QWriteLocker locker { &_entityScriptsLock }; + _entityScripts[entityID] = details; + } emit entityScriptDetailsUpdated(); } void ScriptEngine::updateEntityScriptStatus(const EntityItemID& entityID, const EntityScriptStatus &status, const QString& errorInfo) { - EntityScriptDetails &details = _entityScripts[entityID]; - details.status = status; - details.errorInfo = errorInfo; + { + QWriteLocker locker { &_entityScriptsLock }; + EntityScriptDetails& details = _entityScripts[entityID]; + details.status = status; + details.errorInfo = errorInfo; + } emit entityScriptDetailsUpdated(); } @@ -2042,6 +2049,7 @@ QFuture ScriptEngine::getLocalEntityScriptDetails(const EntityItemID& } bool ScriptEngine::getEntityScriptDetails(const EntityItemID& entityID, EntityScriptDetails &details) const { + QReadLocker locker { &_entityScriptsLock }; auto it = _entityScripts.constFind(entityID); if (it == _entityScripts.constEnd()) { return false; @@ -2050,6 +2058,11 @@ bool ScriptEngine::getEntityScriptDetails(const EntityItemID& entityID, EntitySc return true; } +bool ScriptEngine::hasEntityScriptDetails(const EntityItemID& entityID) const { + QReadLocker locker { &_entityScriptsLock }; + return _entityScripts.contains(entityID); +} + const static EntityItemID BAD_SCRIPT_UUID_PLACEHOLDER { "{20170224-dead-face-0000-cee000021114}" }; void ScriptEngine::processDeferredEntityLoads(const QString& entityScript, const EntityItemID& leaderID) { @@ -2064,14 +2077,15 @@ void ScriptEngine::processDeferredEntityLoads(const QString& entityScript, const } foreach(DeferredLoadEntity retry, retryLoads) { // check whether entity was since been deleted - if (!_entityScripts.contains(retry.entityID)) { + + EntityScriptDetails details; + if (!getEntityScriptDetails(retry.entityID, details)) { qCDebug(scriptengine) << "processDeferredEntityLoads -- entity details gone (entity deleted?)" << retry.entityID; continue; } // check whether entity has since been unloaded or otherwise errored-out - auto details = _entityScripts[retry.entityID]; if (details.status != EntityScriptStatus::PENDING) { qCDebug(scriptengine) << "processDeferredEntityLoads -- entity status no longer PENDING; " << retry.entityID << details.status; @@ -2079,7 +2093,11 @@ void ScriptEngine::processDeferredEntityLoads(const QString& entityScript, const } // propagate leader's failure reasons to the pending entity - const auto leaderDetails = _entityScripts[leaderID]; + EntityScriptDetails leaderDetails; + { + QWriteLocker locker { &_entityScriptsLock }; + leaderDetails = _entityScripts[leaderID]; + } if (leaderDetails.status != EntityScriptStatus::RUNNING) { qCDebug(scriptengine) << QString("... pending load of %1 cancelled (leader: %2 status: %3)") .arg(retry.entityID.toString()).arg(leaderID.toString()).arg(leaderDetails.status); @@ -2125,7 +2143,7 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& return; } - if (!_entityScripts.contains(entityID)) { + if (!hasEntityScriptDetails(entityID)) { // make sure EntityScriptDetails has an entry for this UUID right away // (which allows bailing from the loading/provisioning process early if the Entity gets deleted mid-flight) updateEntityScriptStatus(entityID, EntityScriptStatus::PENDING, "...pending..."); @@ -2166,9 +2184,12 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& _occupiedScriptURLs[entityScript] = entityID; #ifdef DEBUG_ENTITY_STATES - auto previousStatus = _entityScripts.contains(entityID) ? _entityScripts[entityID].status : EntityScriptStatus::PENDING; - qCDebug(scriptengine) << "loadEntityScript.LOADING: " << entityScript << entityID.toString() - << "(previous: " << previousStatus << ")"; + { + EntityScriptDetails details; + bool hasEntityScript = getEntityScriptDetails(entityID, details); + qCDebug(scriptengine) << "loadEntityScript.LOADING: " << entityScript << entityID.toString() + << "(previous: " << (hasEntityScript ? details.status : EntityScriptStatus::PENDING) << ")"; + } #endif EntityScriptDetails newDetails; @@ -2197,7 +2218,7 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& #ifdef DEBUG_ENTITY_STATES qCDebug(scriptengine) << "loadEntityScript.contentAvailable" << status << QUrl(url).fileName() << entityID.toString(); #endif - if (!isStopping() && _entityScripts.contains(entityID)) { + if (!isStopping() && hasEntityScriptDetails(entityID)) { _contentAvailableQueue[entityID] = { entityID, url, contents, isURL, success, status }; } else { #ifdef DEBUG_ENTITY_STATES @@ -2267,8 +2288,11 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co bool isFileUrl = isURL && scriptOrURL.startsWith("file://"); auto fileName = isURL ? scriptOrURL : "about:EmbeddedEntityScript"; - const EntityScriptDetails &oldDetails = _entityScripts[entityID]; - const QString entityScript = oldDetails.scriptText; + QString entityScript; + { + QWriteLocker locker { &_entityScriptsLock }; + entityScript = _entityScripts[entityID].scriptText; + } EntityScriptDetails newDetails; newDetails.scriptText = scriptOrURL; @@ -2446,8 +2470,8 @@ void ScriptEngine::unloadEntityScript(const EntityItemID& entityID, bool shouldR "entityID:" << entityID; #endif - if (_entityScripts.contains(entityID)) { - const EntityScriptDetails &oldDetails = _entityScripts[entityID]; + EntityScriptDetails oldDetails; + if (getEntityScriptDetails(entityID, oldDetails)) { auto scriptText = oldDetails.scriptText; if (isEntityScriptRunning(entityID)) { @@ -2460,7 +2484,10 @@ void ScriptEngine::unloadEntityScript(const EntityItemID& entityID, bool shouldR #endif if (shouldRemoveFromMap) { // this was a deleted entity, we've been asked to remove it from the map - _entityScripts.remove(entityID); + { + QWriteLocker locker { &_entityScriptsLock }; + _entityScripts.remove(entityID); + } emit entityScriptDetailsUpdated(); } else if (oldDetails.status != EntityScriptStatus::UNLOADED) { EntityScriptDetails newDetails; @@ -2491,10 +2518,19 @@ void ScriptEngine::unloadAllEntityScripts() { #ifdef THREAD_DEBUGGING qCDebug(scriptengine) << "ScriptEngine::unloadAllEntityScripts() called on correct thread [" << thread() << "]"; #endif - foreach(const EntityItemID& entityID, _entityScripts.keys()) { + + QList keys; + { + QReadLocker locker{ &_entityScriptsLock }; + keys = _entityScripts.keys(); + } + foreach(const EntityItemID& entityID, keys) { unloadEntityScript(entityID); } - _entityScripts.clear(); + { + QWriteLocker locker{ &_entityScriptsLock }; + _entityScripts.clear(); + } emit entityScriptDetailsUpdated(); _occupiedScriptURLs.clear(); @@ -2508,7 +2544,7 @@ void ScriptEngine::unloadAllEntityScripts() { } void ScriptEngine::refreshFileScript(const EntityItemID& entityID) { - if (!HIFI_AUTOREFRESH_FILE_SCRIPTS || !_entityScripts.contains(entityID)) { + if (!HIFI_AUTOREFRESH_FILE_SCRIPTS || !hasEntityScriptDetails(entityID)) { return; } @@ -2518,7 +2554,11 @@ void ScriptEngine::refreshFileScript(const EntityItemID& entityID) { } recurseGuard = true; - EntityScriptDetails details = _entityScripts[entityID]; + EntityScriptDetails details; + { + QWriteLocker locker { &_entityScriptsLock }; + details = _entityScripts[entityID]; + } // Check to see if a file based script needs to be reloaded (easier debugging) if (details.lastModified > 0) { QString filePath = QUrl(details.scriptText).toLocalFile(); @@ -2584,7 +2624,11 @@ void ScriptEngine::callEntityScriptMethod(const EntityItemID& entityID, const QS refreshFileScript(entityID); } if (isEntityScriptRunning(entityID)) { - EntityScriptDetails details = _entityScripts[entityID]; + EntityScriptDetails details; + { + QWriteLocker locker { &_entityScriptsLock }; + details = _entityScripts[entityID]; + } QScriptValue entityScript = details.scriptObject; // previously loaded // If this is a remote call, we need to check to see if the function is remotely callable @@ -2646,7 +2690,11 @@ void ScriptEngine::callEntityScriptMethod(const EntityItemID& entityID, const QS refreshFileScript(entityID); } if (isEntityScriptRunning(entityID)) { - EntityScriptDetails details = _entityScripts[entityID]; + EntityScriptDetails details; + { + QWriteLocker locker { &_entityScriptsLock }; + details = _entityScripts[entityID]; + } QScriptValue entityScript = details.scriptObject; // previously loaded if (entityScript.property(methodName).isFunction()) { QScriptValueList args; @@ -2680,7 +2728,11 @@ void ScriptEngine::callEntityScriptMethod(const EntityItemID& entityID, const QS refreshFileScript(entityID); } if (isEntityScriptRunning(entityID)) { - EntityScriptDetails details = _entityScripts[entityID]; + EntityScriptDetails details; + { + QWriteLocker locker { &_entityScriptsLock }; + details = _entityScripts[entityID]; + } QScriptValue entityScript = details.scriptObject; // previously loaded if (entityScript.property(methodName).isFunction()) { QScriptValueList args; diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 94b50bfd2c..08e2c492e8 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -461,7 +461,9 @@ public: * @returns {boolean} */ Q_INVOKABLE bool isEntityScriptRunning(const EntityItemID& entityID) { - return _entityScripts.contains(entityID) && _entityScripts[entityID].status == EntityScriptStatus::RUNNING; + QReadLocker locker { &_entityScriptsLock }; + auto it = _entityScripts.constFind(entityID); + return it != _entityScripts.constEnd() && it->status == EntityScriptStatus::RUNNING; } QVariant cloneEntityScriptDetails(const EntityItemID& entityID); QFuture getLocalEntityScriptDetails(const EntityItemID& entityID) override; @@ -559,6 +561,7 @@ public: void clearDebugLogWindow(); int getNumRunningEntityScripts() const; bool getEntityScriptDetails(const EntityItemID& entityID, EntityScriptDetails &details) const; + bool hasEntityScriptDetails(const EntityItemID& entityID) const; public slots: @@ -771,6 +774,7 @@ protected: bool _isInitialized { false }; QHash _timerFunctionMap; QSet _includedURLs; + mutable QReadWriteLock _entityScriptsLock { QReadWriteLock::Recursive }; QHash _entityScripts; QHash _occupiedScriptURLs; QList _deferredEntityLoads;