From 5913d5038b6f55e618c76b855a8b49a27a62b212 Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <odigiman@gmail.com>
Date: Thu, 18 Apr 2019 17:23:48 +0100
Subject: [PATCH] Cleaned up Entity->close() handling

---
 src/pocketmine/Player.php        | 27 +++++++++----------
 src/pocketmine/entity/Entity.php | 46 +++++++++++++++++++++-----------
 src/pocketmine/entity/Human.php  | 22 +++++++--------
 src/pocketmine/entity/Living.php | 16 +++++------
 4 files changed, 61 insertions(+), 50 deletions(-)

diff --git a/src/pocketmine/Player.php b/src/pocketmine/Player.php
index 0121a1489..90194a3ff 100644
--- a/src/pocketmine/Player.php
+++ b/src/pocketmine/Player.php
@@ -2539,21 +2539,20 @@ class Player extends Human implements CommandSender, ChunkLoader, ChunkListener,
 		$this->flagForDespawn();
 	}
 
-	final public function close() : void{
-		if(!$this->closed){
-			$this->disconnect("Player destroyed");
-			$this->networkSession = null;
+	protected function onDispose() : void{
+		$this->disconnect("Player destroyed");
+		$this->cursorInventory->removeAllViewers(true);
+		$this->craftingGrid->removeAllViewers(true);
+		parent::onDispose();
+	}
 
-			$this->cursorInventory = null;
-			$this->craftingGrid = null;
-
-			$this->spawned = false;
-
-			$this->spawnPosition = null;
-			$this->perm = null;
-
-			parent::close();
-		}
+	protected function destroyCycles() : void{
+		$this->networkSession = null;
+		$this->cursorInventory = null;
+		$this->craftingGrid = null;
+		$this->spawnPosition = null;
+		$this->perm = null;
+		parent::destroyCycles();
 	}
 
 	public function __debugInfo(){
diff --git a/src/pocketmine/entity/Entity.php b/src/pocketmine/entity/Entity.php
index 126d2923d..1e33278a6 100644
--- a/src/pocketmine/entity/Entity.php
+++ b/src/pocketmine/entity/Entity.php
@@ -1897,28 +1897,42 @@ abstract class Entity extends Location implements Metadatable, EntityIds{
 	 *
 	 * WARNING: Entities are unusable after this has been executed!
 	 */
-	public function close() : void{
+	final public function close() : void{
 		if(!$this->closed){
-			(new EntityDespawnEvent($this))->call();
 			$this->closed = true;
+			(new EntityDespawnEvent($this))->call();
 
-			$this->despawnFromAll();
-			$this->hasSpawned = [];
-
-			if($this->chunk !== null){
-				$this->chunk->removeEntity($this);
-				$this->chunk = null;
-			}
-
-			if($this->isValid()){
-				$this->level->removeEntity($this);
-				$this->setLevel(null);
-			}
-
-			$this->lastDamageCause = null;
+			$this->onDispose();
+			$this->destroyCycles();
 		}
 	}
 
+	/**
+	 * Called when the entity is disposed to clean up things like viewers. This SHOULD NOT destroy internal state,
+	 * because it may be needed by descendent classes.
+	 */
+	protected function onDispose() : void{
+		$this->despawnFromAll();
+		if($this->chunk !== null){
+			$this->chunk->removeEntity($this);
+		}
+		if($this->isValid()){
+			$this->level->removeEntity($this);
+		}
+	}
+
+	/**
+	 * Called when the entity is disposed, after all events have been fired. This should be used to perform destructive
+	 * circular object references and things which could impact memory usage.
+	 *
+	 * It is expected that the object is unusable after this is called.
+	 */
+	protected function destroyCycles() : void{
+		$this->chunk = null;
+		$this->setLevel(null);
+		$this->lastDamageCause = null;
+	}
+
 	/**
 	 * @param int  $propertyId
 	 * @param int  $flagId
diff --git a/src/pocketmine/entity/Human.php b/src/pocketmine/entity/Human.php
index 5beeb8b66..443a1ffe4 100644
--- a/src/pocketmine/entity/Human.php
+++ b/src/pocketmine/entity/Human.php
@@ -883,18 +883,16 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{
 		}
 	}
 
-	public function close() : void{
-		if(!$this->closed){
-			if($this->inventory !== null){
-				$this->inventory->removeAllViewers(true);
-				$this->inventory = null;
-			}
-			if($this->enderChestInventory !== null){
-				$this->enderChestInventory->removeAllViewers(true);
-				$this->enderChestInventory = null;
-			}
-			parent::close();
-		}
+	protected function onDispose() : void{
+		$this->inventory->removeAllViewers(true);
+		$this->enderChestInventory->removeAllViewers(true);
+		parent::onDispose();
+	}
+
+	protected function destroyCycles() : void{
+		$this->inventory = null;
+		$this->enderChestInventory = null;
+		parent::destroyCycles();
 	}
 
 	/**
diff --git a/src/pocketmine/entity/Living.php b/src/pocketmine/entity/Living.php
index 97bec4f14..2b07024d9 100644
--- a/src/pocketmine/entity/Living.php
+++ b/src/pocketmine/entity/Living.php
@@ -926,13 +926,13 @@ abstract class Living extends Entity implements Damageable{
 		$this->armorInventory->sendContents($player);
 	}
 
-	public function close() : void{
-		if(!$this->closed){
-			if($this->armorInventory !== null){
-				$this->armorInventory->removeAllViewers(true);
-				$this->armorInventory = null;
-			}
-			parent::close();
-		}
+	protected function onDispose() : void{
+		$this->armorInventory->removeAllViewers(true);
+		parent::onDispose();
+	}
+
+	protected function destroyCycles() : void{
+		$this->armorInventory = null;
+		parent::destroyCycles();
 	}
 }