From f5390ea331fb17e9f34e4335eb2101af50b6dba1 Mon Sep 17 00:00:00 2001 From: Ansariel Date: Wed, 11 Oct 2023 14:57:08 +0200 Subject: [PATCH] Revert "Revert "Revert "Revert changes for FIRE-32453 to try out the fix from LL"" once again to try out new fix attempt from LL" This reverts commit ee22125359b4587b0ed3acf5b0677c40531b5307. --- indra/llcommon/llthreadsafequeue.h | 8 ++- indra/llcommon/threadpool.h | 8 ++- indra/llwindow/llwindowwin32.cpp | 80 ++++++++++++++++++++++++++++-- 3 files changed, 88 insertions(+), 8 deletions(-) diff --git a/indra/llcommon/llthreadsafequeue.h b/indra/llcommon/llthreadsafequeue.h index 2a702c8d04..082337cf3e 100644 --- a/indra/llcommon/llthreadsafequeue.h +++ b/indra/llcommon/llthreadsafequeue.h @@ -342,7 +342,13 @@ bool LLThreadSafeQueue::pushIfOpen(T&& element) return true; // Storage Full. Wait for signal. - mCapacityCond.wait(lock1); + // [FIRE-32453][BUG-232971] Improve shutdown behaviour. Time bound the sleep + // mCapacityCond.wait(lock1); + // When the queue is full and the consuming thread has exited we would never wake up. + // For safety, we now wait max half a second then recheck close. + const auto timeout = std::chrono::milliseconds(500); + mCapacityCond.wait_for(lock1, timeout); + // } } diff --git a/indra/llcommon/threadpool.h b/indra/llcommon/threadpool.h index f8eec3b457..e995d9331e 100644 --- a/indra/llcommon/threadpool.h +++ b/indra/llcommon/threadpool.h @@ -46,7 +46,10 @@ namespace LL * ThreadPool listens for application shutdown messages on the "LLApp" * LLEventPump. Call close() to shut down this ThreadPool early. */ - void close(); + // [FIRE-32453][BUG-232971] Improve shutdown behaviour. + // void close(); + virtual void close(); + // std::string getName() const { return mName; } size_t getWidth() const { return mThreads.size(); } @@ -61,7 +64,8 @@ namespace LL private: void run(const std::string& name); - + + protected: // [FIRE-32453][BUG-232971] Improve shutdown behaviour. WorkQueue mQueue; std::string mName; size_t mThreadCount; diff --git a/indra/llwindow/llwindowwin32.cpp b/indra/llwindow/llwindowwin32.cpp index 8f72e84b28..47c172e32e 100644 --- a/indra/llwindow/llwindowwin32.cpp +++ b/indra/llwindow/llwindowwin32.cpp @@ -37,6 +37,7 @@ #include "llwindowcallbacks.h" // Linden library includes +#include "llapp.h" // [FIRE-32453][BUG-232971] Improve shutdown behaviour. #include "llerror.h" #include "llexception.h" #include "llfasttimer.h" @@ -88,6 +89,7 @@ const UINT WM_DUMMY_(WM_USER + 0x0017); const UINT WM_POST_FUNCTION_(WM_USER + 0x0018); extern BOOL gDebugWindowProc; +extern BOOL gDisconnected; // [FIRE-32453][BUG-232971] Improve shutdown behaviour. static std::thread::id sWindowThreadId; static std::thread::id sMainThreadId; @@ -346,6 +348,7 @@ struct LLWindowWin32::LLWindowWin32Thread : public LL::ThreadPool LLWindowWin32Thread(); void run() override; + void close() override; // [FIRE-32453][BUG-232971] Improve shutdown behaviour. /// called by main thread to post work to this window thread template @@ -898,9 +901,12 @@ void LLWindowWin32::close() // This causes WM_DESTROY to be sent *immediately* if (!destroy_window_handler(mWindowHandle)) { - OSMessageBox(mCallbacks->translateString("MBDestroyWinFailed"), - mCallbacks->translateString("MBShutdownErr"), - OSMB_OK); + // Can't use a message box here because we're about to stop servicing the events. + // OSMessageBox(mCallbacks->translateString("MBDestroyWinFailed"), + // mCallbacks->translateString("MBShutdownErr"), + // OSMB_OK); + LL_INFOS("Window") << "Destroying Window failed" << LL_ENDL; + // } } else @@ -919,6 +925,10 @@ void LLWindowWin32::close() // operations we're asking. That's the last time WE should touch it. mhDC = NULL; mWindowHandle = NULL; + // [FIRE-32453][BUG-232971] close the related queues first to prevent spinning. + mFunctionQueue.close(); + mMouseQueue.close(); + // mWindowThread->close(); } @@ -3736,7 +3746,6 @@ void LLWindowWin32::swapBuffers() LL_PROFILER_GPU_COLLECT } - // // LLSplashScreenImp // @@ -4768,10 +4777,45 @@ private: std::string mPrev; }; +// [FIRE-32453][BUG-232971] Improve shutdown behaviour. +// Provide a close() override that ignores the initial close triggered by the threadpool detecting "quit" +// but waits for the viewer to be disconected and the second close call triggered by the app window destructor +void LLWindowWin32::LLWindowWin32Thread::close() +{ + assert_main_thread(); + if (! mQueue.isClosed() && gDisconnected) + { + LL_DEBUGS("ThreadPool") << mName << " closing queue and joining threads" << LL_ENDL; + mQueue.close(); + for (auto& pair: mThreads) + { + LL_DEBUGS("ThreadPool") << mName << " waiting on thread " << pair.first << LL_ENDL; + // As we cannot seem to rely on the clean and timely exit of the windows thread in ALL situations we apply a timeout. + std::future f = std::async(std::launch::async, [&] { pair.second.join(); }); + if (f.wait_until(std::chrono::steady_clock::now() + std::chrono::seconds(5)) == std::future_status::ready) { + LL_DEBUGS("ThreadPool") << mName << " joined normally." << LL_ENDL; + } else { + LL_WARNS("ThreadPool") << mName << " join timed out." << LL_ENDL; + // the specified time point was reached before the thread finished execution and could be joined + } + } + LL_DEBUGS("ThreadPool") << mName << " shutdown complete" << LL_ENDL; + } + else + { + LL_DEBUGS("ThreadPool") << mName << " shutdown request ignored - not yet disconneced." << LL_ENDL; + } +} +// + void LLWindowWin32::LLWindowWin32Thread::run() { sWindowThreadId = std::this_thread::get_id(); LogChange logger("Window"); + // [FIRE-32453][BUG-232971] Improve shutdown behaviour. + try + { + // while (! getQueue().done()) { LL_PROFILE_ZONE_SCOPED_CATEGORY_WIN32; @@ -4798,7 +4842,21 @@ void LLWindowWin32::LLWindowWin32Thread::run() ", ", msg.wParam, ")"); TranslateMessage(&msg); DispatchMessage(&msg); - mMessageQueue.pushFront(msg); + // [FIRE-32453][BUG-232971] Improve shutdown behaviour. + // mMessageQueue.pushFront(msg); + try + { + // Nobody is reading this queue once we are quitting. Writing to it causes a hang. + if(!LLApp::isQuitting()) + mMessageQueue.pushFront(msg); + } + catch (const LLThreadSafeQueueInterrupt&) + { + // Shutdown timing is tricky. The main thread can end up trying + // to post a cursor position after having closed the WorkQueue. + logger.always("Message procesing tried to push() to closed MessageQueue - caught"); + } + // } } @@ -4817,6 +4875,18 @@ void LLWindowWin32::LLWindowWin32Thread::run() } #endif } + // [FIRE-32453][BUG-232971] Improve shutdown behaviour. + } + catch (const std::exception& e) + { + logger.always("Windows thread exiting - Exception: ", e.what()); + } + catch (...) + { + logger.always("Windows thread exiting - Exception: Unknown"); + } + logger.always("done - queue closed on windows thread."); + // } void LLWindowWin32::post(const std::function& func)