Fix nondeterministic conflicting page processing on startup

Sources/Plasma/PubUtilLib/plResMgr/plResManager.cpp

@@ -1299,7 +1299,7 @@ void plResManager::PageInAge(const char *age)
 
 hsBool plResManager::VerifyPages()
 {
-	hsTArray<plRegistryPageNode*> invalidPages, newerPages;
+	PageSet invalidPages, newerPages;
 
 	// Step 1: verify major/minor version changes
 	if (plResMgrSettings::Get().GetFilterNewerPageVersions() ||
@@ -1313,7 +1313,7 @@ hsBool plResManager::VerifyPages()
 
 			if (page->GetPageCondition() == kPageTooNew && plResMgrSettings::Get().GetFilterNewerPageVersions())
 			{
-				newerPages.Append(page);
+				newerPages.insert(page);
 				fAllPages.erase(page);
 			}
 			else if (
@@ -1321,21 +1321,21 @@ hsBool plResManager::VerifyPages()
 				page->GetPageCondition() == kPageOutOfDate)
 				&& plResMgrSettings::Get().GetFilterOlderPageVersions())
 			{
-				invalidPages.Append(page);
+				invalidPages.insert(page);
 				fAllPages.erase(page);
 			}
 		}
 	}
 
 	// Handle all our invalid pages now
-	if (invalidPages.GetCount() > 0)
+	if (!invalidPages.empty())
 	{
 		if (!IDeleteBadPages(invalidPages, false))
 			return false;
 	}
 
 	// Warn about newer pages
-	if (newerPages.GetCount() > 0)
+	if (!newerPages.empty())
 	{
 		if (!IWarnNewerPages(newerPages))
 			return false;
@@ -1343,22 +1343,35 @@ hsBool plResManager::VerifyPages()
 
 	// Step 2 of verification: make sure no sequence numbers conflict
 	PageSet::iterator it = fAllPages.begin();
-	for (; it != fAllPages.end(); it++)
+	for (; it != fAllPages.end();)
 	{
 		plRegistryPageNode* page = *it;
 
 		PageSet::iterator itUp = it;
 		itUp++;
-		for (; itUp != fAllPages.end(); itUp++)
+		bool amValid = true;
+		for (; itUp != fAllPages.end();)
 		{
 			plRegistryPageNode* upPage = *itUp;
-			if (page->GetPageInfo().GetLocation() == upPage->GetPageInfo().GetLocation())
+			if (page->GetPageInfo().GetLocation() == upPage->GetPageInfo().GetLocation()) {
-			{
+				invalidPages.insert(upPage);
-				invalidPages.Append(upPage);
+				itUp = fAllPages.erase(itUp);
-				fAllPages.erase(itUp);
+				amValid = false;
-				break;
+			} else {
+				itUp++;
 			}
 		}
+
+		// Delete *all* conflicting pages, not just conflicting - 1.
+		// If we leave in a single conflicting page, a new conflict can trivially
+		// be introduced if the page we leave behind is defunct and a new version
+		// is later downloaded from the FileSrv.
+		if (!amValid) {
+			invalidPages.insert(page);
+			it = fAllPages.erase(it);
+		} else {
+			it++;
+		}
 	}
 
 	// Redo our loaded pages list, since Verify() might force the page's keys to load or unload
@@ -1374,7 +1387,7 @@ hsBool plResManager::VerifyPages()
 	}
 
 	// Handle all our conflicting pages now
-	if (invalidPages.GetCount() > 0)
+	if (!invalidPages.empty() > 0)
 		return IDeleteBadPages(invalidPages, true);
 
 	return true;
@@ -1384,9 +1397,10 @@ hsBool plResManager::VerifyPages()
 //	Given an array of pages that are invalid (major version out-of-date or
 //	whatnot), asks the user what we should do about them.
 
-static void ICatPageNames(hsTArray<plRegistryPageNode*>& pages, char* buf, int bufSize)
+static void ICatPageNames(const std::set<plRegistryPageNode*>& pages, char* buf, int bufSize)
 {
-	for (int i = 0; i < pages.GetCount(); i++)
+	int i = 0;
+	for (auto it = pages.cbegin(); it != pages.cend(); ++it, ++i)
 	{
 		if (i >= 25)
 		{
@@ -1394,7 +1408,7 @@ static void ICatPageNames(hsTArray<plRegistryPageNode*>& pages, char* buf, int b
 			break;
 		}
 
-		const char* pagePath = pages[i]->GetPagePath();
+		const char* pagePath = (*it)->GetPagePath();
 		const char* pageFile = plFileUtils::GetFileName(pagePath);
 
 		if (strlen(buf) + strlen(pageFile) > bufSize - 5)
@@ -1408,7 +1422,7 @@ static void ICatPageNames(hsTArray<plRegistryPageNode*>& pages, char* buf, int b
 	}
 }
 
-hsBool plResManager::IDeleteBadPages(hsTArray<plRegistryPageNode*>& invalidPages, hsBool conflictingSeqNums)
+hsBool plResManager::IDeleteBadPages(PageSet& invalidPages, hsBool conflictingSeqNums)
 {
 #ifndef PLASMA_EXTERNAL_RELEASE
 	if (!hsMessageBox_SuppressPrompts)
@@ -1430,12 +1444,12 @@ hsBool plResManager::IDeleteBadPages(hsTArray<plRegistryPageNode*>& invalidPages
 #endif // PLASMA_EXTERNAL_RELEASE
 
 	// Delete 'em
-	for (int i = 0; i < invalidPages.GetCount(); i++)
+	for (PageSet::iterator it = invalidPages.begin(); it != invalidPages.end(); ++it)
 	{
-		invalidPages[i]->DeleteSource();
+		(*it)->DeleteSource();
-		delete invalidPages[i];
+		delete *it;
 	}
-	invalidPages.Reset();
+	invalidPages.clear();
 
 	fLastFoundPage = nil;
 
@@ -1447,7 +1461,7 @@ hsBool plResManager::IDeleteBadPages(hsTArray<plRegistryPageNode*>& invalidPages
 //	than the "current" one), warns the user about them but does nothing to
 //	them.
 
-hsBool plResManager::IWarnNewerPages(hsTArray<plRegistryPageNode*> &newerPages)
+hsBool plResManager::IWarnNewerPages(PageSet &newerPages)
 {
 #ifndef PLASMA_EXTERNAL_RELEASE
 	if (!hsMessageBox_SuppressPrompts)
@@ -1465,9 +1479,9 @@ hsBool plResManager::IWarnNewerPages(hsTArray<plRegistryPageNode*> &newerPages)
 
 
 	// Not deleting the files, just delete them from memory
-	for (int i = 0; i < newerPages.GetCount(); i++)
+	for (PageSet::iterator it = newerPages.begin(); it != newerPages.end(); ++it)
-		delete newerPages[i];
+		delete *it;
-	newerPages.Reset();
+	newerPages.clear();
 
 	fLastFoundPage = nil;
 

Sources/Plasma/PubUtilLib/plResMgr/plResManager.h

@@ -61,6 +61,9 @@ typedef	void(*plProgressProc)(plKey key);
 
 class plResManager : public hsResMgr
 {
+protected:
+	typedef std::set<plRegistryPageNode*> PageSet;
+
 public:
 	plResManager();
 	virtual ~plResManager();
@@ -194,8 +197,8 @@ protected:
 
 	void IUnloadPageKeys(plRegistryPageNode* pageNode, hsBool dontClear = false);
 
-	hsBool IDeleteBadPages(hsTArray<plRegistryPageNode*>& invalidPages, hsBool conflictingSeqNums);
+	hsBool IDeleteBadPages(PageSet& invalidPages, hsBool conflictingSeqNums);
-	hsBool IWarnNewerPages(hsTArray<plRegistryPageNode*>& newerPages);
+	hsBool IWarnNewerPages(PageSet& newerPages);
 
 	void ILockPages();
 	void IUnlockPages();
@@ -233,7 +236,6 @@ protected:
 	UInt8 fPageListLock;	// Number of locks on the page lists.  If it's greater than zero, they can't be modified
 	hsBool fPagesNeedCleanup;	// True if something modified the page lists while they were locked.
 
-	typedef std::set<plRegistryPageNode*> PageSet;
 	PageSet fAllPages;		// All the pages, loaded or not
 	PageSet fLoadedPages;	// Just the loaded pages