From cf15a0913ddaaa276005db3c9bd4fff7d88f7662 Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <dktapps@pmmp.io>
Date: Thu, 28 Apr 2022 20:23:23 +0100
Subject: [PATCH] World: fixed a corner case assertion failure in generation
 system

This required the following:
- A generation task (taskA) to already be running for any chunk (chunkA)
- A chunk (chunkB) is requested for generation, and the task (taskB) to do the generation
  is commenced immediately
- chunkB generation promise is aborted (e.g. due to chunk unload) and
  taskB is orphaned
- chunkB is subsequently re-requested, but ends up in the generation
  queue because taskB is still running
- taskA completes and drains the generation queue
- chunkB attempts to be populated a second time, but taskB has not yet
  been collected, resulting in an assertion failure.

This bug has been appearing intermittently ever since PM 4.0 release.
For most users there is no obvious effect since production servers don't
have assertions enabled; however, it's unclear what kind of weird side
effects this bug may have had.
---
 src/world/World.php | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/world/World.php b/src/world/World.php
index 6c2a6a9e5..9be1d06e4 100644
--- a/src/world/World.php
+++ b/src/world/World.php
@@ -2653,8 +2653,13 @@ class World implements ChunkManager{
 		unset($this->changedBlocks[$chunkHash]);
 
 		if(array_key_exists($chunkHash, $this->chunkPopulationRequestMap)){
+			$this->logger->debug("Rejecting population promise for chunk $x $z");
 			$this->chunkPopulationRequestMap[$chunkHash]->reject();
 			unset($this->chunkPopulationRequestMap[$chunkHash]);
+			if(isset($this->activeChunkPopulationTasks[$chunkHash])){
+				$this->logger->debug("Marking population task for chunk $x $z as orphaned");
+				$this->activeChunkPopulationTasks[$chunkHash] = false;
+			}
 		}
 
 		$this->timings->doChunkUnload->stopTiming();
@@ -2830,7 +2835,7 @@ class World implements ChunkManager{
 			unset($this->chunkPopulationRequestQueueIndex[$nextChunkHash]);
 			World::getXZ($nextChunkHash, $nextChunkX, $nextChunkZ);
 			if(isset($this->chunkPopulationRequestMap[$nextChunkHash])){
-				assert(!isset($this->activeChunkPopulationTasks[$nextChunkHash]), "Population for chunk $nextChunkX $nextChunkZ already running");
+				assert(!($this->activeChunkPopulationTasks[$nextChunkHash] ?? false), "Population for chunk $nextChunkX $nextChunkZ already running");
 				if(
 					!$this->orderChunkPopulation($nextChunkX, $nextChunkZ, null)->isResolved() &&
 					!isset($this->activeChunkPopulationTasks[$nextChunkHash])
@@ -2999,10 +3004,13 @@ class World implements ChunkManager{
 		}
 
 		$index = World::chunkHash($x, $z);
-		if(!isset($this->chunkPopulationRequestMap[$index])){
-			$this->logger->debug("Discarding population result for chunk x=$x,z=$z - promise was already broken");
+		if(!isset($this->activeChunkPopulationTasks[$index])){
+			throw new AssumptionFailedError("This should always be set, regardless of whether the task was orphaned or not");
+		}
+		if(!$this->activeChunkPopulationTasks[$index]){
+			$this->logger->debug("Discarding orphaned population result for chunk x=$x,z=$z");
 			unset($this->activeChunkPopulationTasks[$index]);
-		}elseif(isset($this->activeChunkPopulationTasks[$index])){
+		}else{
 			if($dirtyChunks === 0){
 				$oldChunk = $this->loadChunk($x, $z);
 				$this->setChunk($x, $z, $chunk);