diff --git a/indra/llcommon/llthreadsafequeue.h b/indra/llcommon/llthreadsafequeue.h index 16b3966ad2..6a629b5405 100644 --- a/indra/llcommon/llthreadsafequeue.h +++ b/indra/llcommon/llthreadsafequeue.h @@ -308,7 +308,13 @@ bool LLThreadSafeQueue::push_(lock_t& lock, T&& element) { LL_PROFILE_ZONE_SCOPED_CATEGORY_THREAD; if (mStorage.size() >= mCapacity) + // Make thread queue capacity hangs visible + // return false; + { + LL_WARNS("ThreadPool") << "Threadsafe queue push_(lockacquired) queue full " << mStorage.size() << " >= " << mCapacity << LL_ENDL; return false; + } + // mStorage.push(std::forward(element)); lock.unlock(); 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 f002219b3e..4fab03ebee 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 @@ -4735,6 +4738,31 @@ 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; + // if mName does not contain the word Window + pair.second.join(); + } + 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(); @@ -4766,20 +4794,30 @@ 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"); + } + // } } { LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("w32t - Function Queue"); - logger.onChange("runPending()"); + logger.onChange("runPending()"); //process any pending functions getQueue().runPending(); } - // This is very verbose even for debug so it has it's own debug tag - LL_DEBUGS("WindowVerbose") << "PreCheck Done - closed=" << getQueue().isClosed() << " size=" << getQueue().size() << LL_ENDL; - // #if 0 { @@ -4794,12 +4832,26 @@ void LLWindowWin32::LLWindowWin32Thread::run() void LLWindowWin32::post(const std::function& func) { - mFunctionQueue.pushFront(func); + try + { + mFunctionQueue.pushFront(func); + } + catch (const LLThreadSafeQueueInterrupt&) + { + LL_INFOS("Window") << "push function to closed Queue caught" << LL_ENDL; + } } void LLWindowWin32::postMouseButtonEvent(const std::function& func) { - mMouseQueue.pushFront(func); + try + { + mMouseQueue.pushFront(func); + } + catch (const LLThreadSafeQueueInterrupt&) + { + LL_INFOS("Window") << "push mouse event to closed Queue caught" << LL_ENDL; + } } void LLWindowWin32::kickWindowThread(HWND windowHandle) diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 7e4d01b2e8..28f0ec4936 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -1773,9 +1773,7 @@ bool LLAppViewer::doFrame() { LLVoiceClient::getInstance()->terminate(); } - // [FIRE-32453] [BUG-232971] disconnect sooner to force the cache write. - persistCachesAndSettings(); - // + disconnectViewer(); resumeMainloopTimeout(); } @@ -2063,6 +2061,7 @@ bool LLAppViewer::cleanup() LLViewerCamera::deleteSingleton(); LL_INFOS() << "Viewer disconnected" << LL_ENDL; + if (gKeyboard) { gKeyboard->resetKeys(); @@ -4864,9 +4863,6 @@ void LLAppViewer::removeDumpDir() void LLAppViewer::forceQuit() { - // [FIRE-32453] [BUG-232971] disconnect sooner to force the cache write. - persistCachesAndSettings(); - // LLApp::setQuitting(); } @@ -6337,12 +6333,7 @@ void LLAppViewer::idleNetwork() } } add(LLStatViewer::NUM_NEW_OBJECTS, gObjectList.mNumNewObjects); - // [FIRE-32453] [BUG-232971] disconnect sooner to force the cache write. - if(gDisconnected) - { - return; - } - // + // Retransmit unacknowledged packets. gXferManager->retransmitUnackedPackets(); gAssetStorage->checkForTimeouts(); @@ -6364,42 +6355,7 @@ void LLAppViewer::idleNetwork() mAgentRegionLastAlive = this_region_alive; } } -void LLAppViewer::persistCachesAndSettings() -{ - // Save inventory to disk if appropriate - if (gInventory.isInventoryUsable() - && gAgent.getID().notNull()) // Shouldn't be null at this stage - { - LL_INFOS() << "Saving Inventory Cache" << LL_ENDL; - gInventory.cache(gInventory.getRootFolderID(), gAgent.getID()); - if (gInventory.getLibraryRootFolderID().notNull() - && gInventory.getLibraryOwnerID().notNull()) - { - gInventory.cache( - gInventory.getLibraryRootFolderID(), - gInventory.getLibraryOwnerID()); - } - LL_INFOS() << "Saving Inventory Cache : COMPLETED" << LL_ENDL; - } - else - { - LL_INFOS() << "Not Saving Inventory Cache : Inventory is currently unusable" << LL_ENDL; - } - // Persist name cache - LLAvatarNameCache::instance().setCustomNameCheckCallback(LLAvatarNameCache::custom_name_check_callback_t()); // Contact sets - LL_INFOS() << "Saving Name Cache" << LL_ENDL; - saveNameCache(); - LL_INFOS() << "Saving Name Cache : COMPLETED" << LL_ENDL; - // Save experience cache if appropriate - if (LLExperienceCache::instanceExists()) - { - LL_INFOS() << "Saving Experience Cache" << LL_ENDL; - LLExperienceCache::instance().cleanup(); - LL_INFOS() << "Saving Experience Cache : COMPLETED" << LL_ENDL; - } - -} void LLAppViewer::disconnectViewer() { if (gDisconnected) @@ -6435,32 +6391,30 @@ void LLAppViewer::disconnectViewer() { LLSelectMgr::getInstance()->deselectAll(); } - // [FIRE-32453] [BUG-232971] Persist before disconnect - // Moved to separate function - // // save inventory if appropriate - // if (gInventory.isInventoryUsable() - // && gAgent.getID().notNull()) // Shouldn't be null at this stage - // { - // gInventory.cache(gInventory.getRootFolderID(), gAgent.getID()); - // if (gInventory.getLibraryRootFolderID().notNull() - // && gInventory.getLibraryOwnerID().notNull()) - // { - // gInventory.cache( - // gInventory.getLibraryRootFolderID(), - // gInventory.getLibraryOwnerID()); - // } - // } - // LLAvatarNameCache::instance().setCustomNameCheckCallback(LLAvatarNameCache::custom_name_check_callback_t()); // Contact sets - // saveNameCache(); - // if (LLExperienceCache::instanceExists()) - // { - // // TODO: LLExperienceCache::cleanup() logic should be moved to - // // cleanupSingleton(). - // LLExperienceCache::instance().cleanup(); - // } + // save inventory if appropriate + if (gInventory.isInventoryUsable() + && gAgent.getID().notNull()) // Shouldn't be null at this stage + { + gInventory.cache(gInventory.getRootFolderID(), gAgent.getID()); + if (gInventory.getLibraryRootFolderID().notNull() + && gInventory.getLibraryOwnerID().notNull()) + { + gInventory.cache( + gInventory.getLibraryRootFolderID(), + gInventory.getLibraryOwnerID()); + } + } + + LLAvatarNameCache::instance().setCustomNameCheckCallback(LLAvatarNameCache::custom_name_check_callback_t()); // Contact sets + saveNameCache(); + if (LLExperienceCache::instanceExists()) + { + // TODO: LLExperienceCache::cleanup() logic should be moved to + // cleanupSingleton(). + LLExperienceCache::instance().cleanup(); + } - // // close inventory interface, close all windows LLSidepanelInventory::cleanup(); @@ -6490,7 +6444,7 @@ void LLAppViewer::disconnectViewer() LLDestroyClassList::instance().fireCallbacks(); cleanup_xfer_manager(); - gDisconnected = TRUE; + gDisconnected = TRUE; // Pass the connection state to LLUrlEntryParcel not to attempt // parcel info requests while disconnected. diff --git a/indra/newview/llappviewer.h b/indra/newview/llappviewer.h index c548e3c5b8..323cd21c5a 100644 --- a/indra/newview/llappviewer.h +++ b/indra/newview/llappviewer.h @@ -287,7 +287,6 @@ private: void idleNetwork(); void sendLogoutRequest(); - void persistCachesAndSettings(); void disconnectViewer(); // *FIX: the app viewer class should be some sort of singleton, no?