Skip to content

Commit 089d92e

Browse files
authored
Merge pull request #488 from GeneralsOnlineDevelopmentTeam/seer/fix/w3dmouse-thread-safety
bugfix(w3dmouse): Prevent race conditions and use-after-free during shutdown
2 parents 3eef518 + b1d4c99 commit 089d92e

1 file changed

Lines changed: 15 additions & 4 deletions

File tree

  • Core/GameEngineDevice/Source/W3DDevice/GameClient

Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DMouse.cpp

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,15 @@ void MouseThreadClass::Thread_Function()
7575
while (running)
7676
{
7777
isThread = TRUE;
78-
if (TheMouse)
79-
TheMouse->draw();
78+
// Hold the mutex for the TheMouse check AND the draw() call together so
79+
// that TheMouse cannot be destroyed between the null-check and the actual
80+
// dereference. CriticalSectionClass uses a Windows CRITICAL_SECTION which
81+
// is re-entrant per-thread, so the nested acquisition inside draw() is safe.
82+
{
83+
CriticalSectionClass::LockClass m(mutex);
84+
if (TheMouse)
85+
TheMouse->draw();
86+
}
8087
isThread = FALSE;
8188
Switch_Thread();
8289
}
@@ -108,6 +115,12 @@ W3DMouse::W3DMouse()
108115

109116
W3DMouse::~W3DMouse()
110117
{
118+
// Stop the mouse update thread FIRST, before freeing any assets it may be
119+
// actively using. If the thread is mid-draw(), Stop() will wait for it to
120+
// finish before returning, preventing a use-after-free when we release
121+
// D3D/W3D resources below.
122+
thread.Stop();
123+
111124
LPDIRECT3DDEVICE8 m_pDev = DX8Wrapper::_Get_D3D_Device8();
112125

113126
if (m_pDev)
@@ -119,8 +132,6 @@ W3DMouse::~W3DMouse()
119132
freeD3DAssets();
120133
freeW3DAssets();
121134

122-
thread.Stop();
123-
124135
}
125136

126137
void W3DMouse::initPolygonAssets()

0 commit comments

Comments
 (0)