From ad6a918717f7dcec600dd16b5b8ab7850aeb079a Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Sat, 10 Aug 2024 16:19:12 +0300 Subject: [PATCH] objmem: remove `audio_RemoveObj` call from the dtors of the common in-game objects The call to `audio_RemoveObj` inside the destructors of `SIMPLE_OBJECT`, `DROID`, `STRUCTURE`, `FEATURE` and `PROJECTILE` classes can possibly invoke undefined behavior if the warzone process is being forcefully shut down. The reason for this is that by the time a given game object is being destroyed, chances are that the static objects from the audio code, which are being accessed by the `audio_RemoveObj()`, can already be destroyed (due to the inverse of the [Static initialization order fiasco](https://en.cppreference.com/w/cpp/language/siof)). Signed-off-by: Pavel Solodovnikov --- src/baseobject.cpp | 3 --- src/droid.cpp | 4 ---- src/feature.cpp | 5 +---- src/objmem.cpp | 22 +++++++++++++++++++--- src/projectile.cpp | 10 ++++++++++ src/structure.cpp | 3 --- 6 files changed, 30 insertions(+), 17 deletions(-) diff --git a/src/baseobject.cpp b/src/baseobject.cpp index 7385b19b892..0f21f6669af 100644 --- a/src/baseobject.cpp +++ b/src/baseobject.cpp @@ -85,9 +85,6 @@ SIMPLE_OBJECT::SIMPLE_OBJECT(OBJECT_TYPE type, uint32_t id, unsigned player) SIMPLE_OBJECT::~SIMPLE_OBJECT() { - // Make sure to get rid of some final references in the sound code to this object first - audio_RemoveObj(this); - const_cast(type) = (OBJECT_TYPE)(type + 1000000000); // Hopefully this will trigger an assert if someone uses the freed object. const_cast(player) += 100; // Hopefully this will trigger an assert and/or crash if someone uses the freed object. } diff --git a/src/droid.cpp b/src/droid.cpp index 49c22920d16..185ba7158ab 100644 --- a/src/droid.cpp +++ b/src/droid.cpp @@ -439,10 +439,6 @@ DROID::DROID(uint32_t id, unsigned player) */ DROID::~DROID() { - // Make sure to get rid of some final references in the sound code to this object first - // In BASE_OBJECT::~BASE_OBJECT() is too late for this, since some callbacks require us to still be a DROID. - audio_RemoveObj(this); - DROID *psDroid = this; if (psDroid->isTransporter()) diff --git a/src/feature.cpp b/src/feature.cpp index f1b42911672..dde621597dd 100644 --- a/src/feature.cpp +++ b/src/feature.cpp @@ -358,10 +358,7 @@ FEATURE::FEATURE(uint32_t id, FEATURE_STATS const *psStats) /* Release the resources associated with a feature */ FEATURE::~FEATURE() -{ - // Make sure to get rid of some final references in the sound code to this object first - audio_RemoveObj(this); -} +{} void _syncDebugFeature(const char *function, FEATURE const *psFeature, char ch) { diff --git a/src/objmem.cpp b/src/objmem.cpp index 12f641fab97..4a07fd8eb91 100644 --- a/src/objmem.cpp +++ b/src/objmem.cpp @@ -29,6 +29,7 @@ #include "objects.h" #include "lib/gamelib/gtime.h" #include "lib/netplay/sync_debug.h" +#include "lib/sound/audio.h" #include "hci.h" #include "map.h" #include "power.h" @@ -85,12 +86,23 @@ bool objmemInitialise() return true; } +template +static void objMemShutdownContainerImpl(T& container) +{ + for (const auto& entity : container) + { + // Make sure to get rid of some final references in the sound code to this object first + audio_RemoveObj(&entity); + } + container.clear(); +} + /* Release the object heaps */ void objmemShutdown() { - GlobalDroidContainer().clear(); - GlobalStructContainer().clear(); - GlobalFeatureContainer().clear(); + objMemShutdownContainerImpl(GlobalDroidContainer()); + objMemShutdownContainerImpl(GlobalStructContainer()); + objMemShutdownContainerImpl(GlobalFeatureContainer()); } static const char* objTypeToStr(OBJECT_TYPE type) @@ -266,6 +278,10 @@ bool objmemDestroy(BASE_OBJECT *psObj, bool checkRefs) default: ASSERT(!"unknown object type", "unknown object type in destroyed list at 0x%p", static_cast(psObj)); } + + // Make sure to get rid of some final references in the sound code to this object first + audio_RemoveObj(psObj); + if (checkRefs && !checkReferences(psObj)) { return false; diff --git a/src/projectile.cpp b/src/projectile.cpp index c6bc1cd080b..46d2800c9ab 100644 --- a/src/projectile.cpp +++ b/src/projectile.cpp @@ -233,6 +233,12 @@ void proj_AddActiveProjectile(PROJECTILE* p) void proj_FreeAllProjectiles() { + for (const auto* p : psProjectileList) + { + // Make sure to get rid of some final references in the sound code to this object first + audio_RemoveObj(p); + } + psProjectileList.clear(); psProjectileNext = psProjectileList.end(); @@ -1466,6 +1472,10 @@ void proj_UpdateAll() } auto it = globalProjectileStorage.find(*p); ASSERT(it != globalProjectileStorage.end(), "Invalid projectile, not found in global storage"); + + // Make sure to get rid of some final references in the sound code to this object first + audio_RemoveObj(p); + globalProjectileStorage.erase(it); return true; }), psProjectileList.end()); diff --git a/src/structure.cpp b/src/structure.cpp index 001cbdbcd19..fc81e9b9957 100644 --- a/src/structure.cpp +++ b/src/structure.cpp @@ -3992,9 +3992,6 @@ STRUCTURE::STRUCTURE(uint32_t id, unsigned player) /* Release all resources associated with a structure */ STRUCTURE::~STRUCTURE() { - // Make sure to get rid of some final references in the sound code to this object first - audio_RemoveObj(this); - STRUCTURE *psBuilding = this; // free up the space used by the functionality array