From cde2c10c1d66e19f6b1fae4109dcba1579e156ee Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sat, 27 Jul 2019 15:09:42 +0100 Subject: [PATCH 1/4] AsyncTask: partial backport of 6ac0c517f54dc83fc23f41a6797c14e1f4b457a1 (simplify TLS) - deprecated AsyncTask::peekLocal() - AsyncTask::fetchLocal() no longer deletes stored data --- src/pocketmine/scheduler/AsyncTask.php | 69 ++++++++------------------ 1 file changed, 21 insertions(+), 48 deletions(-) diff --git a/src/pocketmine/scheduler/AsyncTask.php b/src/pocketmine/scheduler/AsyncTask.php index c14fd09e0..0e3a4d991 100644 --- a/src/pocketmine/scheduler/AsyncTask.php +++ b/src/pocketmine/scheduler/AsyncTask.php @@ -51,7 +51,7 @@ abstract class AsyncTask extends Collectable{ * @var \SplObjectStorage|null * Used to store objects on the main thread which should not be serialized. */ - private static $localObjectStorage; + private static $threadLocalStorage; /** @var AsyncWorker $worker */ public $worker = null; @@ -229,9 +229,6 @@ abstract class AsyncTask extends Collectable{ * * Scalar types can be stored directly in class properties instead of using this storage. * - * Objects stored in this storage MUST be retrieved through {@link AsyncTask::fetchLocal} when {@link AsyncTask::onCompletion} is called. - * Otherwise, a NOTICE level message will be raised and the reference will be removed after onCompletion exits. - * * WARNING: THIS METHOD SHOULD ONLY BE CALLED FROM THE MAIN THREAD! * * @param mixed $complexData the data to store @@ -243,26 +240,19 @@ abstract class AsyncTask extends Collectable{ throw new \BadMethodCallException("Objects can only be stored from the parent thread"); } - if(self::$localObjectStorage === null){ - self::$localObjectStorage = new \SplObjectStorage(); //lazy init + if(self::$threadLocalStorage === null){ + self::$threadLocalStorage = new \SplObjectStorage(); //lazy init } - if(isset(self::$localObjectStorage[$this])){ + if(isset(self::$threadLocalStorage[$this])){ throw new \InvalidStateException("Already storing complex data for this async task"); } - self::$localObjectStorage[$this] = $complexData; + self::$threadLocalStorage[$this] = $complexData; } /** - * Returns and removes mixed data in thread-local storage on the parent thread. Call this method from - * {@link AsyncTask::onCompletion} to fetch the data stored in the object store, if any. - * - * If no data was stored in the local store, or if the data was already retrieved by a previous call to fetchLocal, - * do NOT call this method, or an exception will be thrown. - * - * Do not call this method from {@link AsyncTask::onProgressUpdate}, because this method deletes stored data, which - * means that you will not be able to retrieve it again afterwards. Use {@link AsyncTask::peekLocal} instead to - * retrieve stored data without removing it from the store. + * Returns data previously stored in thread-local storage on the parent thread. Use this during progress updates or + * task completion to retrieve data you stored using {@link AsyncTask::storeLocal}. * * WARNING: THIS METHOD SHOULD ONLY BE CALLED FROM THE MAIN THREAD! * @@ -272,25 +262,20 @@ abstract class AsyncTask extends Collectable{ * @throws \BadMethodCallException if called from any thread except the main thread */ protected function fetchLocal(){ - try{ - return $this->peekLocal(); - }finally{ - if(self::$localObjectStorage !== null){ - unset(self::$localObjectStorage[$this]); - } + if($this->worker !== null and $this->worker === \Thread::getCurrentThread()){ + throw new \BadMethodCallException("Objects can only be retrieved from the parent thread"); } + + if(self::$threadLocalStorage === null or !isset(self::$threadLocalStorage[$this])){ + throw new \InvalidStateException("No complex data stored for this async task"); + } + + return self::$threadLocalStorage[$this]; } /** - * Returns mixed data in thread-local storage on the parent thread **without clearing** it. Call this method from - * {@link AsyncTask::onProgressUpdate} to fetch the data stored if you need to be able to access the data later on, - * such as in another progress update. - * - * Use {@link AsyncTask::fetchLocal} instead from {@link AsyncTask::onCompletion}, because this method does not delete - * the data, and not clearing the data will result in a warning for memory leak after {@link AsyncTask::onCompletion} - * finished executing. - * - * WARNING: THIS METHOD SHOULD ONLY BE CALLED FROM THE MAIN THREAD! + * @deprecated + * @see AsyncTask::fetchLocal() * * @return mixed * @@ -298,27 +283,15 @@ abstract class AsyncTask extends Collectable{ * @throws \BadMethodCallException if called from any thread except the main thread */ protected function peekLocal(){ - if($this->worker !== null and $this->worker === \Thread::getCurrentThread()){ - throw new \BadMethodCallException("Objects can only be retrieved from the parent thread"); - } - - if(self::$localObjectStorage === null or !isset(self::$localObjectStorage[$this])){ - throw new \InvalidStateException("No complex data stored for this async task"); - } - - return self::$localObjectStorage[$this]; + return $this->fetchLocal(); } /** * @internal Called by the AsyncPool to destroy any leftover stored objects that this task failed to retrieve. - * @return bool */ - public function removeDanglingStoredObjects() : bool{ - if(self::$localObjectStorage !== null and isset(self::$localObjectStorage[$this])){ - unset(self::$localObjectStorage[$this]); - return true; + public function removeDanglingStoredObjects() : void{ + if(self::$threadLocalStorage !== null and isset(self::$threadLocalStorage[$this])){ + unset(self::$threadLocalStorage[$this]); } - - return false; } } From 2d3562c68763d1bfe3dc765728b22a64fb551437 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 29 Jul 2019 17:17:05 +0100 Subject: [PATCH 2/4] World: fixed scheduled updates causing chunk loading this probably needs to be backported. --- src/pocketmine/level/Level.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/pocketmine/level/Level.php b/src/pocketmine/level/Level.php index 754312852..9736c142c 100644 --- a/src/pocketmine/level/Level.php +++ b/src/pocketmine/level/Level.php @@ -820,7 +820,12 @@ class Level implements ChunkManager, Metadatable{ //Delayed updates while($this->scheduledBlockUpdateQueue->count() > 0 and $this->scheduledBlockUpdateQueue->current()["priority"] <= $currentTick){ - $block = $this->getBlock($this->scheduledBlockUpdateQueue->extract()["data"]); + /** @var Vector3 $vec */ + $vec = $this->scheduledBlockUpdateQueue->extract()["data"]; + if(!$this->isInLoadedTerrain($vec)){ + continue; + } + $block = $this->getBlock($vec); unset($this->scheduledBlockUpdateQueueIndex[Level::blockHash($block->x, $block->y, $block->z)]); $block->onScheduledUpdate(); } From 18a1bfe4dd425895a14c82d6633fbf567feeb30c Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 29 Jul 2019 17:27:21 +0100 Subject: [PATCH 3/4] Release 3.9.3 --- changelogs/3.9.md | 5 +++++ src/pocketmine/VersionInfo.php | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/changelogs/3.9.md b/changelogs/3.9.md index 2f35ab897..9e3e86b75 100644 --- a/changelogs/3.9.md +++ b/changelogs/3.9.md @@ -52,3 +52,8 @@ Plugin developers should **only** update their required API to this version if y - Fixed `Item->setCustomName()` with an empty string leaving behind an empty tag. - Fixed incorrect positioning of bucket empty sound. - Fixed some incorrect tag parsing in `/give` involving quoted numbers. + +# 3.9.3 +- Fixed a memory leak on async task removal in error conditions. +- Fixed scheduled block updates (for example liquid) triggering chunk reloading. This could cause a significant performance issue in some conditions. +- Fixed some minor cosmetic issues in documentation. diff --git a/src/pocketmine/VersionInfo.php b/src/pocketmine/VersionInfo.php index d7ae20990..403624794 100644 --- a/src/pocketmine/VersionInfo.php +++ b/src/pocketmine/VersionInfo.php @@ -23,5 +23,5 @@ namespace pocketmine; const NAME = "PocketMine-MP"; const BASE_VERSION = "3.9.3"; -const IS_DEVELOPMENT_BUILD = true; +const IS_DEVELOPMENT_BUILD = false; const BUILD_NUMBER = 0; From 0ea9a08963940517f338c8cffde7e3ccd21cc4f3 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 29 Jul 2019 17:27:21 +0100 Subject: [PATCH 4/4] 3.9.4 is next --- src/pocketmine/VersionInfo.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pocketmine/VersionInfo.php b/src/pocketmine/VersionInfo.php index 403624794..25617f9eb 100644 --- a/src/pocketmine/VersionInfo.php +++ b/src/pocketmine/VersionInfo.php @@ -22,6 +22,6 @@ namespace pocketmine; const NAME = "PocketMine-MP"; -const BASE_VERSION = "3.9.3"; -const IS_DEVELOPMENT_BUILD = false; +const BASE_VERSION = "3.9.4"; +const IS_DEVELOPMENT_BUILD = true; const BUILD_NUMBER = 0;