Merge pull request #15864 from hyperlogic/bug-fix/dependency-manager-shutdown-issues

Fix for DependencyManager crash on shutdown on Mac

interface/src/Application.cpp

@@ -104,6 +104,7 @@
 #include <hfm/ModelFormatRegistry.h>
 #include <model-networking/ModelCacheScriptingInterface.h>
 #include <material-networking/TextureCacheScriptingInterface.h>
+#include <material-networking/MaterialCache.h>
 #include <ModelEntityItem.h>
 #include <NetworkAccessManager.h>
 #include <NetworkingConstants.h>
@@ -877,6 +878,7 @@ bool setupEssentials(int& argc, char** argv, bool runningMarkerExisted) {
     DependencyManager::set<AudioScope>();
     DependencyManager::set<DeferredLightingEffect>();
     DependencyManager::set<TextureCache>();
+    DependencyManager::set<MaterialCache>();
     DependencyManager::set<TextureCacheScriptingInterface>();
     DependencyManager::set<FramebufferCache>();
     DependencyManager::set<AnimationCache>();
@@ -2852,6 +2854,7 @@ Application::~Application() {
     DependencyManager::destroy<AnimationCacheScriptingInterface>();
     DependencyManager::destroy<AnimationCache>();
     DependencyManager::destroy<FramebufferCache>();
+    DependencyManager::destroy<MaterialCache>();
     DependencyManager::destroy<TextureCacheScriptingInterface>();
     DependencyManager::destroy<TextureCache>();
     DependencyManager::destroy<ModelCacheScriptingInterface>();
@@ -2887,6 +2890,16 @@ Application::~Application() {
 
     // Can't log to file past this point, FileLogger about to be deleted
     qInstallMessageHandler(LogHandler::verboseMessageHandler);
+
+#ifdef Q_OS_MAC
+    // Clear the event queue before application is totally destructed.
+    // This will drain the messasge queue of pending "deleteLaters" queued up
+    // during shutdown of the script engines.
+    // We do this here because there is a possiblty that [NSApplication terminate:]
+    // will be called during processEvents which will invoke all static destructors.
+    // We want to postpone this utill the last possible moment.
+    QCoreApplication::processEvents();
+#endif
 }
 
 void Application::initializeGL() {
@@ -5960,7 +5973,7 @@ void Application::reloadResourceCaches() {
     DependencyManager::get<ResourceCacheSharedItems>()->clear();
     DependencyManager::get<AnimationCache>()->refreshAll();
     DependencyManager::get<SoundCache>()->refreshAll();
-    MaterialCache::instance().refreshAll();
+    DependencyManager::get<MaterialCache>()->refreshAll();
     DependencyManager::get<ModelCache>()->refreshAll();
     ShaderCache::instance().refreshAll();
     DependencyManager::get<TextureCache>()->refreshAll();
@@ -7146,7 +7159,7 @@ void Application::clearDomainOctreeDetails(bool clearAll) {
 
     DependencyManager::get<AnimationCache>()->clearUnusedResources();
     DependencyManager::get<SoundCache>()->clearUnusedResources();
-    MaterialCache::instance().clearUnusedResources();
+    DependencyManager::get<MaterialCache>()->clearUnusedResources();
     DependencyManager::get<ModelCache>()->clearUnusedResources();
     ShaderCache::instance().clearUnusedResources();
     DependencyManager::get<TextureCache>()->clearUnusedResources();

libraries/baking/src/MaterialBaker.cpp

@@ -72,7 +72,7 @@ void MaterialBaker::loadMaterial() {
         _materialResource->parsedMaterials = NetworkMaterialResource::parseJSONMaterials(QJsonDocument::fromJson(_materialData.toUtf8()), QUrl());
     } else {
         qCDebug(material_baking) << "Downloading material" << _materialData;
-        _materialResource = MaterialCache::instance().getMaterial(_materialData);
+        _materialResource = DependencyManager::get<MaterialCache>()->getMaterial(_materialData);
     }
 
     if (_materialResource) {
@@ -280,4 +280,4 @@ void MaterialBaker::setMaterials(const QHash<QString, hfm::Material>& materials,
 
 void MaterialBaker::setMaterials(const NetworkMaterialResourcePointer& materialResource) {
     _materialResource = materialResource;
-}
+}

libraries/entities-renderer/src/RenderableMaterialEntityItem.cpp

@@ -172,7 +172,7 @@ void MaterialEntityRenderer::doRenderUpdateAsynchronousTyped(const TypedEntityPo
         }
 
         if (urlChanged && !usingMaterialData) {
-            _networkMaterial = MaterialCache::instance().getMaterial(_materialURL);
+            _networkMaterial = DependencyManager::get<MaterialCache>()->getMaterial(_materialURL);
             auto onMaterialRequestFinished = [this, oldParentID, oldParentMaterialName, newCurrentMaterialName](bool success) {
                 if (success) {
                     deleteMaterial(oldParentID, oldParentMaterialName);
@@ -412,4 +412,4 @@ void MaterialEntityRenderer::applyMaterial() {
 
     // if we've reached this point, we couldn't find our parent, so we need to try again later
     _retryApply = true;
-}
+}

libraries/material-networking/src/material-networking/MaterialCache.cpp

@@ -441,11 +441,6 @@ std::pair<std::string, std::shared_ptr<NetworkMaterial>> NetworkMaterialResource
     return std::pair<std::string, std::shared_ptr<NetworkMaterial>>(name, material);
 }
 
-MaterialCache& MaterialCache::instance() {
-    static MaterialCache _instance;
-    return _instance;
-}
-
 NetworkMaterialResourcePointer MaterialCache::getMaterial(const QUrl& url) {
     return ResourceCache::getResource(url).staticCast<NetworkMaterialResource>();
 }
@@ -761,4 +756,4 @@ void NetworkMaterial::checkResetOpacityMap() {
     if (albedoTexture.texture) {
         resetOpacityMap();
     }
-}
+}

libraries/material-networking/src/material-networking/MaterialCache.h

@@ -110,10 +110,11 @@ using NetworkMaterialResourcePointer = QSharedPointer<NetworkMaterialResource>;
 using MaterialMapping = std::vector<std::pair<std::string, NetworkMaterialResourcePointer>>;
 Q_DECLARE_METATYPE(MaterialMapping)
 
-class MaterialCache : public ResourceCache {
+class MaterialCache : public ResourceCache, public Dependency {
-public:
+    Q_OBJECT
-    static MaterialCache& instance();
+    SINGLETON_DEPENDENCY
 
+public:
     NetworkMaterialResourcePointer getMaterial(const QUrl& url);
 
 protected:

libraries/model-baker/src/model-baker/ParseMaterialMappingTask.cpp

@@ -61,7 +61,7 @@ void processMaterialMapping(MaterialMapping& materialMapping, const QJsonObject&
         } else if (mappingJSON.isString()) {
             auto mappingValue = mappingJSON.toString();
             materialMapping.push_back(std::pair<std::string, NetworkMaterialResourcePointer>(mapping.toStdString(),
-                                                                MaterialCache::instance().getMaterial(url.resolved(mappingValue))));
+                DependencyManager::get<MaterialCache>()->getMaterial(url.resolved(mappingValue))));
         }
     }
 }

libraries/networking/src/AccountManager.cpp

@@ -92,6 +92,7 @@ const QString DOUBLE_SLASH_SUBSTITUTE = "slashslash";
 const QString ACCOUNT_MANAGER_REQUESTED_SCOPE = "owner";
 
 void AccountManager::logout() {
+
     // a logout means we want to delete the DataServerAccountInfo we currently have for this URL, in-memory and in file
     _accountInfo = DataServerAccountInfo();
 
@@ -959,4 +960,4 @@ void AccountManager::saveLoginStatus(bool isLoggedIn) {
             QMetaObject::invokeMethod(qApp, "quit", Qt::QueuedConnection);
         }
     }
-}
+}

libraries/script-engine/src/ScriptEngine.cpp

@@ -224,7 +224,7 @@ ScriptEngine::ScriptEngine(Context context, const QString& scriptContents, const
     if (_type == Type::ENTITY_CLIENT || _type == Type::ENTITY_SERVER) {
         QObject::connect(this, &ScriptEngine::update, this, [this]() {
             // process pending entity script content
-            if (!_contentAvailableQueue.empty()) {
+            if (!_contentAvailableQueue.empty() && !(_isFinished || _isStopping)) {
                 EntityScriptContentAvailableMap pending;
                 std::swap(_contentAvailableQueue, pending);
                 for (auto& pair : pending) {
@@ -343,7 +343,7 @@ void ScriptEngine::runDebuggable() {
         // we check for 'now' in the past in case people set their clock back
         if (_lastUpdate < now) {
             float deltaTime = (float)(now - _lastUpdate) / (float)USECS_PER_SECOND;
-            if (!_isFinished) {
+            if (!(_isFinished || _isStopping)) {
                 emit update(deltaTime);
             }
         }
@@ -411,6 +411,27 @@ void ScriptEngine::waitTillDoneRunning() {
         // We should never be waiting (blocking) on our own thread
         assert(workerThread != QThread::currentThread());
 
+#ifdef Q_OS_MAC
+        // On mac, don't call QCoreApplication::processEvents() here. This is to prevent
+        // [NSApplication terminate:] from prematurely destroying the static destructors
+        // while we are waiting for the scripts to shutdown. We will pump the message
+        // queue later in the Application destructor.
+        if (workerThread->isRunning()) {
+            workerThread->quit();
+
+            if (isEvaluating()) {
+                qCWarning(scriptengine) << "Script Engine has been running too long, aborting:" << getFilename();
+                abortEvaluation();
+            }
+
+            // Wait for the scripting thread to stop running, as
+            // flooding it with aborts/exceptions will persist it longer
+            static const auto MAX_SCRIPT_QUITTING_TIME = 0.5 * MSECS_PER_SECOND;
+            if (!workerThread->wait(MAX_SCRIPT_QUITTING_TIME)) {
+                workerThread->terminate();
+            }
+        }
+#else
         auto startedWaiting = usecTimestampNow();
         while (workerThread->isRunning()) {
             // If the final evaluation takes too long, then tell the script engine to stop running
@@ -448,6 +469,7 @@ void ScriptEngine::waitTillDoneRunning() {
             // Avoid a pure busy wait
             QThread::yieldCurrentThread();
         }
+#endif
 
         scriptInfoMessage("Script Engine has stopped:" + getFilename());
     }