From ed8569a3f4da1c457442db873eff85402a2a5e7a Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <odigiman@gmail.com>
Date: Mon, 5 Nov 2018 17:26:22 +0000
Subject: [PATCH] Remove Collectable class, fix memory leak on AsyncTask fatal
 error

---
 src/pocketmine/Collectable.php         | 37 --------------------------
 src/pocketmine/scheduler/AsyncPool.php |  2 +-
 src/pocketmine/scheduler/AsyncTask.php | 24 ++++++++++++-----
 3 files changed, 18 insertions(+), 45 deletions(-)
 delete mode 100644 src/pocketmine/Collectable.php

diff --git a/src/pocketmine/Collectable.php b/src/pocketmine/Collectable.php
deleted file mode 100644
index 4dde44cb5..000000000
--- a/src/pocketmine/Collectable.php
+++ /dev/null
@@ -1,37 +0,0 @@
-<?php
-
-/*
- *
- *  ____            _        _   __  __ _                  __  __ ____
- * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
- * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
- * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
- * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Lesser General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * @author PocketMine Team
- * @link http://www.pocketmine.net/
- *
- *
-*/
-
-declare(strict_types=1);
-
-namespace pocketmine;
-
-abstract class Collectable extends \Threaded{
-
-	private $isGarbage = false;
-
-	public function isGarbage() : bool{
-		return $this->isGarbage;
-	}
-
-	public function setGarbage(){
-		$this->isGarbage = true;
-	}
-}
diff --git a/src/pocketmine/scheduler/AsyncPool.php b/src/pocketmine/scheduler/AsyncPool.php
index cd8df7ea1..efbb0b5b0 100644
--- a/src/pocketmine/scheduler/AsyncPool.php
+++ b/src/pocketmine/scheduler/AsyncPool.php
@@ -219,7 +219,7 @@ class AsyncPool{
 				/** @var AsyncTask $task */
 				$task = $queue->bottom();
 				$task->checkProgressUpdates();
-				if(!$task->isRunning() and $task->isGarbage()){ //make sure the task actually executed before trying to collect
+				if($task->isFinished()){ //make sure the task actually executed before trying to collect
 					$doGC = true;
 					$queue->dequeue();
 
diff --git a/src/pocketmine/scheduler/AsyncTask.php b/src/pocketmine/scheduler/AsyncTask.php
index b6e927b6d..381207493 100644
--- a/src/pocketmine/scheduler/AsyncTask.php
+++ b/src/pocketmine/scheduler/AsyncTask.php
@@ -23,8 +23,6 @@ declare(strict_types=1);
 
 namespace pocketmine\scheduler;
 
-use pocketmine\Collectable;
-
 /**
  * Class used to run async tasks in other threads.
  *
@@ -42,7 +40,7 @@ use pocketmine\Collectable;
  *
  * WARNING: Do not call PocketMine-MP API methods from other Threads!!
  */
-abstract class AsyncTask extends Collectable{
+abstract class AsyncTask extends \Threaded{
 	/**
 	 * @var \ArrayObject|mixed[] object hash => mixed data
 	 *
@@ -63,6 +61,8 @@ abstract class AsyncTask extends Collectable{
 	private $submitted = false;
 
 	private $crashed = false;
+	/** @var bool */
+	private $finished = false;
 
 	public function run() : void{
 		$this->result = null;
@@ -76,13 +76,23 @@ abstract class AsyncTask extends Collectable{
 			}
 		}
 
-		$this->setGarbage();
+		$this->finished = true;
 	}
 
 	public function isCrashed() : bool{
 		return $this->crashed or $this->isTerminated();
 	}
 
+	/**
+	 * Returns whether this task has finished executing, whether successfully or not. This differs from isRunning()
+	 * because it is not true prior to task execution.
+	 *
+	 * @return bool
+	 */
+	public function isFinished() : bool{
+		return $this->finished or $this->isCrashed();
+	}
+
 	/**
 	 * @return mixed
 	 */
@@ -131,7 +141,7 @@ abstract class AsyncTask extends Collectable{
 	 * @return mixed
 	 */
 	public function getFromThreadStore(string $identifier){
-		if($this->worker === null or $this->isGarbage()){
+		if($this->worker === null or $this->isFinished()){
 			throw new \BadMethodCallException("Objects stored in AsyncWorker thread-local storage can only be retrieved during task execution");
 		}
 		return $this->worker->getFromThreadStore($identifier);
@@ -144,7 +154,7 @@ abstract class AsyncTask extends Collectable{
 	 * @param mixed  $value
 	 */
 	public function saveToThreadStore(string $identifier, $value) : void{
-		if($this->worker === null or $this->isGarbage()){
+		if($this->worker === null or $this->isFinished()){
 			throw new \BadMethodCallException("Objects can only be added to AsyncWorker thread-local storage during task execution");
 		}
 		$this->worker->saveToThreadStore($identifier, $value);
@@ -156,7 +166,7 @@ abstract class AsyncTask extends Collectable{
 	 * @param string $identifier
 	 */
 	public function removeFromThreadStore(string $identifier) : void{
-		if($this->worker === null or $this->isGarbage()){
+		if($this->worker === null or $this->isFinished()){
 			throw new \BadMethodCallException("Objects can only be removed from AsyncWorker thread-local storage during task execution");
 		}
 		$this->worker->removeFromThreadStore($identifier);