[GEN][ZH] Fix undefined behavior with MemoryPoolObject::deleteInstance (#870)

Author: xezon

Core/GameEngine/Source/Common/System/XferSave.cpp

@@ -91,7 +91,7 @@ XferSave::~XferSave( void )
 		{
 
 			next = m_blockStack->next;
-			m_blockStack->deleteInstance();
+			MemoryPoolObject::deleteInstance(m_blockStack);
 			m_blockStack = next;
 
 		}  // end while
@@ -247,7 +247,7 @@ void XferSave::endBlock( void )
 	fseek( m_fileFP, currentFilePos, SEEK_SET );
 
 	// delete the block data as it's all used up now
-	top->deleteInstance();
+	MemoryPoolObject::deleteInstance(top);
 
 }  // end endBlock
 

Generals/Code/GameEngine/Include/Common/GameMemory.h

@@ -756,13 +756,13 @@ class MemoryPoolObject
 
 public: 
 
-	void deleteInstance() 
+	static void deleteInstance(MemoryPoolObject* mpo) 
 	{	
-		if (this)
+		if (mpo)
 		{
-			MemoryPool *pool = this->getObjectMemoryPool(); // save this, since the dtor will nuke our vtbl
-			this->~MemoryPoolObject();	// it's virtual, so the right one will be called.
-			pool->freeBlock((void *)this); 
+			MemoryPool *pool = mpo->getObjectMemoryPool(); // save this, since the dtor will nuke our vtbl
+			mpo->~MemoryPoolObject();	// it's virtual, so the right one will be called.
+			pool->freeBlock((void *)mpo); 
 		}
 	} 
 };
@@ -906,7 +906,7 @@ class MemoryPoolObjectHolder
 	MemoryPoolObjectHolder(MemoryPoolObject *mpo = NULL) : m_mpo(mpo) { }
 	void hold(MemoryPoolObject *mpo) { DEBUG_ASSERTCRASH(!m_mpo, ("already holding")); m_mpo = mpo; }
 	void release() { m_mpo = NULL; }
-	~MemoryPoolObjectHolder() { m_mpo->deleteInstance(); }
+	~MemoryPoolObjectHolder() { MemoryPoolObject::deleteInstance(m_mpo); }
 };
 
 
@@ -919,7 +919,11 @@ class MemoryPoolObjectHolder
 	you really want by including this macro 
 */
 #define MEMORY_POOL_DELETEINSTANCE_VISIBILITY(ARGVIS)\
-ARGVIS:	void deleteInstance() { MemoryPoolObject::deleteInstance(); } public: 
+ARGVIS: \
+	static void deleteInstance(MemoryPoolObject* object) { \
+		MemoryPoolObject::deleteInstance(object); \
+	} \
+public: 
 
 
 #define EMPTY_DTOR(CLASS) inline CLASS::~CLASS() { }

Generals/Code/GameEngine/Include/Common/GameMemoryNull.h

@@ -108,9 +108,9 @@ class MemoryPoolObject
 
 public:
 
-	void deleteInstance() 
+	static void deleteInstance(MemoryPoolObject* mpo) 
 	{
-		delete this;
+		delete mpo;
 	}
 };
 

Generals/Code/GameEngine/Include/Common/Overridable.h

@@ -108,7 +108,7 @@ class Overridable : public MemoryPoolObject
 		{
 			if ( m_isOverride )
 			{
-				deleteInstance();
+				MemoryPoolObject::deleteInstance(this);
 				return NULL;
 			}
 			else if ( m_nextOverride )
@@ -123,7 +123,7 @@ class Overridable : public MemoryPoolObject
 __inline Overridable::~Overridable() 
 {
 	if (m_nextOverride) 
-		m_nextOverride->deleteInstance();
+		MemoryPoolObject::deleteInstance(m_nextOverride);
 }
 
 

Generals/Code/GameEngine/Include/Common/ThingTemplate.h

@@ -159,7 +159,7 @@ class AudioArray
 	{
 		for (Int i = 0; i < TTAUDIO_COUNT; ++i)
 			if (m_audio[i])
-				m_audio[i]->deleteInstance();
+				MemoryPoolObject::deleteInstance(m_audio[i]);
 	}
 
 	AudioArray(const AudioArray& that)

Generals/Code/GameEngine/Include/GameLogic/Object.h

@@ -386,7 +386,7 @@ class Object : public Thing, public Snapshot
 	Bool isInList(Object **pListHead) const;
 
 	// this is intended for use ONLY by GameLogic.
-	void friend_deleteInstance() { deleteInstance(); }
+	static void friend_deleteInstance(MemoryPoolObject* object) { MemoryPoolObject::deleteInstance(object); }
 
 	/// cache the partition module (should be called only by PartitionData)
 	void friend_setPartitionData(PartitionData *pd) { m_partitionData = pd; }

Generals/Code/GameEngine/Include/GameLogic/ObjectIter.h

@@ -67,7 +67,7 @@ enum IterOrderType CPP_11(: Int)
 	{
 		// do something with other
 	}
-	iter->deleteInstance();								// you own it, so you must delete it
+	MemoryPoolObject::deleteInstance(iter);								// you own it, so you must delete it
 
 	note that the iterator is required to deal intelligently with deleted objects; 
 	in particular, next() will check if an obj has been killed and simply skip it.

Generals/Code/GameEngine/Source/Common/Audio/GameAudio.cpp

@@ -188,7 +188,7 @@ AudioManager::~AudioManager()
 	for (it = m_allAudioEventInfo.begin(); it != m_allAudioEventInfo.end(); ++it) {
 		AudioEventInfo *eventInfo = (*it).second;
 		if (eventInfo) {
-			eventInfo->deleteInstance();
+			MemoryPoolObject::deleteInstance(eventInfo);
 			eventInfo = NULL;
 		}
 	}
@@ -811,7 +811,7 @@ AudioRequest *AudioManager::allocateAudioRequest( Bool useAudioEvent )
 void AudioManager::releaseAudioRequest( AudioRequest *requestToRelease )
 {
 	if (requestToRelease) {
-		requestToRelease->deleteInstance();
+		MemoryPoolObject::deleteInstance(requestToRelease);
 	}
 }
 
@@ -894,7 +894,7 @@ void AudioManager::removeLevelSpecificAudioEventInfos(void)
 
     if ( it->second->isLevelSpecific() )
     {
-      it->second->deleteInstance();
+      MemoryPoolObject::deleteInstance(it->second);
       m_allAudioEventInfo.erase( it );
     }
 

Generals/Code/GameEngine/Source/Common/GameEngine.cpp

@@ -536,7 +536,7 @@ void GameEngine::reset( void )
 	if(background)
 	{
 		background->destroyWindows();
-		background->deleteInstance();
+		MemoryPoolObject::deleteInstance(background);
 		background = NULL;
 	}
 }

Generals/Code/GameEngine/Source/Common/GlobalData.cpp

@@ -1061,7 +1061,7 @@ GlobalData::~GlobalData( void )
 	DEBUG_ASSERTCRASH( TheWritableGlobalData->m_next == NULL, ("~GlobalData: theOriginal is not original\n") );
 
 	if (m_weaponBonusSet)
-		m_weaponBonusSet->deleteInstance();
+		MemoryPoolObject::deleteInstance(m_weaponBonusSet);
 
 	if( m_theOriginal == this )	{
 		m_theOriginal = NULL;