From 50f9a499116df1166be01fa5f9c625d615ec836a Mon Sep 17 00:00:00 2001
From: hg <4683435+hgtw@users.noreply.github.com>
Date: Sun, 17 May 2020 00:07:52 -0400
Subject: [PATCH] Check for empty expedition via database not cache

Checking the cache on member removal here isn't reliable due to race
with cross zone message

If a zone removes a member at the same time as another zone, neither zone
can know if the expedition will be empty via cache unless it processes the
world message from the other zone's member removal first.
---
 zone/expedition.cpp          | 18 ++++++++++++++++--
 zone/expedition_database.cpp | 19 +++++++++++++++++++
 zone/expedition_database.h   |  1 +
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/zone/expedition.cpp b/zone/expedition.cpp
index c800c8d78..98fcda966 100644
--- a/zone/expedition.cpp
+++ b/zone/expedition.cpp
@@ -509,9 +509,13 @@ bool Expedition::RemoveMember(const std::string& remove_char_name)
 		ChooseNewLeader();
 	}
 
-	if (m_members.empty())
+	// we can't check for empty member count via cache because if other zones
+	// remove members at the same time then we race. cache member count won't
+	// be accurate until the world messages from other zones are processed
+	uint32_t member_count = ExpeditionDatabase::GetExpeditionMemberCount(m_id);
+	if (member_count == 0)
 	{
-		// cache removal will occur in world message handler
+		// zone cache removal will occur in world message handler
 		ExpeditionDatabase::DeleteExpedition(m_id);
 		if (RuleB(Expedition, EmptyDzShutdownEnabled))
 		{
@@ -1163,6 +1167,11 @@ void Expedition::ProcessMemberRemoved(std::string removed_char_name, uint32_t re
 
 		it = is_removed ? m_members.erase(it) : it + 1;
 	}
+
+	LogExpeditionsDetail(
+		"Processed member [{}] ({}) removal, current zone cache member count: [{}]",
+		removed_char_name, removed_char_id, m_members.size()
+	);
 }
 
 void Expedition::ProcessLockoutUpdate(
@@ -1581,6 +1590,11 @@ void Expedition::HandleWorldMessage(ServerPacket* pack)
 		auto expedition = Expedition::FindCachedExpeditionByID(buf->expedition_id);
 		if (expedition && zone)
 		{
+			LogExpeditionsDetail(
+				"World member change message -- remove: [{}] name: [{}] zone: [{}]:[{}] sender: [{}]:[{}]",
+				buf->removed, buf->char_name, zone->GetZoneID(), zone->GetInstanceID(), buf->sender_zone_id, buf->sender_instance_id
+			);
+
 			if (!zone->IsZone(buf->sender_zone_id, buf->sender_instance_id))
 			{
 				if (buf->removed)
diff --git a/zone/expedition_database.cpp b/zone/expedition_database.cpp
index c3dcd3ef8..4ecda739d 100644
--- a/zone/expedition_database.cpp
+++ b/zone/expedition_database.cpp
@@ -419,6 +419,25 @@ ExpeditionMember ExpeditionDatabase::GetExpeditionLeader(uint32_t expedition_id)
 	return leader;
 }
 
+uint32_t ExpeditionDatabase::GetExpeditionMemberCount(uint32_t expedition_id)
+{
+	auto query = fmt::format(SQL(
+		SELECT COUNT(IF(is_current_member = TRUE, 1, NULL)) member_count
+		FROM expedition_members
+		WHERE expedition_id = {};
+	), expedition_id);
+
+	auto results = database.QueryDatabase(query);
+
+	uint32_t member_count = 0;
+	if (results.Success() && results.RowCount() > 0)
+	{
+		auto row = results.begin();
+		member_count = static_cast<uint32_t>(strtoul(row[0], nullptr, 10));
+	}
+	return member_count;
+}
+
 void ExpeditionDatabase::InsertCharacterLockouts(
 	uint32_t character_id, const std::vector<ExpeditionLockoutTimer>& lockouts,
 	bool update_expire_times, bool is_pending)
diff --git a/zone/expedition_database.h b/zone/expedition_database.h
index bb8804d81..d47ab09f3 100644
--- a/zone/expedition_database.h
+++ b/zone/expedition_database.h
@@ -57,6 +57,7 @@ namespace ExpeditionDatabase
 	uint32_t GetExpeditionIDFromCharacterID(uint32_t character_id);
 	uint32_t GetExpeditionIDFromInstanceID(uint32_t instance_id);
 	ExpeditionMember GetExpeditionLeader(uint32_t expedition_id);
+	uint32_t GetExpeditionMemberCount(uint32_t expedition_id);
 	void InsertCharacterLockouts(
 		uint32_t character_id, const std::vector<ExpeditionLockoutTimer>& lockouts,
 		bool update_expire_times, bool is_pending = false);