Fix crash in DismissPet when owner dies

Fix #224 crash

EQ2/source/WorldServer/Combat.cpp

@@ -1080,14 +1080,18 @@ void Entity::KillSpawn(Spawn* dead, int8 damage_type, int16 kill_blow_type) {
 	//if (dead->IsEntity())								same code called in zone server
 		//((Entity*)dead)->InCombat(false);
 	
+
+	/* just for sake of not knowing if we are in a read lock, write lock, or no lock
+	**  say spawnlist is locked (DismissPet arg 3 true), which means RemoveSpawn will remove the id from the spawn_list outside of the potential lock
+	*/
 	if (dead->IsPet())
-		((NPC*)dead)->GetOwner()->DismissPet((NPC*)dead, true);
+		((NPC*)dead)->GetOwner()->DismissPet((NPC*)dead, true, true);
 	else if (dead->IsEntity()) {
 		// remove all pets for this entity
-		((Entity*)dead)->DismissPet((NPC*)((Entity*)dead)->GetPet());
-		((Entity*)dead)->DismissPet((NPC*)((Entity*)dead)->GetCharmedPet());
-		((Entity*)dead)->DismissPet((NPC*)((Entity*)dead)->GetDeityPet());
-		((Entity*)dead)->DismissPet((NPC*)((Entity*)dead)->GetCosmeticPet());
+		((Entity*)dead)->DismissPet((NPC*)((Entity*)dead)->GetPet(), false, true);
+		((Entity*)dead)->DismissPet((NPC*)((Entity*)dead)->GetCharmedPet(), false, true);
+		((Entity*)dead)->DismissPet((NPC*)((Entity*)dead)->GetDeityPet(), false, true);
+		((Entity*)dead)->DismissPet((NPC*)((Entity*)dead)->GetCosmeticPet(), false, true);
 	}
 
 	// If not in combat and no one in the encounter list add this killer to the list

EQ2/source/WorldServer/Entity.cpp

@@ -1125,7 +1125,7 @@ void Entity::HideCosmeticPet(bool val) {
 		cosmeticPet->MakeSpawnPublic();
 }
 
-void Entity::DismissPet(NPC* pet, bool from_death) {
+void Entity::DismissPet(NPC* pet, bool from_death, bool spawnListLocked) {
 	if (!pet)
 		return;
 
@@ -1164,7 +1164,7 @@ void Entity::DismissPet(NPC* pet, bool from_death) {
 
 	// remove the spawn from the world
 	if (!from_death && pet->GetPetType() != PET_TYPE_CHARMED)
-		GetZone()->RemoveSpawn(false, pet);
+		GetZone()->RemoveSpawn(spawnListLocked, pet);
 }
 
 float Entity::CalculateBonusMod() {

EQ2/source/WorldServer/Entity.h

@@ -680,7 +680,7 @@ public:
 
 	void HideDeityPet(bool val);
 	void HideCosmeticPet(bool val);
-	void DismissPet(NPC* pet, bool from_death = false);
+	void DismissPet(NPC* pet, bool from_death = false, bool spawnListLocked = false);
 
 	/// <summary>Creates a loot chest to drop in the world</summary>
 	/// <returns>Pointer to the chest</returns>

EQ2/source/WorldServer/LuaFunctions.cpp

@@ -4948,7 +4948,7 @@ int EQ2Emu_lua_DismissPet(lua_State* state) {
 	}
 
 	if (!((NPC*)spawn)->IsDismissing())
-		((NPC*)spawn)->GetOwner()->DismissPet((NPC*)spawn);
+		((NPC*)spawn)->GetOwner()->DismissPet((NPC*)spawn, false, true);
 
 	return 0;
 }

EQ2/source/WorldServer/LuaInterface.cpp

@@ -560,8 +560,7 @@ void LuaInterface::AddSpawnPointers(LuaSpell* spell, bool first_cast, bool preca
 	if (temp_spawn)
 		SetSpawnValue(spell->state, temp_spawn);
 	else {
-		if(spell->caster && spell->initial_target)
-		if(spell->caster && spell->initial_target)
+		if(spell->caster && spell->caster->GetZone() != nullptr && spell->initial_target)
 		{
 			// easier to debug target id as ptr
 			Spawn* new_target = spell->caster->GetZone()->GetSpawnByID(spell->initial_target);

EQ2/source/WorldServer/client.cpp

@@ -9230,7 +9230,6 @@ bool Client::HandleNewLogin(int32 account_id, int32 access_code)
 						GetPlayer()->SetPendingDeletion(false);
 						GetPlayer()->ResetSavedSpawns();
 						GetPlayer()->SetReturningFromLD(true);
-						GetPlayer()->GetZone()->RemoveDelayedSpawnRemove(GetPlayer());
 					}
 					ZoneServer* tmpZone = client->GetCurrentZone();
 					tmpZone->RemoveClientImmediately(client);

EQ2/source/WorldServer/zoneserver.cpp

@@ -158,6 +158,8 @@ ZoneServer::ZoneServer(const char* name) {
 	
 	reloading = true;
 	watchdogTimestamp = Timer::GetCurrentTime2();
+
+	MPendingSpawnRemoval.SetName("ZoneServer::MPendingSpawnRemoval");
 }
 
 ZoneServer::~ZoneServer() {
@@ -180,7 +182,6 @@ ZoneServer::~ZoneServer() {
 	DeleteData(true);
 	RemoveLocationProximities();
 	RemoveLocationGrids();
-	DelayedSpawnRemoval(true);
 	DeleteSpawns(true);
 	
 	DeleteGlobalSpawns();
@@ -1166,50 +1167,6 @@ bool ZoneServer::CombatProcess(Spawn* spawn) {
 	return ret;
 }
 
-void ZoneServer::AddDelayedSpawnRemove(Spawn* spawn, int32 time) {
-	// this prevents a crash when a zone shuts down with a client in it
-	if (zoneShuttingDown)
-		return;
-
-	if (delayed_spawn_remove_list.count(spawn->GetID(), true) == 0)
-		delayed_spawn_remove_list.Put(spawn->GetID(), Timer::GetCurrentTime2() + time);
-}
-
-void ZoneServer::RemoveDelayedSpawnRemove(Spawn* spawn) {
-	if (delayed_spawn_remove_list.count(spawn->GetID(), true) > 0)
-		delayed_spawn_remove_list.erase(spawn->GetID());
-}
-
-void ZoneServer::DelayedSpawnRemoval(bool force_delete_all) {
-	if (delayed_spawn_remove_list.size(true) > 0) {
-		MutexMap<int32, int32>::iterator itr = delayed_spawn_remove_list.begin();
-		int32 current_time = Timer::GetCurrentTime2();
-		Spawn* spawn = 0;
-		// Cycle through all spawns scheduled for removal
-		while (itr.Next()) {
-			// If it is time for removal, or a force delete of all, then remove the spawn
-			if (force_delete_all || current_time >= itr->second) {
-				spawn = GetSpawnByID(itr->first);
-				delayed_spawn_remove_list.erase(itr->first);
-				if (spawn) {
-					if (spawn->IsEntity()) {
-						// Remove pets
-						((Entity*)spawn)->DismissPet((NPC*)((Entity*)spawn)->GetPet());
-						((Entity*)spawn)->DismissPet((NPC*)((Entity*)spawn)->GetCharmedPet());
-						((Entity*)spawn)->DismissPet((NPC*)((Entity*)spawn)->GetDeityPet());
-						((Entity*)spawn)->DismissPet((NPC*)((Entity*)spawn)->GetCosmeticPet());
-					}
-
-					if (spawn->IsPlayer())
-						RemoveSpawn(false, spawn, false);
-					else
-						RemoveSpawn(false, spawn);
-				}
-			}
-		}
-	}
-}
-
 void ZoneServer::AddPendingDelete(Spawn* spawn) {
 	MSpawnDeleteList.writelock(__FUNCTION__, __LINE__);
 	if (spawn_delete_list.count(spawn) == 0)
@@ -1389,10 +1346,6 @@ bool ZoneServer::Process()
 			return !zoneShuttingDown;
 		}
 
-		// Remove LD Player whos LD timer has expired
-		if (!zoneShuttingDown)
-			DelayedSpawnRemoval(false);
-
 		// client loop
 		if(charsheet_changes.Check())
 			SendCharSheetChanges();
@@ -1547,6 +1500,19 @@ bool ZoneServer::SpawnProcess(){
 			SendSpawnChanges();
 		}
 
+		// Check to see if there are any spawn id's that need to be removed from the spawn list, if so remove them all
+		MPendingSpawnRemoval.writelock(__FUNCTION__, __LINE__);
+		if (m_pendingSpawnRemove.size() > 0) {
+			MSpawnList.writelock(__FUNCTION__, __LINE__);
+			vector<int32>::iterator itr2;
+			for (itr2 = m_pendingSpawnRemove.begin(); itr2 != m_pendingSpawnRemove.end(); itr2++) 
+				spawn_list.erase(*itr2);
+
+			m_pendingSpawnRemove.clear();
+			MSpawnList.releasewritelock(__FUNCTION__, __LINE__);
+		}
+		MPendingSpawnRemoval.releasewritelock(__FUNCTION__, __LINE__);
+
 		if (movement || aggroCheck)
 		{
 			MSpawnList.readlock(__FUNCTION__, __LINE__);
@@ -3001,7 +2967,6 @@ void ZoneServer::RemoveClient(Client* client)
 			if( (client->GetPlayer()->GetActivityStatus() & ACTIVITY_STATUS_LINKDEAD) > 0)
 			{
 				LogWrite(ZONE__DEBUG, 0, "Zone", "Removing client '%s' (%u) due to LD/Exit...", client->GetPlayer()->GetName(), client->GetPlayer()->GetCharacterID());
-				//AddDelayedSpawnRemove(client->GetPlayer(), LD_Timer);
 				//connected_clients.Remove(client, true, LD_Timer + DisconnectClientTimer);
 			}
 			else
@@ -3848,16 +3813,17 @@ void ZoneServer::RemoveSpawn(bool spawnListLocked, Spawn* spawn, bool delete_spa
 	if (spawn_expire_timers.count(spawn->GetID()) > 0)
 		spawn_expire_timers.erase(spawn->GetID());
 	
-	RemoveDelayedSpawnRemove(spawn);
-
 	// Clear the pointer in the spawn list, spawn thread will remove the key
 	if (!spawnListLocked)
+	{
 		MSpawnList.writelock(__FUNCTION__, __LINE__);
-	
-	spawn_list.erase(spawn->GetID());
-	
-	if (!spawnListLocked)
+		spawn_list.erase(spawn->GetID());
 		MSpawnList.releasewritelock(__FUNCTION__, __LINE__);
+	}
+	else
+	{
+		AddPendingSpawnRemove(spawn->GetID());
+	}
 
 	PacketStruct* packet = 0;
 	int16 packet_version = 0;
@@ -7832,4 +7798,11 @@ Spawn* ZoneServer::GetSpawnFromUniqueItemID(int32 unique_id)
 	MSpawnList.releasereadlock();
 
 	return nullptr;
+}
+
+void ZoneServer::AddPendingSpawnRemove(int32 id)
+{
+		MPendingSpawnRemoval.writelock(__FUNCTION__, __LINE__);
+		m_pendingSpawnRemove.push_back(id);
+		MPendingSpawnRemoval.releasewritelock(__FUNCTION__, __LINE__);
 }

EQ2/source/WorldServer/zoneserver.h

@@ -404,15 +404,6 @@ public:
 	
 	void    RemoveTargetFromSpell(LuaSpell* spell, Spawn* target);
 
-	/// <summary>Schedules a spawn for removal</summary>
-	/// <param name='spawn'>The spawn to remove</param>
-	/// <param name='time'>The delay before removing the spawn</param>
-	void	AddDelayedSpawnRemove(Spawn* spawn, int32 time);
-
-	/// <summary>Removes a spawn from scheduled removal (used mainly for players returning from LD)</summary>
-	/// <param name='spawn'>The spawn to remove from the scheduled removal</param>
-	void	RemoveDelayedSpawnRemove(Spawn* spawn);
-
 	/// <summary>Set the rain levl in the zone</summary>
 	/// <param name='val'>Level of rain in the zone 0.0 - 1.1 (rain starts at 0.76)</param>
 	void	SetRain(float val);
@@ -669,6 +660,8 @@ public:
 	int32 GetWatchdogTime() { return watchdogTimestamp; }
 	void SetWatchdogTime(int32 time) { watchdogTimestamp = time; }
 	void CancelThreads();
+
+	void AddPendingSpawnRemove(int32 id);
 private:
 #ifndef WIN32
 	pthread_t ZoneThread;
@@ -746,7 +739,6 @@ private:
 	void	ProcessAggroChecks(Spawn* spawn);																	// never used outside zone server
 	/// <summary>Checks to see if it is time to remove a spawn and removes it</summary>
 	/// <param name='force_delete_all'>Forces all spawns scheduled to be removed regardless of time</param>
-	void	DelayedSpawnRemoval(bool force_delete_all);															// never used outside zone server
 	bool CombatProcess(Spawn* spawn);																			// never used outside zone server
 	void	InitWeather();																						// never used outside zone server
 	///<summary>Dismiss all pets in the zone, useful when the spell process needs to be reloaded</summary>
@@ -768,7 +760,6 @@ private:
 	
 	/* Mutex Maps */
 	MutexMap<Spawn*, Client*>						client_spawn_map;								// ok
-	MutexMap<int32, int32>							delayed_spawn_remove_list;						// 1st int32 = spawn id, 2nd int32 = time
 	MutexMap<Client*, int32>						drowning_victims;
 	MutexMap<Spawn*, int32>							heading_timers;
 	MutexMap<int32, int32>							movement_spawns;								// 1st int32 = spawn id
@@ -947,6 +938,9 @@ private:
 	map<int32, string> m_transportMaps;
 	
 	int32 watchdogTimestamp;
+
+	vector<int32> m_pendingSpawnRemove;
+	Mutex MPendingSpawnRemoval;
 public:
 	Spawn*				GetSpawn(int32 id);