fix #5560

rts/Sim/Misc/SimObjectMemPool.h

@@ -16,6 +16,9 @@ template<size_t N> struct SimObjectMemPool {
 public:
 	template<typename T, typename... A> T* alloc(A&&... a) {
 		static_assert(sizeof(T) <= N, "");
+		// assert(!recursed());
+
+		in_ctor = true;
 
 		T* p = nullptr;
 		uint8_t* m = nullptr;
@@ -28,33 +31,42 @@ template<size_t N> struct SimObjectMemPool {
 			i = pages.size() - 1;
 			m = &pages[i][0];
 			p = new (m) T(std::forward<A>(a)...);
-
-			table.emplace(p, i);
 		} else {
+			// must pop before ctor runs; objects can be created recursively
 			i = spring::VectorBackPop(indcs);
 			m = &pages[i][0];
 			p = new (m) T(std::forward<A>(a)...);
-
-			table.emplace(p, i);
-			// must pop before ctor runs; objects can be created recursively
-			// indcs.pop_back();
 		}
 
+		table.emplace(p, i);
+
+		in_ctor = false;
 		return p;
 	}
 
 	template<typename T> void free(T*& p) {
-		const auto pit = table.find(p); assert(pit != table.end());
-		const size_t idx = pit->second; assert(idx != -1lu);
+		assert(mapped(p));
+		// assert(!recursed());
+
+		in_dtor = true;
 
-		indcs.push_back(idx);
-		table.erase(pit);
+		const auto iter = table.find(p);
+		const auto pair = std::pair<void*, size_t>{iter->first, iter->second};
 
 		spring::SafeDestruct(p);
-		std::memset(&pages[idx][0], 0, N);
+		std::memset(&pages[pair.second][0], 0, N);
+
+		// must push after dtor runs, since that can trigger *another* ctor call
+		// by proxy (~CUnit -> ~CObject -> DependentDied -> CommandAI::FinishCmd
+		// -> CBuilderCAI::ExecBuildCmd -> UnitLoader::LoadUnit -> CUnit e.g.)
+		indcs.push_back(pair.second);
+		table.erase(pair.first);
+
+		in_dtor = false;
 	}
 
-	template<typename T> bool mapped(const T* p) const { return (table.find(p) != table.end()); }
+	bool mapped(void* p) const { return (table.find(p) != table.end()); }
+	bool recursed() const { return (in_ctor || in_dtor); }
 
 	void clear() {
 		pages.clear();
@@ -72,6 +84,9 @@ template<size_t N> struct SimObjectMemPool {
 
 	// <pointer, page index> (non-intrusive)
 	spring::unsynced_map<void*, size_t> table;
+
+	bool in_ctor = false;
+	bool in_dtor = false;
 };
 
 #endif

rts/Sim/Units/UnitHandler.cpp

@@ -187,9 +187,8 @@ void CUnitHandler::DeleteUnitNow(CUnit* delUnit)
 	eventHandler.RenderUnitDestroyed(delUnit);
 
 	const auto it = std::find(activeUnits.begin(), activeUnits.end(), delUnit);
-	assert(it != activeUnits.end());
 
-	{
+	if (it != activeUnits.end()) {
 		const int delTeam = delUnit->team;
 		const int delType = delUnit->unitDef->id;
 
@@ -208,15 +207,9 @@ void CUnitHandler::DeleteUnitNow(CUnit* delUnit)
 		CSolidObject::SetDeletingRefID(delUnit->id);
 		memPool.free(delUnit);
 		CSolidObject::SetDeletingRefID(-1);
+	} else {
+		assert(false);
 	}
-
-#ifdef _DEBUG
-	for (CUnit* u: activeUnits) {
-		if (u == delUnit) {
-			LOG_L(L_ERROR, "Duplicated unit found in active units on erase");
-		}
-	}
-#endif
 }
 
 

rts/System/Object.cpp

@@ -7,7 +7,6 @@
 #include "System/Log/ILog.h"
 #include "System/Platform/CrashHandler.h"
 
-
 CR_BIND(CObject, )
 
 CR_REG_METADATA(CObject, (
@@ -186,13 +185,19 @@ void CObject::PostLoad()
 #endif //USING_CREG
 
 
-// NOTE that we can be listening to a single object from several different places,
-// however objects are responsible for not adding the same dependence more than once,
-// and preferably try to delete the dependence asap in order not to waste memory
+// NOTE:
+//   we can be listening to a single object from several different places
+//   objects are responsible for not adding the same dependence more than
+//   once, and preferably try to delete the dependence ASAP in order not
+//   to waste memory
 void CObject::AddDeathDependence(CObject* obj, DependenceType dep)
 {
 	assert(!detached && !obj->detached);
 
+	// check this explicitly
+	if (detached || obj->detached)
+		return;
+
 	VectorInsertSorted(const_cast<TSyncSafeSet&>(     GetListening(dep)),  obj);
 	VectorInsertSorted(const_cast<TSyncSafeSet&>(obj->GetListeners(dep)), this);
 }
@@ -201,7 +206,8 @@ void CObject::AddDeathDependence(CObject* obj, DependenceType dep)
 void CObject::DeleteDeathDependence(CObject* obj, DependenceType dep)
 {
 	assert(!detached);
-	if (obj->detached)
+
+	if (detached || obj->detached)
 		return;
 
 	const auto it =      listeningDepTbl.find(dep);