From ae75d73f4894fb792649e7a6af1a63376d162578 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Thu, 4 Feb 2021 16:28:49 +0000 Subject: [PATCH] Extract MainLoggerThread unit from MainLogger MainLogger is no longer a Thread, as per the recent changes to pocketmine/log-pthreads. --- composer.lock | 12 +-- phpstan.neon.dist | 1 + src/PocketMine.php | 3 +- src/entity/EntitySizeInfo.php | 2 + src/item/Banner.php | 1 + .../protocol/ClientboundMapItemDataPacket.php | 4 +- src/utils/EnumTrait.php | 1 - src/utils/MainLogger.php | 73 +++---------- src/utils/MainLoggerThread.php | 101 ++++++++++++++++++ tests/phpstan/bootstrap.php | 8 +- .../check-explicit-mixed-baseline.neon | 2 +- .../block/regenerate_consistency_check.php | 8 +- tests/phpunit/scheduler/AsyncPoolTest.php | 3 +- 13 files changed, 140 insertions(+), 79 deletions(-) create mode 100644 src/utils/MainLoggerThread.php diff --git a/composer.lock b/composer.lock index 1b56339b8..a778a5579 100644 --- a/composer.lock +++ b/composer.lock @@ -513,17 +513,17 @@ "source": { "type": "git", "url": "https://github.com/pmmp/LogPthreads.git", - "reference": "273e4ba9715c7eb916c6e50e0b92ac43b7ac389e" + "reference": "634af620e1cb3b4f3c7ed356cc31d778b00fd717" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/pmmp/LogPthreads/zipball/273e4ba9715c7eb916c6e50e0b92ac43b7ac389e", - "reference": "273e4ba9715c7eb916c6e50e0b92ac43b7ac389e", + "url": "https://api.github.com/repos/pmmp/LogPthreads/zipball/634af620e1cb3b4f3c7ed356cc31d778b00fd717", + "reference": "634af620e1cb3b4f3c7ed356cc31d778b00fd717", "shasum": "" }, "require": { "ext-pthreads": "~3.2.0", - "php": ">=7.2", + "php": "^7.2 || ^8.0", "pocketmine/log": "^0.2.0 || dev-master" }, "conflict": { @@ -531,7 +531,7 @@ }, "require-dev": { "phpstan/extension-installer": "^1.0", - "phpstan/phpstan": "0.12.63", + "phpstan/phpstan": "0.12.71", "phpstan/phpstan-strict-rules": "^0.12.4" }, "type": "library", @@ -549,7 +549,7 @@ "issues": "https://github.com/pmmp/LogPthreads/issues", "source": "https://github.com/pmmp/LogPthreads/tree/master" }, - "time": "2021-01-06T21:23:23+00:00" + "time": "2021-02-04T15:41:29+00:00" }, { "name": "pocketmine/math", diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 1a61846ff..6ac4a2b95 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -18,6 +18,7 @@ includes: rules: - pocketmine\phpstan\rules\DisallowEnumComparisonRule +# - pocketmine\phpstan\rules\ThreadedSupportedTypesRule parameters: level: 8 diff --git a/src/PocketMine.php b/src/PocketMine.php index 15ec49cdd..22357648e 100644 --- a/src/PocketMine.php +++ b/src/PocketMine.php @@ -272,8 +272,7 @@ namespace pocketmine { } }while(false); - $logger->shutdown(); - $logger->join(); + $logger->shutdownLogWriterThread(); echo Terminal::$FORMAT_RESET . PHP_EOL; diff --git a/src/entity/EntitySizeInfo.php b/src/entity/EntitySizeInfo.php index 1456cc2fb..91cf7c5f7 100644 --- a/src/entity/EntitySizeInfo.php +++ b/src/entity/EntitySizeInfo.php @@ -23,6 +23,8 @@ declare(strict_types=1); namespace pocketmine\entity; +use function min; + final class EntitySizeInfo{ /** @var float */ private $height; diff --git a/src/item/Banner.php b/src/item/Banner.php index 744c6fb89..d3c440bee 100644 --- a/src/item/Banner.php +++ b/src/item/Banner.php @@ -30,6 +30,7 @@ use pocketmine\block\utils\DyeColor; use pocketmine\data\bedrock\DyeColorIdMap; use pocketmine\nbt\tag\CompoundTag; use pocketmine\nbt\tag\ListTag; +use function count; class Banner extends ItemBlockWallOrFloor{ public const TAG_PATTERNS = TileBanner::TAG_PATTERNS; diff --git a/src/network/mcpe/protocol/ClientboundMapItemDataPacket.php b/src/network/mcpe/protocol/ClientboundMapItemDataPacket.php index d92077cf1..7c0f5a37a 100644 --- a/src/network/mcpe/protocol/ClientboundMapItemDataPacket.php +++ b/src/network/mcpe/protocol/ClientboundMapItemDataPacket.php @@ -30,9 +30,9 @@ use pocketmine\network\mcpe\protocol\serializer\PacketSerializer; use pocketmine\network\mcpe\protocol\types\DimensionIds; use pocketmine\network\mcpe\protocol\types\MapDecoration; use pocketmine\network\mcpe\protocol\types\MapTrackedObject; -use function count; -#ifndef COMPILE use pocketmine\utils\Binary; +#ifndef COMPILE +use function count; #endif class ClientboundMapItemDataPacket extends DataPacket implements ClientboundPacket{ diff --git a/src/utils/EnumTrait.php b/src/utils/EnumTrait.php index dea079738..163c0b0b2 100644 --- a/src/utils/EnumTrait.php +++ b/src/utils/EnumTrait.php @@ -23,7 +23,6 @@ declare(strict_types=1); namespace pocketmine\utils; -use function getmypid; use function preg_match; trait EnumTrait{ diff --git a/src/utils/MainLogger.php b/src/utils/MainLogger.php index 3757d65b8..886c87677 100644 --- a/src/utils/MainLogger.php +++ b/src/utils/MainLogger.php @@ -27,11 +27,7 @@ use LogLevel; use pocketmine\errorhandler\ErrorTypeToStringMap; use pocketmine\thread\Thread; use pocketmine\thread\Worker; -use function fclose; -use function fopen; -use function fwrite; use function get_class; -use function is_resource; use function preg_replace; use function sprintf; use function touch; @@ -43,14 +39,8 @@ class MainLogger extends \AttachableThreadedLogger implements \BufferedLogger{ /** @var string */ protected $logFile; - /** @var \Threaded */ - protected $logStream; - /** @var bool */ - protected $shutdown = false; /** @var bool */ protected $logDebug; - /** @var bool */ - private $syncFlush = false; /** @var string */ private $format = TextFormat::AQUA . "[%s] " . TextFormat::RESET . "%s[%s/%s]: %s" . TextFormat::RESET; @@ -61,6 +51,9 @@ class MainLogger extends \AttachableThreadedLogger implements \BufferedLogger{ /** @var string */ private $timezone; + /** @var MainLoggerThread */ + private $logWriterThread; + /** * @throws \RuntimeException */ @@ -69,13 +62,13 @@ class MainLogger extends \AttachableThreadedLogger implements \BufferedLogger{ touch($logFile); $this->logFile = $logFile; $this->logDebug = $logDebug; - $this->logStream = new \Threaded; //Child threads may not inherit command line arguments, so if there's an override it needs to be recorded here $this->mainThreadHasFormattingCodes = Terminal::hasFormattingCodes(); $this->timezone = Timezone::get(); - $this->start(PTHREADS_INHERIT_NONE); + $this->logWriterThread = new MainLoggerThread($this->logFile); + $this->logWriterThread->start(PTHREADS_INHERIT_NONE); } /** @@ -218,9 +211,12 @@ class MainLogger extends \AttachableThreadedLogger implements \BufferedLogger{ $this->synchronized($c); } - public function shutdown() : void{ - $this->shutdown = true; - $this->notify(); + public function shutdownLogWriterThread() : void{ + if(\Thread::getCurrentThreadId() === $this->logWriterThread->getCreatorId()){ + $this->logWriterThread->shutdown(); + }else{ + throw new \LogicException("Only the creator thread can shutdown the logger thread"); + } } /** @@ -254,54 +250,17 @@ class MainLogger extends \AttachableThreadedLogger implements \BufferedLogger{ $attachment->call($level, $message); } - $this->logStream[] = $time->format("Y-m-d") . " " . TextFormat::clean($message) . PHP_EOL; - $this->notify(); + $this->logWriterThread->write($time->format("Y-m-d") . " " . TextFormat::clean($message) . PHP_EOL); }); } public function syncFlushBuffer() : void{ - $this->syncFlush = true; - $this->synchronized(function() : void{ - $this->notify(); //write immediately - - while($this->syncFlush){ - $this->wait(); //block until it's all been written to disk - } - }); + $this->logWriterThread->syncFlushBuffer(); } - /** - * @param resource $logResource - */ - private function writeLogStream($logResource) : void{ - while($this->logStream->count() > 0){ - $chunk = $this->logStream->shift(); - fwrite($logResource, $chunk); + public function __destruct(){ + if(!$this->logWriterThread->isJoined() && \Thread::getCurrentThreadId() === $this->logWriterThread->getCreatorId()){ + $this->shutdownLogWriterThread(); } - - $this->synchronized(function() : void{ - if($this->syncFlush){ - $this->syncFlush = false; - $this->notify(); //if this was due to a sync flush, tell the caller to stop waiting - } - }); - } - - public function run() : void{ - $logResource = fopen($this->logFile, "ab"); - if(!is_resource($logResource)){ - throw new \RuntimeException("Couldn't open log file"); - } - - while(!$this->shutdown){ - $this->writeLogStream($logResource); - $this->synchronized(function() : void{ - $this->wait(); - }); - } - - $this->writeLogStream($logResource); - - fclose($logResource); } } diff --git a/src/utils/MainLoggerThread.php b/src/utils/MainLoggerThread.php new file mode 100644 index 000000000..49e4f3e13 --- /dev/null +++ b/src/utils/MainLoggerThread.php @@ -0,0 +1,101 @@ +buffer = new \Threaded(); + $this->logFile = $logFile; + } + + public function write(string $line) : void{ + $this->synchronized(function() use ($line) : void{ + $this->buffer[] = $line; + $this->notify(); + }); + } + + public function syncFlushBuffer() : void{ + $this->syncFlush = true; + $this->synchronized(function() : void{ + $this->notify(); //write immediately + + while($this->syncFlush){ + $this->wait(); //block until it's all been written to disk + } + }); + } + + public function shutdown() : void{ + $this->shutdown = true; + $this->notify(); + $this->join(); + } + + /** + * @param resource $logResource + */ + private function writeLogStream($logResource) : void{ + while($this->buffer->count() > 0){ + $chunk = $this->buffer->shift(); + fwrite($logResource, $chunk); + } + + $this->synchronized(function() : void{ + if($this->syncFlush){ + $this->syncFlush = false; + $this->notify(); //if this was due to a sync flush, tell the caller to stop waiting + } + }); + } + + public function run() : void{ + $logResource = fopen($this->logFile, "ab"); + if(!is_resource($logResource)){ + throw new \RuntimeException("Couldn't open log file"); + } + + while(!$this->shutdown){ + $this->writeLogStream($logResource); + $this->synchronized(function() : void{ + $this->wait(); + }); + } + + $this->writeLogStream($logResource); + + fclose($logResource); + } +} diff --git a/tests/phpstan/bootstrap.php b/tests/phpstan/bootstrap.php index ba8ccd3fc..d6cbdd166 100644 --- a/tests/phpstan/bootstrap.php +++ b/tests/phpstan/bootstrap.php @@ -21,13 +21,13 @@ declare(strict_types=1); -if(!defined('LEVELDB_ZLIB_RAW_COMPRESSION')){ +if(!\defined('LEVELDB_ZLIB_RAW_COMPRESSION')){ //leveldb might not be loaded - define('LEVELDB_ZLIB_RAW_COMPRESSION', 4); + \define('LEVELDB_ZLIB_RAW_COMPRESSION', 4); } -if(!extension_loaded('libdeflate')){ +if(!\extension_loaded('libdeflate')){ function libdeflate_deflate_compress(string $data, int $level = 6) : string{} } //TODO: these need to be defined properly or removed -define('pocketmine\COMPOSER_AUTOLOADER_PATH', dirname(__DIR__, 2) . '/vendor/autoload.php'); +\define('pocketmine\COMPOSER_AUTOLOADER_PATH', \dirname(__DIR__, 2) . '/vendor/autoload.php'); diff --git a/tests/phpstan/configs/check-explicit-mixed-baseline.neon b/tests/phpstan/configs/check-explicit-mixed-baseline.neon index 5ffcc2913..63009a2d1 100644 --- a/tests/phpstan/configs/check-explicit-mixed-baseline.neon +++ b/tests/phpstan/configs/check-explicit-mixed-baseline.neon @@ -283,7 +283,7 @@ parameters: - message: "#^Parameter \\#2 \\$str of function fwrite expects string, mixed given\\.$#" count: 1 - path: ../../../src/utils/MainLogger.php + path: ../../../src/utils/MainLoggerThread.php - message: "#^Cannot access offset 'status' on mixed\\.$#" diff --git a/tests/phpunit/block/regenerate_consistency_check.php b/tests/phpunit/block/regenerate_consistency_check.php index aac5194c9..d1ff6c770 100644 --- a/tests/phpunit/block/regenerate_consistency_check.php +++ b/tests/phpunit/block/regenerate_consistency_check.php @@ -21,14 +21,14 @@ declare(strict_types=1); -require dirname(__DIR__, 3) . '/vendor/autoload.php'; +require \dirname(__DIR__, 3) . '/vendor/autoload.php'; /* This script needs to be re-run after any intentional blockfactory change (adding or removing a block state). */ $factory = new \pocketmine\block\BlockFactory(); -$old = json_decode(file_get_contents(__DIR__ . '/block_factory_consistency_check.json'), true); -$new = array_map( +$old = \json_decode(\file_get_contents(__DIR__ . '/block_factory_consistency_check.json'), true); +$new = \array_map( function(\pocketmine\block\Block $block) : string{ return $block->getName(); }, @@ -46,6 +46,6 @@ foreach($new as $k => $name){ echo "Name changed (" . ($k >> 4) . ":" . ($k & 0xf) . "): " . $old[$k] . " -> " . $name . "\n"; } } -file_put_contents(__DIR__ . '/block_factory_consistency_check.json', json_encode( +\file_put_contents(__DIR__ . '/block_factory_consistency_check.json', \json_encode( $new )); diff --git a/tests/phpunit/scheduler/AsyncPoolTest.php b/tests/phpunit/scheduler/AsyncPoolTest.php index ab2bc5a29..810e475e9 100644 --- a/tests/phpunit/scheduler/AsyncPoolTest.php +++ b/tests/phpunit/scheduler/AsyncPoolTest.php @@ -50,8 +50,7 @@ class AsyncPoolTest extends TestCase{ public function tearDown() : void{ $this->pool->shutdown(); - $this->mainLogger->shutdown(); - $this->mainLogger->join(); + $this->mainLogger->shutdownLogWriterThread(); } public function testTaskLeak() : void{