From 08336bb469b8db41809893a2ed26599777383eed Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Mon, 8 Nov 2021 15:15:56 -0500
Subject: 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.
---
indra/llwindow/llwindowwin32.cpp | 30 +++++++++++++-----------------
indra/llwindow/llwindowwin32.h | 1 -
2 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/indra/llwindow/llwindowwin32.cpp b/indra/llwindow/llwindowwin32.cpp
index 4b40fae1bf..a26924c2fb 100644
--- a/indra/llwindow/llwindowwin32.cpp
+++ b/indra/llwindow/llwindowwin32.cpp
@@ -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));
diff --git a/indra/llwindow/llwindowwin32.h b/indra/llwindow/llwindowwin32.h
index 97d1e6d7a3..c3ce1e25a9 100644
--- a/indra/llwindow/llwindowwin32.h
+++ b/indra/llwindow/llwindowwin32.h
@@ -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;
--
cgit v1.2.3