Adapt from std::map locks to simple QHash for flagging busy scriptURLs

humbletim committed 9 years ago

Commit c89d2031 parent 3ad3fd42 files · +44 −27Browse files

Showing the changes to libraries/script-engine/src/ScriptEngine.cpp (1 of 2 files). Show all

Unified · Split

libraries/script-engine/src/ScriptEngine.cpp+43−26

 const auto RETRY_ATTEMPT_BATCH = 100; //glm::clamp(qgetenv("RETRY_ATTEMPT_BATCH").toInt(), 1, 50);
 const auto RETRY_ATTEMPT_MSECS = 250; //glm::clamp(qgetenv("RETRY_ATTEMPT_MSECS").toInt(), 50, 1000);
+const static EntityItemID BAD_SCRIPT_UUID_PLACEHOLDER { "{20170224-dead-face-0000-cee000021114}" };
 void ScriptEngine::deferLoadEntityScript(const EntityItemID& entityID, const QString& entityScript, bool forceRedownload) {
     const auto needTimer = _pendingLoadEntities.isEmpty();
                 if (details.status != EntityScriptStatus::PENDING) {
                     qCDebug(scriptengine) << "deferLoadEntityScript.retry -- entity status no longer PENDING; " << details.status;
                 } else {
-                    auto &locker = _lockPerUniqueScriptURL[retry.entityScript];
+                    if (!_occupiedScriptURLs.contains(retry.entityScript)) {
-                    if (locker.tryLock()) {
+                        // the URL is no longer busy with another Entity, so now it's our turn
-                        locker.unlock();
                         loadEntityScript(retry.entityID, retry.entityScript, false);
                     } else {
-                        stillPending << retry;
+                        auto entityID = _occupiedScriptURLs[retry.entityScript];
+                        if (entityID == BAD_SCRIPT_UUID_PLACEHOLDER) {
+                            // the URL was marked bad by an earlier load attempt, so abort the retry mission
+                            qCDebug(scriptengine) << "... pending load of " << retry.entityID << " cancelled";
+                        } else {
+                            // the URL is still occupied by another Entity, so add to next retry pass
+                            stillPending << retry;
+                        }
                     }
                 }
             }
         // processDeferredEntities gets wrapped as a "regular" JavaScript callback so that it can then
         // be scheduled using setInterval per usual -- allowing the script engine to own its lifecycle.
-        // we stil need to call clearInterval manually when done, so its passed to calle() above as a property.
+        // we stil need to call clearInterval manually when done, so its passed to callee() above as a property.
         QScriptValue function = newLambdaFunction(processDeferredEntities, "processDeferredEntities-loader");
         QObject *interval = setInterval( function, RETRY_ATTEMPT_MSECS );
         updateEntityScriptStatus(entityID, EntityScriptStatus::PENDING, "...pending...");
     }
-    // This lock/defer approach allows multiple Entities to boot from the same script URL while still taking
+    // This "occupied" approach allows multiple Entities to boot from the same script URL while still taking
     // full advantage of cacheable require modules.  This only affects Entities literally coming in back-to-back
     // before the first one has time to finish loading.
-    auto &locker = _lockPerUniqueScriptURL[entityScript];
+    if (!_occupiedScriptURLs.contains(entityScript)) {
-    if (!locker.tryLock()) {
+        // the scriptURL is available -- flag as in-use and proceed
-        qCDebug(scriptengine) << QString("loadEntityScript.deferring[%1] entity: %2 script: %3")
+        _occupiedScriptURLs[entityScript] = entityID;
-            .arg( _pendingLoadEntities.size()).arg(entityID.toString()).arg(entityScript);
+    } else {
-        deferLoadEntityScript(entityID, entityScript, forceRedownload);
+        // another Entity is either still booting from this script URL or has failed and marked it "out of order"
+        auto currentEntityID = _occupiedScriptURLs[entityScript];
+        if (currentEntityID == BAD_SCRIPT_UUID_PLACEHOLDER && !forceRedownload) {
+            // since not force-reloading, assume that the exact same input would produce the exact same output again
+            // note: this state gets reset with "reload all scripts," leaving/returning to a Domain, clear cache, etc.
+            qCDebug(scriptengine) << QString("loadEntityScript.cancelled entity: %1 script: %2 (previous script failure)")
+                .arg(entityID.toString()).arg(entityScript);
+            updateEntityScriptStatus(entityID, EntityScriptStatus::ERROR_LOADING_SCRIPT,
+                                     "Cached Script was marked invalid by previous Entity load failure.");
+        } else {
+            // another entity is busy loading from this script URL so wait for them to finish
+            qCDebug(scriptengine) << QString("loadEntityScript.deferring[%0] entity: %1 script: %2")
+                .arg( _pendingLoadEntities.size()).arg(entityID.toString()).arg(entityScript);
+            deferLoadEntityScript(entityID, entityScript, forceRedownload);
+        }
         return;
     }
     newDetails.scriptText = entityScript;
     newDetails.status = EntityScriptStatus::LOADING;
     newDetails.definingSandboxURL = currentSandboxURL;
-    _entityScripts[entityID] = newDetails;
+    setEntityScriptDetails(entityID, newDetails);
     auto scriptCache = DependencyManager::get<ScriptCache>();
     scriptCache->getScriptContents(entityScript,
-        [=, &locker](const QString& url, const QString& contents, bool isURL, bool success, const QString& status) {
+        [this, entityScript, entityID](const QString& url, const QString& contents, bool isURL, bool success, const QString& status) {
             if (isStopping()) {
 #ifdef DEBUG_ENTITY_STATES
                 qCDebug(scriptengine) << "loadEntityScript.contentAvailable -- stopping";
 #endif
-                locker.unlock();
                 return;
             }
-            executeOnScriptThread([=, &locker]{
+            executeOnScriptThread([=]{
 #ifdef DEBUG_ENTITY_STATES
                 qCDebug(scriptengine) << "loadEntityScript.contentAvailable" << status << QUrl(url).fileName() << entityID.toString();
 #endif
                     qCDebug(scriptengine) << "loadEntityScript.contentAvailable -- aborting";
 #endif
                 }
-                locker.unlock();
+                // recheck whether us since may have been set to BAD_SCRIPT_UUID_PLACEHOLDER in entityScriptContentAvailable
+                if (_occupiedScriptURLs[entityScript] == entityID) {
+                    _occupiedScriptURLs.remove(entityScript);
+                }
             });
     }, forceRedownload);
 }
         newDetails.status = status;
         setEntityScriptDetails(entityID, newDetails);
-        QMutableListIterator<DeferredLoadEntity> i(_pendingLoadEntities);
+        qCDebug(scriptengine) << "entityScriptContentAvailable -- flagging " << oldDetails.scriptText << " as BAD_SCRIPT_UUID_PLACEHOLDER";
+        // flag this scriptURL as unusuable
         // note: we want the *original* entityScript string (not the potentially normalized one from ScriptCache results)
-        const auto entityScript = oldDetails.scriptText;
+        _occupiedScriptURLs[oldDetails.scriptText] = BAD_SCRIPT_UUID_PLACEHOLDER;
-        while (i.hasNext()) {
-            auto retry = i.next();
-            if (retry.entityScript == entityScript) {
-                qCDebug(scriptengine) << "... pending load of " << retry.entityID << " cancelled";
-                i.remove();
-            }
-        }
     };
     // NETWORK / FILESYSTEM ERRORS
     }
     _entityScripts.clear();
     emit entityScriptDetailsUpdated();
-    _lockPerUniqueScriptURL.clear();
+    _occupiedScriptURLs.clear();
 #ifdef DEBUG_ENGINE_STATE
     _debugDump(