Fix for DependencyManager crash on shutdown on Mac

On Mac, it is possible to crash when shutting down, it is not clear if this is due to
shutting down the app on another thread during logout or something that can happen
during normal shutdown, because it is so difficult to reproduce.

However, from looking at the stack traces it is possible for a [NSApplication terminate:]
event to get processed while Appliction::aboutToQuit() is calling ScriptEngine::waitTillDoneRunning()
This causes AppKit to invoke the static destructors too early.  Which in turn, causes the
DependencyManager destructor to fire while there are still many dependencies running.
Unfortunatly, the order of destruction is not determinstic, causing them to get shutdown
in an incorrect order.

To workaround this, we delay the call to QCoreApplication::processEvents() as late as possible,
in the Application destructor. Theoretically, this will be a safe time for the static destructors
to be invoked, because it is after all of the DependencyManager's dependencies have been
manually destroyed.

However, this is only a speculative fix, because this is so difficult to reproduce.

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 {
-public:
-    static MaterialCache& instance();
+class MaterialCache : public ResourceCache, public Dependency {
+    Q_OBJECT
+    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());
     }