Skip to content

Commit f4cf0f6

Browse files
committed
SL-18721 Shutdown fixes
1. After window closes viewer still takes some time to shut down, so added splash screen to not confuse users (and to see if something gets stuck) 2. Having two identical mWindowHandle caused confusion for me, so I split them. It looks like there might have been issues with thread being stuck because thread's handle wasn't cleaned up. 3. Made region clean mCacheMap immediately instead of spending time making copies on shutdown
1 parent 320204e commit f4cf0f6

File tree

5 files changed

+71
-40
lines changed

5 files changed

+71
-40
lines changed

indra/llcommon/threadpool.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,15 @@ struct sleepy_robin: public boost::fibers::algo::round_robin
6060
/*****************************************************************************
6161
* ThreadPoolBase
6262
*****************************************************************************/
63-
LL::ThreadPoolBase::ThreadPoolBase(const std::string& name, size_t threads,
64-
WorkQueueBase* queue):
63+
LL::ThreadPoolBase::ThreadPoolBase(const std::string& name,
64+
size_t threads,
65+
WorkQueueBase* queue,
66+
bool auto_shutdown):
6567
super(name),
6668
mName("ThreadPool:" + name),
6769
mThreadCount(getConfiguredWidth(name, threads)),
68-
mQueue(queue)
70+
mQueue(queue),
71+
mAutomaticShutdown(auto_shutdown)
6972
{}
7073

7174
void LL::ThreadPoolBase::start()
@@ -79,6 +82,14 @@ void LL::ThreadPoolBase::start()
7982
run(tname);
8083
});
8184
}
85+
86+
if (!mAutomaticShutdown)
87+
{
88+
// Some threads, like main window's might need to run a bit longer
89+
// to wait for a proper shutdown message
90+
return;
91+
}
92+
8293
// Listen on "LLApp", and when the app is shutting down, close the queue
8394
// and join the workers.
8495
LLEventPumps::instance().obtain("LLApp").listen(

indra/llcommon/threadpool.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ namespace LL
4040
* overrides this parameter.
4141
*/
4242
ThreadPoolBase(const std::string& name, size_t threads,
43-
WorkQueueBase* queue);
43+
WorkQueueBase* queue, bool auto_shutdown = true);
4444
virtual ~ThreadPoolBase();
4545

4646
/**
@@ -87,6 +87,7 @@ namespace LL
8787

8888
protected:
8989
std::unique_ptr<WorkQueueBase> mQueue;
90+
bool mAutomaticShutdown;
9091

9192
private:
9293
void run(const std::string& name);
@@ -117,8 +118,11 @@ namespace LL
117118
* Constraining the queue can cause a submitter to block. Do not
118119
* constrain any ThreadPool accepting work from the main thread.
119120
*/
120-
ThreadPoolUsing(const std::string& name, size_t threads=1, size_t capacity=1024*1024):
121-
ThreadPoolBase(name, threads, new queue_t(name, capacity))
121+
ThreadPoolUsing(const std::string& name,
122+
size_t threads=1,
123+
size_t capacity=1024*1024,
124+
bool auto_shutdown = true):
125+
ThreadPoolBase(name, threads, new queue_t(name, capacity), auto_shutdown)
122126
{}
123127
~ThreadPoolUsing() override {}
124128

indra/llwindow/llwindowwin32.cpp

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -412,8 +412,8 @@ struct LLWindowWin32::LLWindowWin32Thread : public LL::ThreadPool
412412
using FuncType = std::function<void()>;
413413
// call GetMessage() and pull enqueue messages for later processing
414414
void gatherInput();
415-
HWND mWindowHandle = NULL;
416-
HDC mhDC = 0;
415+
HWND mWindowHandleThrd = NULL;
416+
HDC mhDCThrd = 0;
417417

418418
// *HACK: Attempt to prevent startup crashes by deferring memory accounting
419419
// until after some graphics setup. See SL-20177. -Cosmic,2023-09-18
@@ -987,23 +987,23 @@ void LLWindowWin32::close()
987987

988988
LL_DEBUGS("Window") << "Destroying Window" << LL_ENDL;
989989

990-
mWindowThread->post([=]()
990+
mWindowThread->post([this, self = mWindowThread]()
991991
{
992-
if (IsWindow(mWindowHandle))
992+
if (IsWindow(self->mWindowHandleThrd))
993993
{
994-
if (mhDC)
994+
if (self->mhDCThrd)
995995
{
996-
if (!ReleaseDC(mWindowHandle, mhDC))
996+
if (!ReleaseDC(self->mWindowHandleThrd, self->mhDCThrd))
997997
{
998998
LL_WARNS("Window") << "Release of ghDC failed!" << LL_ENDL;
999999
}
10001000
}
10011001

10021002
// Make sure we don't leave a blank toolbar button.
1003-
ShowWindow(mWindowHandle, SW_HIDE);
1003+
ShowWindow(self->mWindowHandleThrd, SW_HIDE);
10041004

10051005
// This causes WM_DESTROY to be sent *immediately*
1006-
if (!destroy_window_handler(mWindowHandle))
1006+
if (!destroy_window_handler(self->mWindowHandleThrd))
10071007
{
10081008
OSMessageBox(mCallbacks->translateString("MBDestroyWinFailed"),
10091009
mCallbacks->translateString("MBShutdownErr"),
@@ -1015,17 +1015,18 @@ void LLWindowWin32::close()
10151015
// Something killed the window while we were busy destroying gl or handle somehow got broken
10161016
LL_WARNS("Window") << "Failed to destroy Window, invalid handle!" << LL_ENDL;
10171017
}
1018-
1018+
self->mWindowHandleThrd = NULL;
1019+
self->mhDCThrd = NULL;
1020+
self->mGLReady = false;
10191021
});
1020-
// Window thread might be waiting for a getMessage(), give it
1021-
// a push to enshure it will process destroy_window_handler
1022-
kickWindowThread();
10231022

1024-
// Even though the above lambda might not yet have run, we've already
1025-
// bound mWindowHandle into it by value, which should suffice for the
1026-
// operations we're asking. That's the last time WE should touch it.
10271023
mhDC = NULL;
10281024
mWindowHandle = NULL;
1025+
1026+
// Window thread might be waiting for a getMessage(), give it
1027+
// a push to enshure it will process destroy_window_handler
1028+
kickWindowThread();
1029+
10291030
mWindowThread->close();
10301031
}
10311032

@@ -1777,8 +1778,8 @@ void LLWindowWin32::recreateWindow(RECT window_rect, DWORD dw_ex_style, DWORD dw
17771778
()
17781779
{
17791780
LL_DEBUGS("Window") << "recreateWindow(): window_work entry" << LL_ENDL;
1780-
self->mWindowHandle = 0;
1781-
self->mhDC = 0;
1781+
self->mWindowHandleThrd = 0;
1782+
self->mhDCThrd = 0;
17821783

17831784
if (oldWindowHandle)
17841785
{
@@ -1813,20 +1814,20 @@ void LLWindowWin32::recreateWindow(RECT window_rect, DWORD dw_ex_style, DWORD dw
18131814
{
18141815
// Failed to create window: clear the variables. This
18151816
// assignment is valid because we're running on mWindowThread.
1816-
self->mWindowHandle = NULL;
1817-
self->mhDC = 0;
1817+
self->mWindowHandleThrd = NULL;
1818+
self->mhDCThrd = 0;
18181819
}
18191820
else
18201821
{
18211822
// Update mWindowThread's own mWindowHandle and mhDC.
1822-
self->mWindowHandle = handle;
1823-
self->mhDC = GetDC(handle);
1823+
self->mWindowHandleThrd = handle;
1824+
self->mhDCThrd = GetDC(handle);
18241825
}
18251826

18261827
updateWindowRect();
18271828

18281829
// It's important to wake up the future either way.
1829-
promise.set_value(std::make_pair(self->mWindowHandle, self->mhDC));
1830+
promise.set_value(std::make_pair(self->mWindowHandleThrd, self->mhDCThrd));
18301831
LL_DEBUGS("Window") << "recreateWindow(): window_work done" << LL_ENDL;
18311832
};
18321833
// But how we pass window_work to the window thread depends on whether we
@@ -4589,7 +4590,7 @@ U32 LLWindowWin32::getAvailableVRAMMegabytes()
45894590
#endif // LL_WINDOWS
45904591

45914592
inline LLWindowWin32::LLWindowWin32Thread::LLWindowWin32Thread()
4592-
: LL::ThreadPool("Window Thread", 1, MAX_QUEUE_SIZE)
4593+
: LL::ThreadPool("Window Thread", 1, MAX_QUEUE_SIZE, false)
45934594
{
45944595
LL::ThreadPool::start();
45954596
}
@@ -4745,7 +4746,7 @@ void LLWindowWin32::LLWindowWin32Thread::initD3D()
47454746
{
47464747
if (!mGLReady) { return; }
47474748

4748-
if (mDXGIAdapter == NULL && mD3DDevice == NULL && mWindowHandle != 0)
4749+
if (mDXGIAdapter == NULL && mD3DDevice == NULL && mWindowHandleThrd != 0)
47494750
{
47504751
mD3D = Direct3DCreate9(D3D_SDK_VERSION);
47514752

@@ -4755,7 +4756,7 @@ void LLWindowWin32::LLWindowWin32Thread::initD3D()
47554756
d3dpp.Windowed = TRUE;
47564757
d3dpp.SwapEffect = D3DSWAPEFFECT_DISCARD;
47574758

4758-
HRESULT res = mD3D->CreateDevice(D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, mWindowHandle, D3DCREATE_SOFTWARE_VERTEXPROCESSING, &d3dpp, &mD3DDevice);
4759+
HRESULT res = mD3D->CreateDevice(D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, mWindowHandleThrd, D3DCREATE_SOFTWARE_VERTEXPROCESSING, &d3dpp, &mD3DDevice);
47594760

47604761
if (FAILED(res))
47614762
{
@@ -4861,24 +4862,24 @@ void LLWindowWin32::LLWindowWin32Thread::run()
48614862
// lazily call initD3D inside this loop to catch when mGLReady has been set to true
48624863
initDX();
48634864

4864-
if (mWindowHandle != 0)
4865+
if (mWindowHandleThrd != 0)
48654866
{
48664867
// lazily call initD3D inside this loop to catch when mWindowHandle has been set, and mGLReady has been set to true
48674868
// *TODO: Shutdown if this fails when mWindowHandle exists
48684869
initD3D();
48694870

48704871
MSG msg;
48714872
BOOL status;
4872-
if (mhDC == 0)
4873+
if (mhDCThrd == 0)
48734874
{
48744875
LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("w32t - PeekMessage");
4875-
logger.onChange("PeekMessage(", std::hex, mWindowHandle, ")");
4876-
status = PeekMessage(&msg, mWindowHandle, 0, 0, PM_REMOVE);
4876+
logger.onChange("PeekMessage(", std::hex, mWindowHandleThrd, ")");
4877+
status = PeekMessage(&msg, mWindowHandleThrd, 0, 0, PM_REMOVE);
48774878
}
48784879
else
48794880
{
48804881
LL_PROFILE_ZONE_NAMED_CATEGORY_WIN32("w32t - GetMessage");
4881-
logger.always("GetMessage(", std::hex, mWindowHandle, ")");
4882+
logger.always("GetMessage(", std::hex, mWindowHandleThrd, ")");
48824883
status = GetMessage(&msg, NULL, 0, 0);
48834884
}
48844885
if (status > 0)

indra/newview/llappviewer.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1892,6 +1892,9 @@ bool LLAppViewer::cleanup()
18921892
LL_INFOS() << "ViewerWindow deleted" << LL_ENDL;
18931893
}
18941894

1895+
LLSplashScreen::show();
1896+
LLSplashScreen::update(LLTrans::getString("ShuttingDown"));
1897+
18951898
LL_INFOS() << "Cleaning up Keyboard & Joystick" << LL_ENDL;
18961899

18971900
// viewer UI relies on keyboard so keep it aound until viewer UI isa gone
@@ -2170,6 +2173,8 @@ bool LLAppViewer::cleanup()
21702173
// deleteSingleton() methods.
21712174
LLSingletonBase::deleteAll();
21722175

2176+
LLSplashScreen::hide();
2177+
21732178
LL_INFOS() << "Goodbye!" << LL_ENDL;
21742179

21752180
removeDumpDir();
@@ -5058,6 +5063,9 @@ void LLAppViewer::idleShutdown()
50585063
&& gLogoutTimer.getElapsedTimeF32() < SHUTDOWN_UPLOAD_SAVE_TIME
50595064
&& !logoutRequestSent())
50605065
{
5066+
gViewerWindow->setShowProgress(TRUE);
5067+
gViewerWindow->setProgressPercent(100.f);
5068+
gViewerWindow->setProgressString(LLTrans::getString("LoggingOut"));
50615069
return;
50625070
}
50635071

indra/newview/llviewerregion.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -829,10 +829,17 @@ void LLViewerRegion::saveObjectCache()
829829
mCacheDirty = FALSE;
830830
}
831831

832-
// Map of LLVOCacheEntry takes time to release, store map for cleanup on idle
833-
sRegionCacheCleanup.insert(mImpl->mCacheMap.begin(), mImpl->mCacheMap.end());
834-
mImpl->mCacheMap.clear();
835-
// TODO - probably need to do the same for overrides cache
832+
if (LLAppViewer::instance()->isQuitting())
833+
{
834+
mImpl->mCacheMap.clear();
835+
}
836+
else
837+
{
838+
// Map of LLVOCacheEntry takes time to release, store map for cleanup on idle
839+
sRegionCacheCleanup.insert(mImpl->mCacheMap.begin(), mImpl->mCacheMap.end());
840+
mImpl->mCacheMap.clear();
841+
// TODO - probably need to do the same for overrides cache
842+
}
836843
}
837844

838845
void LLViewerRegion::sendMessage()

0 commit comments

Comments
 (0)