diff --git a/Core/GameEngine/Source/Common/Audio/AudioEventRTS.cpp b/Core/GameEngine/Source/Common/Audio/AudioEventRTS.cpp index 3eb9db82f39..6cfaa6fdd1c 100644 --- a/Core/GameEngine/Source/Common/Audio/AudioEventRTS.cpp +++ b/Core/GameEngine/Source/Common/Audio/AudioEventRTS.cpp @@ -56,6 +56,7 @@ #include "GameClient/Drawable.h" // For getPosition #include "GameClient/GameClient.h" // For getDrawableByID +#include "Common/ScopedMutex.h" // For ScopedMutex //------------------------------------------------------------------------------------------------- @@ -741,13 +742,21 @@ const Coord3D *AudioEventRTS::getCurrentPosition() return &m_positionOfAudio; case OT_Drawable: - if (Drawable *draw = TheGameClient->findDrawableByID(m_drawableID)) { - m_positionOfAudio.set( draw->getPosition() ); - } - else - { - m_ownerType = OT_Dead; + // Hold the drawable lookup mutex for the duration of the find-and-read to prevent + // the main thread from destroying the drawable (and freeing its memory) between + // findDrawableByID() returning a non-null pointer and getPosition() being called. + // This function can be invoked from the Miles audio background thread via + // notifyOfAudioCompletion(), so the mutex is needed to close this race window. + ScopedMutex mut(TheGameClient->getDrawableLookupMutex()); + if (Drawable *draw = TheGameClient->findDrawableByID(m_drawableID)) + { + m_positionOfAudio.set( draw->getPosition() ); + } + else + { + m_ownerType = OT_Dead; + } } return &m_positionOfAudio; diff --git a/Generals/Code/GameEngine/Include/GameClient/GameClient.h b/Generals/Code/GameEngine/Include/GameClient/GameClient.h index 826359a9971..e5f4b1ae5a0 100644 --- a/Generals/Code/GameEngine/Include/GameClient/GameClient.h +++ b/Generals/Code/GameEngine/Include/GameClient/GameClient.h @@ -99,6 +99,8 @@ class GameClient : public SubsystemInterface, virtual Drawable *findDrawableByID( const DrawableID id ); ///< Given an ID, return the associated drawable + HANDLE getDrawableLookupMutex() const { return m_drawableLookupMutex; } ///< Returns the mutex that guards drawable hash access and drawable lifetime + void setDrawableIDCounter( DrawableID nextDrawableID ) { m_nextDrawableID = nextDrawableID; } DrawableID getDrawableIDCounter() { return m_nextDrawableID; } @@ -160,6 +162,7 @@ class GameClient : public SubsystemInterface, Drawable *m_drawableList; ///< All of the drawables in the world DrawablePtrHash m_drawableHash; ///< Used for DrawableID lookups + HANDLE m_drawableLookupMutex; ///< Guards drawable hash access and drawable lifetime against concurrent audio thread access DrawableID m_nextDrawableID; ///< For allocating drawable id's DrawableID allocDrawableID(); ///< Returns a new unique drawable id diff --git a/Generals/Code/GameEngine/Source/GameClient/GameClient.cpp b/Generals/Code/GameEngine/Source/GameClient/GameClient.cpp index b059a775a59..9fe1ed7c77d 100644 --- a/Generals/Code/GameEngine/Source/GameClient/GameClient.cpp +++ b/Generals/Code/GameEngine/Source/GameClient/GameClient.cpp @@ -33,6 +33,7 @@ // USER INCLUDES ////////////////////////////////////////////////////////////// #include "Common/ActionManager.h" +#include "Common/ScopedMutex.h" #include "Common/GameEngine.h" #include "Common/GameState.h" #include "Common/GameUtility.h" @@ -107,6 +108,8 @@ GameClient::GameClient() m_nextDrawableID = (DrawableID)1; TheDrawGroupInfo = new DrawGroupInfo; + + m_drawableLookupMutex = CreateMutex(nullptr, FALSE, nullptr); } //std::vector preloadTextureNamesGlobalHack; @@ -223,6 +226,9 @@ GameClient::~GameClient() delete TheEva; TheEva = nullptr; + CloseHandle(m_drawableLookupMutex); + m_drawableLookupMutex = nullptr; + } //------------------------------------------------------------------------------------------------- @@ -821,11 +827,13 @@ void GameClient::destroyDrawable( Drawable *draw ) } - // remove the drawable from our hash of drawables - removeDrawableFromLookupTable( draw ); - - // free storage - deleteInstance(draw); + // Remove from hash and free storage under the mutex so that audio threads looking up + // drawables by ID cannot observe a drawable pointer after its memory has been freed. + { + ScopedMutex mut(m_drawableLookupMutex); + removeDrawableFromLookupTable( draw ); + deleteInstance(draw); + } } diff --git a/GeneralsMD/Code/GameEngine/Include/GameClient/GameClient.h b/GeneralsMD/Code/GameEngine/Include/GameClient/GameClient.h index fe42a637f5b..0718c74ada5 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameClient/GameClient.h +++ b/GeneralsMD/Code/GameEngine/Include/GameClient/GameClient.h @@ -104,6 +104,8 @@ class GameClient : public SubsystemInterface, virtual Drawable *findDrawableByID( const DrawableID id ); ///< Given an ID, return the associated drawable + HANDLE getDrawableLookupMutex() const { return m_drawableLookupMutex; } ///< Returns the mutex that guards drawable lookup access and drawable lifetime + void setDrawableIDCounter( DrawableID nextDrawableID ) { m_nextDrawableID = nextDrawableID; } DrawableID getDrawableIDCounter() { return m_nextDrawableID; } @@ -181,6 +183,7 @@ class GameClient : public SubsystemInterface, Drawable *m_drawableList; ///< All of the drawables in the world // DrawablePtrHash m_drawableHash; ///< Used for DrawableID lookups DrawablePtrVector m_drawableVector; + HANDLE m_drawableLookupMutex; ///< Guards drawable lookup access and drawable lifetime against concurrent audio thread access DrawableID m_nextDrawableID; ///< For allocating drawable id's DrawableID allocDrawableID(); ///< Returns a new unique drawable id diff --git a/GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp b/GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp index 9b5300448c0..a805d34d102 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp @@ -33,6 +33,7 @@ // USER INCLUDES ////////////////////////////////////////////////////////////// #include "Common/ActionManager.h" +#include "Common/ScopedMutex.h" #include "Common/GameEngine.h" #include "Common/GameState.h" #include "Common/GameUtility.h" @@ -119,6 +120,8 @@ GameClient::GameClient() m_nextDrawableID = (DrawableID)1; TheDrawGroupInfo = new DrawGroupInfo; + + m_drawableLookupMutex = CreateMutex(nullptr, FALSE, nullptr); } //std::vector preloadTextureNamesGlobalHack; @@ -241,6 +244,9 @@ GameClient::~GameClient() delete TheSnowManager; TheSnowManager = nullptr; + CloseHandle(m_drawableLookupMutex); + m_drawableLookupMutex = nullptr; + } //------------------------------------------------------------------------------------------------- @@ -884,11 +890,13 @@ void GameClient::destroyDrawable( Drawable *draw ) } - // remove the drawable from our hash of drawables - removeDrawableFromLookupTable( draw ); - - // free storage - deleteInstance(draw); + // Remove from lookup and free storage under the mutex so that audio threads looking up + // drawables by ID cannot observe a drawable pointer after its memory has been freed. + { + ScopedMutex mut(m_drawableLookupMutex); + removeDrawableFromLookupTable( draw ); + deleteInstance(draw); + } }