From 34bb86d2bf135dbe1d5ac412252c46d3608166e3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 1 Nov 2023 15:26:30 +0000 Subject: [PATCH 1/7] Bump phpstan/phpstan-strict-rules from 1.5.1 to 1.5.2 (#6125) Bumps [phpstan/phpstan-strict-rules](https://github.com/phpstan/phpstan-strict-rules) from 1.5.1 to 1.5.2. - [Release notes](https://github.com/phpstan/phpstan-strict-rules/releases) - [Commits](https://github.com/phpstan/phpstan-strict-rules/compare/1.5.1...1.5.2) --- updated-dependencies: - dependency-name: phpstan/phpstan-strict-rules dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- composer.lock | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/composer.lock b/composer.lock index cdb41d563..64a614166 100644 --- a/composer.lock +++ b/composer.lock @@ -1492,21 +1492,21 @@ }, { "name": "phpstan/phpstan-strict-rules", - "version": "1.5.1", + "version": "1.5.2", "source": { "type": "git", "url": "https://github.com/phpstan/phpstan-strict-rules.git", - "reference": "b21c03d4f6f3a446e4311155f4be9d65048218e6" + "reference": "7a50e9662ee9f3942e4aaaf3d603653f60282542" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpstan/phpstan-strict-rules/zipball/b21c03d4f6f3a446e4311155f4be9d65048218e6", - "reference": "b21c03d4f6f3a446e4311155f4be9d65048218e6", + "url": "https://api.github.com/repos/phpstan/phpstan-strict-rules/zipball/7a50e9662ee9f3942e4aaaf3d603653f60282542", + "reference": "7a50e9662ee9f3942e4aaaf3d603653f60282542", "shasum": "" }, "require": { "php": "^7.2 || ^8.0", - "phpstan/phpstan": "^1.10" + "phpstan/phpstan": "^1.10.34" }, "require-dev": { "nikic/php-parser": "^4.13.0", @@ -1535,9 +1535,9 @@ "description": "Extra strict and opinionated rules for PHPStan", "support": { "issues": "https://github.com/phpstan/phpstan-strict-rules/issues", - "source": "https://github.com/phpstan/phpstan-strict-rules/tree/1.5.1" + "source": "https://github.com/phpstan/phpstan-strict-rules/tree/1.5.2" }, - "time": "2023-03-29T14:47:40+00:00" + "time": "2023-10-30T14:35:06+00:00" }, { "name": "phpunit/php-code-coverage", @@ -2964,5 +2964,5 @@ "platform-overrides": { "php": "8.1.0" }, - "plugin-api-version": "2.6.0" + "plugin-api-version": "2.3.0" } From 14025710559ea4bdd22cf2c07e9dff66b40eb6b0 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 1 Nov 2023 15:26:46 +0000 Subject: [PATCH 2/7] Bump phpstan/phpstan from 1.10.39 to 1.10.40 (#6126) Bumps [phpstan/phpstan](https://github.com/phpstan/phpstan) from 1.10.39 to 1.10.40. - [Release notes](https://github.com/phpstan/phpstan/releases) - [Changelog](https://github.com/phpstan/phpstan/blob/1.11.x/CHANGELOG.md) - [Commits](https://github.com/phpstan/phpstan/compare/1.10.39...1.10.40) --- updated-dependencies: - dependency-name: phpstan/phpstan dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- composer.json | 2 +- composer.lock | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/composer.json b/composer.json index 1775f4e4d..2f0b68730 100644 --- a/composer.json +++ b/composer.json @@ -52,7 +52,7 @@ "symfony/filesystem": "~6.3.0" }, "require-dev": { - "phpstan/phpstan": "1.10.39", + "phpstan/phpstan": "1.10.40", "phpstan/phpstan-phpunit": "^1.1.0", "phpstan/phpstan-strict-rules": "^1.2.0", "phpunit/phpunit": "~10.3.0 || ~10.2.0 || ~10.1.0" diff --git a/composer.lock b/composer.lock index 64a614166..4c9588263 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "feefde772166966ee8065e613fe9a56e", + "content-hash": "ca499c3c5acafac837f7384ecdae136e", "packages": [ { "name": "adhocore/json-comment", @@ -1378,16 +1378,16 @@ }, { "name": "phpstan/phpstan", - "version": "1.10.39", + "version": "1.10.40", "source": { "type": "git", "url": "https://github.com/phpstan/phpstan.git", - "reference": "d9dedb0413f678b4d03cbc2279a48f91592c97c4" + "reference": "93c84b5bf7669920d823631e39904d69b9c7dc5d" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpstan/phpstan/zipball/d9dedb0413f678b4d03cbc2279a48f91592c97c4", - "reference": "d9dedb0413f678b4d03cbc2279a48f91592c97c4", + "url": "https://api.github.com/repos/phpstan/phpstan/zipball/93c84b5bf7669920d823631e39904d69b9c7dc5d", + "reference": "93c84b5bf7669920d823631e39904d69b9c7dc5d", "shasum": "" }, "require": { @@ -1436,7 +1436,7 @@ "type": "tidelift" } ], - "time": "2023-10-17T15:46:26+00:00" + "time": "2023-10-30T14:48:31+00:00" }, { "name": "phpstan/phpstan-phpunit", From 9eb2a46942c806ed1115517e5056653781595a68 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 1 Nov 2023 15:53:37 +0000 Subject: [PATCH 3/7] World: remove useless isChunkLoaded checks getChunkEntities() will return an empty array if the chunk isn't loaded anyway, so this is just wasting CPU cycles. --- src/world/World.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/world/World.php b/src/world/World.php index 1ecbe20a9..4ff930dd1 100644 --- a/src/world/World.php +++ b/src/world/World.php @@ -2292,9 +2292,6 @@ class World implements ChunkManager{ for($x = $minX; $x <= $maxX; ++$x){ for($z = $minZ; $z <= $maxZ; ++$z){ - if(!$this->isChunkLoaded($x, $z)){ - continue; - } foreach($this->getChunkEntities($x, $z) as $ent){ if($ent !== $entity && $ent->boundingBox->intersectsWith($bb)){ $nearby[] = $ent; @@ -2335,9 +2332,6 @@ class World implements ChunkManager{ for($x = $minX; $x <= $maxX; ++$x){ for($z = $minZ; $z <= $maxZ; ++$z){ - if(!$this->isChunkLoaded($x, $z)){ - continue; - } foreach($this->getChunkEntities($x, $z) as $entity){ if(!($entity instanceof $entityType) || $entity->isFlaggedForDespawn() || (!$includeDead && !$entity->isAlive())){ continue; From 0093732d498273d49fe68f9377c3c173f2cae186 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 1 Nov 2023 16:13:28 +0000 Subject: [PATCH 4/7] PermissionManager: fixed non-reentrant-safe permission unsubscribing during unset(), the destructors for other objects with cyclic references can get triggered, resulting in the functions being reentered before the count() call. This leads to a crash because the offset no longer exists. Instead, we check if only the given PermissibleInternal is present, and clean everything up with a single unset instead of two. This could also have been solved by adding extra isset() checks before checking the counts, but this way seemed more elegant. This is similar to an issue with AsyncTask thread-local storage a few months ago, which was also caused by GC reentrancy. closes #6119 --- src/permission/PermissionManager.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/permission/PermissionManager.php b/src/permission/PermissionManager.php index 2d8324887..1291ba86b 100644 --- a/src/permission/PermissionManager.php +++ b/src/permission/PermissionManager.php @@ -72,19 +72,21 @@ class PermissionManager{ } public function unsubscribeFromPermission(string $permission, PermissibleInternal $permissible) : void{ - if(isset($this->permSubs[$permission])){ - unset($this->permSubs[$permission][spl_object_id($permissible)]); - if(count($this->permSubs[$permission]) === 0){ + if(isset($this->permSubs[$permission][spl_object_id($permissible)])){ + if(count($this->permSubs[$permission]) === 1){ unset($this->permSubs[$permission]); + }else{ + unset($this->permSubs[$permission][spl_object_id($permissible)]); } } } public function unsubscribeFromAllPermissions(PermissibleInternal $permissible) : void{ foreach($this->permSubs as $permission => $subs){ - unset($this->permSubs[$permission][spl_object_id($permissible)]); - if(count($this->permSubs[$permission]) === 0){ + if(count($subs) === 1 && isset($subs[spl_object_id($permissible)])){ unset($this->permSubs[$permission]); + }else{ + unset($this->permSubs[$permission][spl_object_id($permissible)]); } } } From e6e2c54ec95b3382b85665ac14a906ecf0873184 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 1 Nov 2023 16:28:59 +0000 Subject: [PATCH 5/7] Fixed various reentrant-unsafe 2D array element unsets (similar to previous commit) this pattern was used in various places --- src/Server.php | 5 +++-- src/plugin/PluginManager.php | 9 ++++++--- src/world/World.php | 27 ++++++++++++++++----------- 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/Server.php b/src/Server.php index 9adc8e794..dad61f48c 100644 --- a/src/Server.php +++ b/src/Server.php @@ -1259,9 +1259,10 @@ class Server{ */ public function unsubscribeFromBroadcastChannel(string $channelId, CommandSender $subscriber) : void{ if(isset($this->broadcastSubscribers[$channelId][spl_object_id($subscriber)])){ - unset($this->broadcastSubscribers[$channelId][spl_object_id($subscriber)]); - if(count($this->broadcastSubscribers[$channelId]) === 0){ + if(count($this->broadcastSubscribers[$channelId]) === 1){ unset($this->broadcastSubscribers[$channelId]); + }else{ + unset($this->broadcastSubscribers[$channelId][spl_object_id($subscriber)]); } } } diff --git a/src/plugin/PluginManager.php b/src/plugin/PluginManager.php index 67ca8cc37..301c66854 100644 --- a/src/plugin/PluginManager.php +++ b/src/plugin/PluginManager.php @@ -510,9 +510,12 @@ class PluginManager{ unset($this->enabledPlugins[$plugin->getDescription()->getName()]); foreach(Utils::stringifyKeys($this->pluginDependents) as $dependency => $dependentList){ - unset($this->pluginDependents[$dependency][$plugin->getDescription()->getName()]); - if(count($this->pluginDependents[$dependency]) === 0){ - unset($this->pluginDependents[$dependency]); + if(isset($this->pluginDependents[$dependency][$plugin->getDescription()->getName()])){ + if(count($this->pluginDependents[$dependency]) === 1){ + unset($this->pluginDependents[$dependency]); + }else{ + unset($this->pluginDependents[$dependency][$plugin->getDescription()->getName()]); + } } } diff --git a/src/world/World.php b/src/world/World.php index 8265aa384..e705124f3 100644 --- a/src/world/World.php +++ b/src/world/World.php @@ -826,14 +826,15 @@ class World implements ChunkManager{ $chunkHash = World::chunkHash($chunkX, $chunkZ); $loaderId = spl_object_id($loader); if(isset($this->chunkLoaders[$chunkHash][$loaderId])){ - unset($this->chunkLoaders[$chunkHash][$loaderId]); - if(count($this->chunkLoaders[$chunkHash]) === 0){ + if(count($this->chunkLoaders[$chunkHash]) === 1){ unset($this->chunkLoaders[$chunkHash]); $this->unloadChunkRequest($chunkX, $chunkZ, true); if(isset($this->chunkPopulationRequestMap[$chunkHash]) && !isset($this->activeChunkPopulationTasks[$chunkHash])){ $this->chunkPopulationRequestMap[$chunkHash]->reject(); unset($this->chunkPopulationRequestMap[$chunkHash]); } + }else{ + unset($this->chunkLoaders[$chunkHash][$loaderId]); } } } @@ -861,11 +862,12 @@ class World implements ChunkManager{ public function unregisterChunkListener(ChunkListener $listener, int $chunkX, int $chunkZ) : void{ $hash = World::chunkHash($chunkX, $chunkZ); if(isset($this->chunkListeners[$hash])){ - unset($this->chunkListeners[$hash][spl_object_id($listener)]); - unset($this->playerChunkListeners[$hash][spl_object_id($listener)]); - if(count($this->chunkListeners[$hash]) === 0){ + if(count($this->chunkListeners[$hash]) === 1){ unset($this->chunkListeners[$hash]); unset($this->playerChunkListeners[$hash]); + }else{ + unset($this->chunkListeners[$hash][spl_object_id($listener)]); + unset($this->playerChunkListeners[$hash][spl_object_id($listener)]); } } } @@ -1224,13 +1226,14 @@ class World implements ChunkManager{ $chunkHash = World::chunkHash($chunkX, $chunkZ); $tickerId = spl_object_id($ticker); if(isset($this->registeredTickingChunks[$chunkHash][$tickerId])){ - unset($this->registeredTickingChunks[$chunkHash][$tickerId]); - if(count($this->registeredTickingChunks[$chunkHash]) === 0){ + if(count($this->registeredTickingChunks[$chunkHash]) === 1){ unset( $this->registeredTickingChunks[$chunkHash], $this->recheckTickingChunks[$chunkHash], $this->validTickingChunks[$chunkHash] ); + }else{ + unset($this->registeredTickingChunks[$chunkHash][$tickerId]); } } } @@ -2645,9 +2648,10 @@ class World implements ChunkManager{ $pos = $this->entityLastKnownPositions[$entity->getId()]; $chunkHash = World::chunkHash($pos->getFloorX() >> Chunk::COORD_BIT_SIZE, $pos->getFloorZ() >> Chunk::COORD_BIT_SIZE); if(isset($this->entitiesByChunk[$chunkHash][$entity->getId()])){ - unset($this->entitiesByChunk[$chunkHash][$entity->getId()]); - if(count($this->entitiesByChunk[$chunkHash]) === 0){ + if(count($this->entitiesByChunk[$chunkHash]) === 1){ unset($this->entitiesByChunk[$chunkHash]); + }else{ + unset($this->entitiesByChunk[$chunkHash][$entity->getId()]); } } unset($this->entityLastKnownPositions[$entity->getId()]); @@ -2680,9 +2684,10 @@ class World implements ChunkManager{ if($oldChunkX !== $newChunkX || $oldChunkZ !== $newChunkZ){ $oldChunkHash = World::chunkHash($oldChunkX, $oldChunkZ); if(isset($this->entitiesByChunk[$oldChunkHash][$entity->getId()])){ - unset($this->entitiesByChunk[$oldChunkHash][$entity->getId()]); - if(count($this->entitiesByChunk[$oldChunkHash]) === 0){ + if(count($this->entitiesByChunk[$oldChunkHash]) === 1){ unset($this->entitiesByChunk[$oldChunkHash]); + }else{ + unset($this->entitiesByChunk[$oldChunkHash][$entity->getId()]); } } From 2c17f82eb89adcbcaf58dcaf122dc62b73bb9f9a Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 1 Nov 2023 16:37:46 +0000 Subject: [PATCH 6/7] Release 5.7.1 --- changelogs/5.7.md | 7 +++++++ src/VersionInfo.php | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/changelogs/5.7.md b/changelogs/5.7.md index 65830c220..9134f299e 100644 --- a/changelogs/5.7.md +++ b/changelogs/5.7.md @@ -18,3 +18,10 @@ Consider using the `mcpe-protocol` directive in `plugin.yml` as a constraint if ## Fixes - Fixed `cartography_table`, `smithing_table`, `stripped_cherry_log` and `stripped_cherry_wood` not working in `StringToItemParser`. - Fixed `Promise::onCompletion()` always calling the reject handler if the promise was already completed. + +# 5.7.1 +Released 1st November 2023. + +## Fixes +- Fixed non-reentrant-safe code in `PermissionManager` and various other subscriber subsystems. + - These issues caused server crashes when deleting a subscriber indirectly triggered the deletion of other subscribers (e.g. due to the GC activating in `unset()`). diff --git a/src/VersionInfo.php b/src/VersionInfo.php index e6cae5059..2033dc9da 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -32,7 +32,7 @@ use function str_repeat; final class VersionInfo{ public const NAME = "PocketMine-MP"; public const BASE_VERSION = "5.7.1"; - public const IS_DEVELOPMENT_BUILD = true; + public const IS_DEVELOPMENT_BUILD = false; public const BUILD_CHANNEL = "stable"; /** From 55f3477ed99c55c0c1582dcfbd298cb1cd90f9c5 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Wed, 1 Nov 2023 16:37:46 +0000 Subject: [PATCH 7/7] 5.7.2 is next --- src/VersionInfo.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/VersionInfo.php b/src/VersionInfo.php index 2033dc9da..365411cc2 100644 --- a/src/VersionInfo.php +++ b/src/VersionInfo.php @@ -31,8 +31,8 @@ use function str_repeat; final class VersionInfo{ public const NAME = "PocketMine-MP"; - public const BASE_VERSION = "5.7.1"; - public const IS_DEVELOPMENT_BUILD = false; + public const BASE_VERSION = "5.7.2"; + public const IS_DEVELOPMENT_BUILD = true; public const BUILD_CHANNEL = "stable"; /**