Add lock around _entityScripts

This commit is contained in:
Clement 2018-09-13 12:43:14 -07:00 committed by Atlante45
parent d5456eda6c
commit e6b4ccef16
2 changed files with 82 additions and 26 deletions

View file

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

View file

@ -461,7 +461,9 @@ public:
* @returns {boolean} * @returns {boolean}
*/ */
Q_INVOKABLE bool isEntityScriptRunning(const EntityItemID& entityID) { 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); QVariant cloneEntityScriptDetails(const EntityItemID& entityID);
QFuture<QVariant> getLocalEntityScriptDetails(const EntityItemID& entityID) override; QFuture<QVariant> getLocalEntityScriptDetails(const EntityItemID& entityID) override;
@ -559,6 +561,7 @@ public:
void clearDebugLogWindow(); void clearDebugLogWindow();
int getNumRunningEntityScripts() const; int getNumRunningEntityScripts() const;
bool getEntityScriptDetails(const EntityItemID& entityID, EntityScriptDetails &details) const; bool getEntityScriptDetails(const EntityItemID& entityID, EntityScriptDetails &details) const;
bool hasEntityScriptDetails(const EntityItemID& entityID) const;
public slots: public slots:
@ -771,6 +774,7 @@ protected:
bool _isInitialized { false }; bool _isInitialized { false };
QHash<QTimer*, CallbackData> _timerFunctionMap; QHash<QTimer*, CallbackData> _timerFunctionMap;
QSet<QUrl> _includedURLs; QSet<QUrl> _includedURLs;
mutable QReadWriteLock _entityScriptsLock { QReadWriteLock::Recursive };
QHash<EntityItemID, EntityScriptDetails> _entityScripts; QHash<EntityItemID, EntityScriptDetails> _entityScripts;
QHash<QString, EntityItemID> _occupiedScriptURLs; QHash<QString, EntityItemID> _occupiedScriptURLs;
QList<DeferredLoadEntity> _deferredEntityLoads; QList<DeferredLoadEntity> _deferredEntityLoads;