Skip to content

Commit

Permalink
Fix a rare, but nasty desync caused by a unit selection (or lack ther…
Browse files Browse the repository at this point in the history
…eof) (#1174)

Thanks @6AKU66 for discovering this one.
  • Loading branch information
lhog authored Jan 2, 2024
1 parent 4eadd5f commit 192c9dd
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 14 deletions.
23 changes: 17 additions & 6 deletions rts/System/Object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,25 @@ CObject::~CObject()
assert(!detached);
detached = true;

for (const auto& p: listenersDepTbl) {
assert(p.first >= DEPENDENCE_ATTACKER && p.first < DEPENDENCE_COUNT);
assert(p.second < listeners.size());

for (CObject* obj: listeners[p.second]) {
// NB: listenersDepTbl must be iterated sequentially
// due to the presence of obj->DependentDied(this);
// The order of naive iteration becomes undefined in
// case "unsynced" dependencies like DEPENDENCE_SELECTED
// are present in listenersDepTbl
// Ex: can't use `for (const auto& p : listenersDepTbl) {}`
for (std::underlying_type_t<DependenceType> dt = DEPENDENCE_ATTACKER, cnt = 0; dt < DEPENDENCE_COUNT && cnt < listenersDepTbl.size(); ++dt) {
const auto it = listenersDepTbl.find(dt);
if (it == listenersDepTbl.end())
continue;

++cnt;

assert(it->second < listeners.size());

for (CObject* obj: listeners[it->second]) {
obj->DependentDied(this);

const auto jt = obj->listeningDepTbl.find(p.first);
const auto jt = obj->listeningDepTbl.find(it->first);

if (jt == obj->listeningDepTbl.end())
continue;
Expand Down
5 changes: 3 additions & 2 deletions rts/System/Object.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <atomic>
#include <functional>
#include <type_traits>
#include <vector>

#include "ObjectDependenceTypes.h"
Expand Down Expand Up @@ -125,8 +126,8 @@ class CObject
template<size_t N> void FilterListening(const TObjFilterPred& fp, std::array<int, N>& ids) const { FilterDepObjects(listening, fp, ids); }

protected:
spring::unordered_map<int, size_t> listenersDepTbl; // maps dependence-type to index into listeners
spring::unordered_map<int, size_t> listeningDepTbl; // maps dependence-type to index into listening
spring::unordered_map<std::underlying_type_t<DependenceType>, size_t> listenersDepTbl; // maps dependence-type to index into listeners
spring::unordered_map<std::underlying_type_t<DependenceType>, size_t> listeningDepTbl; // maps dependence-type to index into listening

TDependenceMap listeners;
TDependenceMap listening;
Expand Down
14 changes: 8 additions & 6 deletions rts/System/ObjectDependenceTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// AddDeathDependence / DeleteDeathDependence calls in order not to risk a crash. With dependence types this can never happen, i.e.
// DeleteDeathDependence(object, DEPENDENCE_ATTACKER) can be called a hundred times without any risk of losing some other type of dependence.
// Dependence types also makes it easy to detect deletion of non-existent dependence types, and output a warning for debugging purposes.
enum DependenceType {
enum DependenceType : uint8_t {
DEPENDENCE_ATTACKER,
DEPENDENCE_BUILD,
DEPENDENCE_BUILDER,
Expand All @@ -21,14 +21,9 @@ enum DependenceType {
DEPENDENCE_INTERCEPTABLE,
DEPENDENCE_INTERCEPTTARGET,
DEPENDENCE_LASTCOLWARN,
DEPENDENCE_LIGHT,
DEPENDENCE_ORDERTARGET,
DEPENDENCE_RECLAIM,
DEPENDENCE_REPULSE,
DEPENDENCE_REPULSED,
DEPENDENCE_RESURRECT,
DEPENDENCE_SELECTED,
DEPENDENCE_SOLIDONTOP,
DEPENDENCE_TARGET,
DEPENDENCE_TARGETUNIT,
DEPENDENCE_TERRAFORM,
Expand All @@ -37,6 +32,13 @@ enum DependenceType {
DEPENDENCE_WAITCMD,
DEPENDENCE_WEAPON,
DEPENDENCE_WEAPONTARGET,
DEPENDENCE_REPULSE,
DEPENDENCE_REPULSED,
DEPENDENCE_SOLIDONTOP,
// unsynced, keep them last
DEPENDENCE_LIGHT,
DEPENDENCE_SELECTED,

DEPENDENCE_NONE,
DEPENDENCE_COUNT
};
Expand Down

0 comments on commit 192c9dd

Please sign in to comment.