SL-16094: Zap thread safety land mine; thin PostMessage() calls.

LLWindowWin32::mWndProc was a public WNDPROC member. If set non-NULL,
mainWindowProc() would call that before falling into its own handler code. But
now, mWndProc would be called on the window thread instead of on the main
thread. Running arbitrary callback code on the window thread could cause all
sorts of problems.

It could be made safe by posting the callback call to the "mainloop" WorkQueue
for execution on the main thread. But as no code actually references it,
delete it instead.

Per DaveP, the recent change to LLWindowsWin32Thread::post() could end up
calling PostMessage() many times per frame, with nontrivial overhead.
Reinstate the more selective code that calls PostMessage() with the dummy
message (to bust us out of GetMessage() to check pending window-thread work
requests) at most once per frame.
master
Nat Goodspeed 2021-11-08 15:15:56 -05:00
parent c7af4921db
commit 08336bb469
2 changed files with 13 additions and 18 deletions

View File

@ -488,11 +488,6 @@ struct LLWindowWin32::LLWindowWin32Thread : public LL::ThreadPool
void post(CALLABLE&& func)
{
getQueue().post(std::forward<CALLABLE>(func));
// bump us out of blocked GetMessage() call
if (mWindowHandle)
{
PostMessage(mWindowHandle, WM_USER + 0x0017, 0xB0B0, 0x1337);
}
}
// call PeekMessage and pull enqueue messages for later processing
@ -583,7 +578,6 @@ LLWindowWin32::LLWindowWin32(LLWindowCallbacks* callbacks,
// Make an instance of our window then define the window class
mhInstance = GetModuleHandle(NULL);
mWndProc = NULL;
// Init Direct Input - needed for joystick / Spacemouse
@ -2175,6 +2169,19 @@ void LLWindowWin32::gatherInput()
mRawMouseDelta.mX = 0;
mRawMouseDelta.mY = 0;
}
if (mWindowThread->getQueue().size())
{
LL_PROFILE_ZONE_NAMED("gi - PostMessage");
if (mWindowHandle)
{
// post a nonsense user message to wake up the Window Thread in
// case any functions are pending and no windows events came
// through this frame
PostMessage(mWindowHandle, WM_USER + 0x0017, 0xB0B0, 0x1337);
}
}
while (mWindowThread->mMessageQueue.tryPopBack(msg))
{
@ -2250,17 +2257,6 @@ LRESULT CALLBACK LLWindowWin32::mainWindowProc(HWND h_wnd, UINT u_msg, WPARAM w_
if (NULL != window_imp)
{
// Has user provided their own window callback?
if (NULL != window_imp->mWndProc)
{
LL_PROFILE_ZONE_NAMED("mwp - WndProc");
if (!window_imp->mWndProc(h_wnd, u_msg, w_param, l_param))
{
// user has handled window message
return 0;
}
}
// Juggle to make sure we can get negative positions for when
// mouse is outside window.
LLCoordWindow window_coord((S32)(S16)LOWORD(l_param), (S32)(S16)HIWORD(l_param));

View File

@ -185,7 +185,6 @@ protected:
HGLRC mhRC = 0; // OpenGL rendering context
HDC mhDC = 0; // Windows Device context handle
HINSTANCE mhInstance; // handle to application instance
WNDPROC mWndProc; // user-installable window proc
RECT mOldMouseClip; // Screen rect to which the mouse cursor was globally constrained before we changed it in clipMouse()
WPARAM mLastSizeWParam;
F32 mOverrideAspectRatio;