From a6395b9025763ccc7b962458c32da65f1c27833a Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Sun, 6 Mar 2022 13:11:36 -0600 Subject: [PATCH] Fix short UI hang when manually stopping the stream after losing connection with the host --- app/gui/StreamSegue.qml | 4 ++++ app/streaming/session.cpp | 14 ++++---------- app/streaming/session.h | 7 ++++++- app/streaming/video/overlaymanager.cpp | 13 +++++++++++-- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/app/gui/StreamSegue.qml b/app/gui/StreamSegue.qml index 0ec48373..5ddc1678 100644 --- a/app/gui/StreamSegue.qml +++ b/app/gui/StreamSegue.qml @@ -103,7 +103,10 @@ Item { streamSegueErrorDialog.open() } } + } + function sessionReadyForDeletion() + { // Garbage collect the Session object since it's pretty heavyweight // and keeps other libraries (like SDL_TTF) around until it is deleted. session = null @@ -130,6 +133,7 @@ Item { session.displayLaunchWarning.connect(displayLaunchWarning) session.quitStarting.connect(quitStarting) session.sessionFinished.connect(sessionFinished) + session.readyForDeletion.connect(sessionReadyForDeletion) // Kick off the stream spinnerTimer.start() diff --git a/app/streaming/session.cpp b/app/streaming/session.cpp index 6919baba..75287b51 100644 --- a/app/streaming/session.cpp +++ b/app/streaming/session.cpp @@ -464,16 +464,6 @@ Session::Session(NvComputer* computer, NvApp& app, StreamingPreferences *prefere SDL_AtomicSet(&m_NeedsIdr, 0); } -// NB: This may not get destroyed for a long time! Don't put any vital cleanup here. -// Use Session::exec() or DeferredSessionCleanupTask instead. -Session::~Session() -{ - // Acquire session semaphore to ensure all cleanup is done before the destructor returns - // and the object is deallocated. - s_ActiveSessionSemaphore.acquire(); - s_ActiveSessionSemaphore.release(); -} - bool Session::initialize() { if (SDL_InitSubSystem(SDL_INIT_VIDEO) != 0) { @@ -856,6 +846,9 @@ private: // Allow another session to start now that we're cleaned up Session::s_ActiveSession = nullptr; Session::s_ActiveSessionSemaphore.release(); + + // Notify that the session is ready to be cleaned up + emit m_Session->readyForDeletion(); } void run() override @@ -1316,6 +1309,7 @@ void Session::execInternal() // called on the main thread. if (!initialize()) { emit sessionFinished(0); + emit readyForDeletion(); return; } diff --git a/app/streaming/session.h b/app/streaming/session.h index 00bae632..bf5bbfc9 100644 --- a/app/streaming/session.h +++ b/app/streaming/session.h @@ -22,7 +22,9 @@ class Session : public QObject public: explicit Session(NvComputer* computer, NvApp& app, StreamingPreferences *preferences = nullptr); - virtual ~Session(); + // NB: This may not get destroyed for a long time! Don't put any cleanup here. + // Use Session::exec() or DeferredSessionCleanupTask instead. + virtual ~Session() {}; Q_INVOKABLE void exec(int displayOriginX, int displayOriginY); @@ -60,6 +62,9 @@ signals: void sessionFinished(int portTestResult); + // Emitted after sessionFinished() when the session is ready to be destroyed + void readyForDeletion(); + private: void execInternal(); diff --git a/app/streaming/video/overlaymanager.cpp b/app/streaming/video/overlaymanager.cpp index b037a478..69c53fb3 100644 --- a/app/streaming/video/overlaymanager.cpp +++ b/app/streaming/video/overlaymanager.cpp @@ -15,7 +15,11 @@ OverlayManager::OverlayManager() : m_Overlays[OverlayType::OverlayStatusUpdate].color = {0xCC, 0x00, 0x00, 0xFF}; m_Overlays[OverlayType::OverlayStatusUpdate].fontSize = 36; - SDL_assert(TTF_WasInit() == 0); + // While TTF will usually not be initialized here, it is valid for that not to + // be the case, since Session destruction is deferred and could overlap with + // the lifetime of a new Session object. + //SDL_assert(TTF_WasInit() == 0); + if (TTF_Init() != 0) { SDL_LogWarn(SDL_LOG_CATEGORY_APPLICATION, "TTF_Init() failed: %s", @@ -36,7 +40,12 @@ OverlayManager::~OverlayManager() } TTF_Quit(); - SDL_assert(TTF_WasInit() == 0); + + // For similar reasons to the comment in the constructor, this will usually, + // but not always, deinitialize TTF. In the cases where Session objects overlap + // in lifetime, there may be an additional reference on TTF for the new Session + // that means it will not be cleaned up here. + //SDL_assert(TTF_WasInit() == 0); } bool OverlayManager::isOverlayEnabled(OverlayType type)