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 10, 2024
1 parent 3379e5e commit cbba9a9
Show file tree
Hide file tree
Showing 6 changed files with 26 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
18 changes: 15 additions & 3 deletions src/objmem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@
*/
#include <string.h>

#include "lib/framework/for_each_tuple.h"
#include "lib/framework/frame.h"
#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 @@ -88,9 +90,15 @@ bool objmemInitialise()
/* Release the object heaps */
void objmemShutdown()
{
GlobalDroidContainer().clear();
GlobalStructContainer().clear();
GlobalFeatureContainer().clear();
for_each_tuple(std::tie(GlobalDroidContainer(), GlobalStructContainer(), GlobalFeatureContainer()), [](auto& 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();
});
}

static const char* objTypeToStr(OBJECT_TYPE type)
Expand Down Expand Up @@ -266,6 +274,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 cbba9a9

Please sign in to comment.