Skip to content

Commit

Permalink
objmem: remove audio_RemoveObj call from the dtors of the common in…
Browse files Browse the repository at this point in the history
…-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 <[email protected]>
  • Loading branch information
ManManson committed Aug 11, 2024
1 parent 85310b6 commit 97fc9e5
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 17 deletions.
3 changes: 0 additions & 3 deletions src/baseobject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<OBJECT_TYPE volatile &>(type) = (OBJECT_TYPE)(type + 1000000000); // Hopefully this will trigger an assert if someone uses the freed object.
const_cast<UBYTE volatile &>(player) += 100; // Hopefully this will trigger an assert and/or crash if someone uses the freed object.
}
Expand Down
4 changes: 0 additions & 4 deletions src/droid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
5 changes: 1 addition & 4 deletions src/feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
22 changes: 19 additions & 3 deletions src/objmem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -85,12 +86,23 @@ bool objmemInitialise()
return true;
}

template<typename T>
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)
Expand Down Expand Up @@ -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<void *>(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;
Expand Down
10 changes: 10 additions & 0 deletions src/projectile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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());
Expand Down
3 changes: 0 additions & 3 deletions src/structure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 97fc9e5

Please sign in to comment.