From dcebbada28ad436646a17b2eecc0b9241cda6cef Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 19 Jun 2018 10:13:48 -0700 Subject: [PATCH 1/6] fix various crashes-upon-exits --- interface/src/Application.cpp | 22 +++++++++++-------- interface/src/Application_render.cpp | 5 ++++- interface/src/ui/ApplicationOverlay.cpp | 2 ++ libraries/networking/src/ResourceManager.cpp | 9 ++++++-- .../plugins/src/plugins/PluginManager.cpp | 5 ++--- libraries/plugins/src/plugins/PluginManager.h | 10 +++++++-- 6 files changed, 36 insertions(+), 17 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 2033e8ee8e..54ecaf1201 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -814,6 +814,7 @@ bool setupEssentials(int& argc, char** argv, bool runningMarkerExisted) { } // Tell the plugin manager about our statically linked plugins + DependencyManager::set(); auto pluginManager = PluginManager::getInstance(); pluginManager->setInputPluginProvider([] { return getInputPlugins(); }); pluginManager->setDisplayPluginProvider([] { return getDisplayPlugins(); }); @@ -1375,6 +1376,10 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo initializeRenderEngine(); qCDebug(interfaceapp, "Initialized Render Engine."); + // Overlays need to exist before we set the ContextOverlayInterface dependency + _overlays.init(); // do this before scripts load + DependencyManager::set(); + // Initialize the user interface and menu system // Needs to happen AFTER the render engine initialization to access its configuration initializeUi(); @@ -1511,10 +1516,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo // allow you to move an entity around in your hand _entityEditSender.setPacketsPerSecond(3000); // super high!! - // Overlays need to exist before we set the ContextOverlayInterface dependency - _overlays.init(); // do this before scripts load - DependencyManager::set(); - // Make sure we don't time out during slow operations at startup updateHeartbeat(); @@ -2552,12 +2553,18 @@ Application::~Application() { _octreeProcessor.terminate(); _entityEditSender.terminate(); + if (auto steamClient = PluginManager::getInstance()->getSteamClientPlugin()) { + steamClient->shutdown(); + } + DependencyManager::destroy(); + + DependencyManager::destroy(); // must be destroyed before the FramebufferCache + DependencyManager::destroy(); DependencyManager::destroy(); DependencyManager::destroy(); DependencyManager::destroy(); DependencyManager::destroy(); - DependencyManager::destroy(); DependencyManager::destroy(); DependencyManager::destroy(); DependencyManager::destroy(); @@ -2567,10 +2574,6 @@ Application::~Application() { // remove the NodeList from the DependencyManager DependencyManager::destroy(); - if (auto steamClient = PluginManager::getInstance()->getSteamClientPlugin()) { - steamClient->shutdown(); - } - #if 0 ConnexionClient::getInstance().destroy(); #endif @@ -2890,6 +2893,7 @@ void Application::initializeUi() { auto compositorHelper = DependencyManager::get(); connect(compositorHelper.data(), &CompositorHelper::allowMouseCaptureChanged, this, [=] { if (isHMDMode()) { + auto compositorHelper = DependencyManager::get(); // don't capture outer smartpointer showCursor(compositorHelper->getAllowMouseCapture() ? Cursor::Manager::lookupIcon(_preferredCursor.get()) : Cursor::Icon::SYSTEM); diff --git a/interface/src/Application_render.cpp b/interface/src/Application_render.cpp index 2daa49dcf7..6b4840e3e5 100644 --- a/interface/src/Application_render.cpp +++ b/interface/src/Application_render.cpp @@ -139,7 +139,10 @@ void Application::paintGL() { frame->frameIndex = _renderFrameCount; frame->framebuffer = finalFramebuffer; frame->framebufferRecycler = [](const gpu::FramebufferPointer& framebuffer) { - DependencyManager::get()->releaseFramebuffer(framebuffer); + auto frameBufferCache = DependencyManager::get(); + if (frameBufferCache) { + frameBufferCache->releaseFramebuffer(framebuffer); + } }; // deliver final scene rendering commands to the display plugin { diff --git a/interface/src/ui/ApplicationOverlay.cpp b/interface/src/ui/ApplicationOverlay.cpp index ea660fb0e2..7db6a49d1c 100644 --- a/interface/src/ui/ApplicationOverlay.cpp +++ b/interface/src/ui/ApplicationOverlay.cpp @@ -50,6 +50,8 @@ ApplicationOverlay::~ApplicationOverlay() { geometryCache->releaseID(_magnifierBorder); geometryCache->releaseID(_qmlGeometryId); } + + DependencyManager::destroy(); } // Renders the overlays either to a texture or to the screen diff --git a/libraries/networking/src/ResourceManager.cpp b/libraries/networking/src/ResourceManager.cpp index 6df15129a2..46cf43b97d 100644 --- a/libraries/networking/src/ResourceManager.cpp +++ b/libraries/networking/src/ResourceManager.cpp @@ -39,8 +39,13 @@ ResourceManager::ResourceManager(bool atpSupportEnabled) : _atpSupportEnabled(at } ResourceManager::~ResourceManager() { - _thread.terminate(); - _thread.wait(); + if (_thread.isRunning()) { + _thread.quit(); + static const auto MAX_RESOURCE_MANAGER_THREAD_QUITTING_TIME = 0.5 * MSECS_PER_SECOND; + if (!_thread.wait(MAX_RESOURCE_MANAGER_THREAD_QUITTING_TIME)) { + _thread.terminate(); + } + } } void ResourceManager::setUrlPrefixOverride(const QString& prefix, const QString& replacement) { diff --git a/libraries/plugins/src/plugins/PluginManager.cpp b/libraries/plugins/src/plugins/PluginManager.cpp index e9c084e132..40b6ed820a 100644 --- a/libraries/plugins/src/plugins/PluginManager.cpp +++ b/libraries/plugins/src/plugins/PluginManager.cpp @@ -40,9 +40,8 @@ void PluginManager::setInputPluginSettingsPersister(const InputPluginSettingsPer _inputSettingsPersister = persister; } -PluginManager* PluginManager::getInstance() { - static PluginManager _manager; - return &_manager; +PluginManagerPointer PluginManager::getInstance() { + return DependencyManager::get(); } QString getPluginNameFromMetaData(QJsonObject object) { diff --git a/libraries/plugins/src/plugins/PluginManager.h b/libraries/plugins/src/plugins/PluginManager.h index f16ad7d09f..226ccc8060 100644 --- a/libraries/plugins/src/plugins/PluginManager.h +++ b/libraries/plugins/src/plugins/PluginManager.h @@ -9,11 +9,17 @@ #include +#include + #include "Forward.h" -class PluginManager : public QObject { + +class PluginManager; +using PluginManagerPointer = QSharedPointer; + +class PluginManager : public QObject, public Dependency { public: - static PluginManager* getInstance(); + static PluginManagerPointer getInstance(); PluginManager(); const DisplayPluginList& getDisplayPlugins(); From 18219464f191b79d8a6d0254c8c41af000975ccf Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Wed, 27 Jun 2018 09:39:19 -0700 Subject: [PATCH 2/6] Fix shutdown crash on Mac in Application::onPresent --- interface/src/Application.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 2033e8ee8e..c2600f9fca 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -269,9 +269,6 @@ public: } _renderContext->doneCurrent(); - // Deleting the object with automatically shutdown the thread - connect(qApp, &QCoreApplication::aboutToQuit, this, &QObject::deleteLater); - // Transfer to a new thread moveToNewNamedThread(this, "RenderThread", [this](QThread* renderThread) { hifi::qt::addBlockingForbiddenThread("Render", renderThread); @@ -2590,6 +2587,8 @@ Application::~Application() { // Can't log to file passed this point, FileLogger about to be deleted qInstallMessageHandler(LogHandler::verboseMessageHandler); + + _renderEventHandler->deleteLater(); } void Application::initializeGL() { From 4afb19e09106f4605a14e7634bca0a071a9cacb2 Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Wed, 27 Jun 2018 09:49:51 -0700 Subject: [PATCH 3/6] Remove assert in ~Frame causing shutdown crash on Mac in dev build --- libraries/gpu/src/gpu/Frame.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libraries/gpu/src/gpu/Frame.cpp b/libraries/gpu/src/gpu/Frame.cpp index d08a8ab56d..f1001d97d2 100644 --- a/libraries/gpu/src/gpu/Frame.cpp +++ b/libraries/gpu/src/gpu/Frame.cpp @@ -21,10 +21,7 @@ Frame::~Frame() { framebuffer.reset(); } - assert(bufferUpdates.empty()); - if (!bufferUpdates.empty()) { - qFatal("Buffer sync error... frame destroyed without buffer updates being applied"); - } + bufferUpdates.clear(); } void Frame::finish() { From 9dc03f50133a104b3f7c553737e54f1098dca372 Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Fri, 29 Jun 2018 10:39:16 -0700 Subject: [PATCH 4/6] Destroy GeometryCache explicitly in Application destructor --- interface/src/Application.cpp | 1 + interface/src/ui/ApplicationOverlay.cpp | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 4b32783607..63fff2af5b 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2565,6 +2565,7 @@ Application::~Application() { DependencyManager::destroy(); DependencyManager::destroy(); DependencyManager::destroy(); + DependencyManager::destroy(); DependencyManager::get()->cleanup(); diff --git a/interface/src/ui/ApplicationOverlay.cpp b/interface/src/ui/ApplicationOverlay.cpp index 7db6a49d1c..ea660fb0e2 100644 --- a/interface/src/ui/ApplicationOverlay.cpp +++ b/interface/src/ui/ApplicationOverlay.cpp @@ -50,8 +50,6 @@ ApplicationOverlay::~ApplicationOverlay() { geometryCache->releaseID(_magnifierBorder); geometryCache->releaseID(_qmlGeometryId); } - - DependencyManager::destroy(); } // Renders the overlays either to a texture or to the screen From e4377a6c95b6cd99fb56116f0768d23e7c901b3a Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Fri, 29 Jun 2018 10:55:01 -0700 Subject: [PATCH 5/6] Prevent type conversion in ~ResourceManager constant --- libraries/networking/src/ResourceManager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/ResourceManager.cpp b/libraries/networking/src/ResourceManager.cpp index 46cf43b97d..553f0d0a61 100644 --- a/libraries/networking/src/ResourceManager.cpp +++ b/libraries/networking/src/ResourceManager.cpp @@ -41,7 +41,7 @@ ResourceManager::ResourceManager(bool atpSupportEnabled) : _atpSupportEnabled(at ResourceManager::~ResourceManager() { if (_thread.isRunning()) { _thread.quit(); - static const auto MAX_RESOURCE_MANAGER_THREAD_QUITTING_TIME = 0.5 * MSECS_PER_SECOND; + static const auto MAX_RESOURCE_MANAGER_THREAD_QUITTING_TIME = MSECS_PER_SECOND / 2; if (!_thread.wait(MAX_RESOURCE_MANAGER_THREAD_QUITTING_TIME)) { _thread.terminate(); } From ba53e7add8feb39ab4922938eed0ff3f71972527 Mon Sep 17 00:00:00 2001 From: Clement Date: Fri, 29 Jun 2018 15:51:40 -0700 Subject: [PATCH 6/6] Fix ACs startup crash --- assignment-client/src/Agent.cpp | 3 +++ assignment-client/src/audio/AudioMixer.cpp | 7 ++++++- assignment-client/src/audio/AudioMixer.h | 3 +++ assignment-client/src/scripts/EntityScriptServer.cpp | 3 +++ libraries/plugins/src/plugins/PluginManager.cpp | 3 --- libraries/plugins/src/plugins/PluginManager.h | 5 ++++- 6 files changed, 19 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index 42924a8487..73444d1198 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -64,6 +64,7 @@ Agent::Agent(ReceivedMessage& message) : DependencyManager::get()->setPacketSender(&_entityEditSender); DependencyManager::set(); + DependencyManager::set(); DependencyManager::registerInheritance(); @@ -833,6 +834,8 @@ void Agent::aboutToFinish() { DependencyManager::get()->cleanup(); + DependencyManager::destroy(); + // cleanup the AudioInjectorManager (and any still running injectors) DependencyManager::destroy(); diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index 34eb138697..d56b22466e 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -65,7 +65,8 @@ AudioMixer::AudioMixer(ReceivedMessage& message) : // hash the available codecs (on the mixer) _availableCodecs.clear(); // Make sure struct is clean - auto codecPlugins = PluginManager::getInstance()->getCodecPlugins(); + auto pluginManager = DependencyManager::set(); + auto codecPlugins = pluginManager->getCodecPlugins(); std::for_each(codecPlugins.cbegin(), codecPlugins.cend(), [&](const CodecPluginPointer& codec) { _availableCodecs[codec->getName()] = codec; @@ -106,6 +107,10 @@ AudioMixer::AudioMixer(ReceivedMessage& message) : connect(nodeList.data(), &NodeList::nodeKilled, this, &AudioMixer::handleNodeKilled); } +void AudioMixer::aboutToFinish() { + DependencyManager::destroy(); +} + void AudioMixer::queueAudioPacket(QSharedPointer message, SharedNodePointer node) { if (message->getType() == PacketType::SilentAudioFrame) { _numSilentPackets++; diff --git a/assignment-client/src/audio/AudioMixer.h b/assignment-client/src/audio/AudioMixer.h index 8c47893aa3..f9eb18da6d 100644 --- a/assignment-client/src/audio/AudioMixer.h +++ b/assignment-client/src/audio/AudioMixer.h @@ -58,6 +58,9 @@ public: to.getPublicSocket() != from.getPublicSocket() && to.getLocalSocket() != from.getLocalSocket(); } + + virtual void aboutToFinish() override; + public slots: void run() override; void sendStatsPacket() override; diff --git a/assignment-client/src/scripts/EntityScriptServer.cpp b/assignment-client/src/scripts/EntityScriptServer.cpp index eea8e8b470..607ab28b20 100644 --- a/assignment-client/src/scripts/EntityScriptServer.cpp +++ b/assignment-client/src/scripts/EntityScriptServer.cpp @@ -58,6 +58,7 @@ EntityScriptServer::EntityScriptServer(ReceivedMessage& message) : ThreadedAssig DependencyManager::get()->setPacketSender(&_entityEditSender); DependencyManager::set(); + DependencyManager::set(); DependencyManager::registerInheritance(); @@ -572,6 +573,8 @@ void EntityScriptServer::aboutToFinish() { DependencyManager::get()->cleanup(); + DependencyManager::destroy(); + // cleanup the AudioInjectorManager (and any still running injectors) DependencyManager::destroy(); DependencyManager::destroy(); diff --git a/libraries/plugins/src/plugins/PluginManager.cpp b/libraries/plugins/src/plugins/PluginManager.cpp index 40b6ed820a..94ce16cf00 100644 --- a/libraries/plugins/src/plugins/PluginManager.cpp +++ b/libraries/plugins/src/plugins/PluginManager.cpp @@ -135,9 +135,6 @@ const LoaderList& getLoadedPlugins() { return loadedPlugins; } -PluginManager::PluginManager() { -} - const CodecPluginList& PluginManager::getCodecPlugins() { static CodecPluginList codecPlugins; static std::once_flag once; diff --git a/libraries/plugins/src/plugins/PluginManager.h b/libraries/plugins/src/plugins/PluginManager.h index 226ccc8060..65a4012aed 100644 --- a/libraries/plugins/src/plugins/PluginManager.h +++ b/libraries/plugins/src/plugins/PluginManager.h @@ -18,9 +18,10 @@ class PluginManager; using PluginManagerPointer = QSharedPointer; class PluginManager : public QObject, public Dependency { + SINGLETON_DEPENDENCY + public: static PluginManagerPointer getInstance(); - PluginManager(); const DisplayPluginList& getDisplayPlugins(); const InputPluginList& getInputPlugins(); @@ -45,6 +46,8 @@ public: void setInputPluginSettingsPersister(const InputPluginSettingsPersister& persister); private: + PluginManager() = default; + DisplayPluginProvider _displayPluginProvider { []()->DisplayPluginList { return {}; } }; InputPluginProvider _inputPluginProvider { []()->InputPluginList { return {}; } }; CodecPluginProvider _codecPluginProvider { []()->CodecPluginList { return {}; } };