From 67a5f3f5572a8058f3e1804273ecf21c6ea068c4 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Mon, 5 Nov 2018 19:01:59 +0000 Subject: [PATCH] Register MainLogger as SPL global, remove hard MainLogger dependency from many areas, break a bunch of cyclic dependencies --- src/pocketmine/PocketMine.php | 8 +- src/pocketmine/ThreadManager.php | 4 +- src/pocketmine/level/Position.php | 3 +- .../format/io/region/RegionLevelProvider.php | 3 +- .../level/format/io/region/RegionLoader.php | 3 +- .../network/mcpe/protocol/LoginPacket.php | 3 +- src/pocketmine/permission/BanList.php | 7 +- src/pocketmine/utils/Config.php | 2 +- src/pocketmine/utils/Timezone.php | 81 +++++++++---------- src/pocketmine/utils/Utils.php | 5 +- .../src/pmmp/TesterPlugin/Main.php | 1 - .../tests/AsyncTaskMainLoggerTest.php | 68 ---------------- 12 files changed, 52 insertions(+), 136 deletions(-) delete mode 100644 tests/plugins/TesterPlugin/src/pmmp/TesterPlugin/tests/AsyncTaskMainLoggerTest.php diff --git a/src/pocketmine/PocketMine.php b/src/pocketmine/PocketMine.php index 9023c2e19..dabc0c835 100644 --- a/src/pocketmine/PocketMine.php +++ b/src/pocketmine/PocketMine.php @@ -193,15 +193,11 @@ namespace pocketmine { } //Logger has a dependency on timezone - $tzError = Timezone::init(); + Timezone::init(); $logger = new MainLogger(\pocketmine\DATA . "server.log"); $logger->registerStatic(); - - foreach($tzError as $e){ - $logger->warning($e); - } - unset($tzError); + \GlobalLogger::set($logger); if(extension_loaded("xdebug")){ $logger->warning(PHP_EOL . PHP_EOL . PHP_EOL . "\tYou are running " . \pocketmine\NAME . " with xdebug enabled. This has a major impact on performance." . PHP_EOL . PHP_EOL); diff --git a/src/pocketmine/ThreadManager.php b/src/pocketmine/ThreadManager.php index 0691a3b89..980515eda 100644 --- a/src/pocketmine/ThreadManager.php +++ b/src/pocketmine/ThreadManager.php @@ -23,8 +23,6 @@ declare(strict_types=1); namespace pocketmine; -use pocketmine\utils\MainLogger; - class ThreadManager extends \Volatile{ /** @var ThreadManager */ @@ -72,7 +70,7 @@ class ThreadManager extends \Volatile{ } public function stopAll() : int{ - $logger = MainLogger::getLogger(); + $logger = \GlobalLogger::get(); $erroredThreads = 0; diff --git a/src/pocketmine/level/Position.php b/src/pocketmine/level/Position.php index b2e31bb4c..a693ccda5 100644 --- a/src/pocketmine/level/Position.php +++ b/src/pocketmine/level/Position.php @@ -24,7 +24,6 @@ declare(strict_types=1); namespace pocketmine\level; use pocketmine\math\Vector3; -use pocketmine\utils\MainLogger; class Position extends Vector3{ @@ -63,7 +62,7 @@ class Position extends Vector3{ */ public function getLevel(){ if($this->level !== null and $this->level->isClosed()){ - MainLogger::getLogger()->debug("Position was holding a reference to an unloaded Level"); + \GlobalLogger::get()->debug("Position was holding a reference to an unloaded Level"); $this->level = null; } diff --git a/src/pocketmine/level/format/io/region/RegionLevelProvider.php b/src/pocketmine/level/format/io/region/RegionLevelProvider.php index 6c9e637f5..8e58ce2f5 100644 --- a/src/pocketmine/level/format/io/region/RegionLevelProvider.php +++ b/src/pocketmine/level/format/io/region/RegionLevelProvider.php @@ -28,7 +28,6 @@ use pocketmine\level\format\io\BaseLevelProvider; use pocketmine\level\format\io\data\JavaLevelData; use pocketmine\level\format\io\LevelData; use pocketmine\level\Level; -use pocketmine\utils\MainLogger; abstract class RegionLevelProvider extends BaseLevelProvider{ @@ -131,7 +130,7 @@ abstract class RegionLevelProvider extends BaseLevelProvider{ try{ $region->open(); }catch(CorruptedRegionException $e){ - $logger = MainLogger::getLogger(); + $logger = \GlobalLogger::get(); $logger->error("Corrupted region file detected: " . $e->getMessage()); $region->close(false); //Do not write anything to the file diff --git a/src/pocketmine/level/format/io/region/RegionLoader.php b/src/pocketmine/level/format/io/region/RegionLoader.php index 1412f759d..08b680a18 100644 --- a/src/pocketmine/level/format/io/region/RegionLoader.php +++ b/src/pocketmine/level/format/io/region/RegionLoader.php @@ -26,7 +26,6 @@ namespace pocketmine\level\format\io\region; use pocketmine\level\format\ChunkException; use pocketmine\level\format\io\exception\CorruptedChunkException; use pocketmine\utils\Binary; -use pocketmine\utils\MainLogger; class RegionLoader{ public const COMPRESSION_GZIP = 1; @@ -117,7 +116,7 @@ class RegionLoader{ } if($length > ($this->locationTable[$index][1] << 12)){ //Invalid chunk, bigger than defined number of sectors - MainLogger::getLogger()->error("Corrupted bigger chunk detected (bigger than number of sectors given in header)"); + \GlobalLogger::get()->error("Corrupted bigger chunk detected (bigger than number of sectors given in header)"); $this->locationTable[$index][1] = $length >> 12; $this->writeLocationIndex($index); }elseif($compression !== self::COMPRESSION_ZLIB and $compression !== self::COMPRESSION_GZIP){ diff --git a/src/pocketmine/network/mcpe/protocol/LoginPacket.php b/src/pocketmine/network/mcpe/protocol/LoginPacket.php index a2f7f2c7c..3a801b8e9 100644 --- a/src/pocketmine/network/mcpe/protocol/LoginPacket.php +++ b/src/pocketmine/network/mcpe/protocol/LoginPacket.php @@ -29,7 +29,6 @@ namespace pocketmine\network\mcpe\protocol; use pocketmine\entity\Skin; use pocketmine\network\mcpe\handler\SessionHandler; use pocketmine\utils\BinaryStream; -use pocketmine\utils\MainLogger; use pocketmine\utils\Utils; class LoginPacket extends DataPacket{ @@ -89,7 +88,7 @@ class LoginPacket extends DataPacket{ throw $e; } - $logger = MainLogger::getLogger(); + $logger = \GlobalLogger::get(); $logger->debug(get_class($e) . " was thrown while decoding connection request in login (protocol version " . ($this->protocol ?? "unknown") . "): " . $e->getMessage()); foreach(Utils::getTrace(0, $e->getTrace()) as $line){ $logger->debug($line); diff --git a/src/pocketmine/permission/BanList.php b/src/pocketmine/permission/BanList.php index 7970f8848..681dc23d5 100644 --- a/src/pocketmine/permission/BanList.php +++ b/src/pocketmine/permission/BanList.php @@ -24,7 +24,6 @@ declare(strict_types=1); namespace pocketmine\permission; use pocketmine\Server; -use pocketmine\utils\MainLogger; class BanList{ @@ -153,7 +152,7 @@ class BanList{ $this->list[$entry->getName()] = $entry; } }catch(\Throwable $e){ - $logger = MainLogger::getLogger(); + $logger = \GlobalLogger::get(); $logger->critical("Failed to parse ban entry from string \"$line\": " . $e->getMessage()); $logger->logException($e); } @@ -161,7 +160,7 @@ class BanList{ } fclose($fp); }else{ - MainLogger::getLogger()->error("Could not load ban list"); + \GlobalLogger::get()->error("Could not load ban list"); } } @@ -182,7 +181,7 @@ class BanList{ } fclose($fp); }else{ - MainLogger::getLogger()->error("Could not save ban list"); + \GlobalLogger::get()->error("Could not save ban list"); } } } diff --git a/src/pocketmine/utils/Config.php b/src/pocketmine/utils/Config.php index e09c86cfb..353ca1e43 100644 --- a/src/pocketmine/utils/Config.php +++ b/src/pocketmine/utils/Config.php @@ -525,7 +525,7 @@ class Config{ break; } if(isset($this->config[$k])){ - MainLogger::getLogger()->debug("[Config] Repeated property " . $k . " on file " . $this->file); + \GlobalLogger::get()->debug("[Config] Repeated property " . $k . " on file " . $this->file); } $this->config[$k] = $v; } diff --git a/src/pocketmine/utils/Timezone.php b/src/pocketmine/utils/Timezone.php index 1138f66a7..a1169ddfa 100644 --- a/src/pocketmine/utils/Timezone.php +++ b/src/pocketmine/utils/Timezone.php @@ -29,54 +29,49 @@ abstract class Timezone{ return ini_get('date.timezone'); } - public static function init() : array{ - $messages = []; - do{ - $timezone = ini_get("date.timezone"); - if($timezone !== ""){ - /* - * This is here so that people don't come to us complaining and fill up the issue tracker when they put - * an incorrect timezone abbreviation in php.ini apparently. - */ - if(strpos($timezone, "/") === false){ - $default_timezone = timezone_name_from_abbr($timezone); - if($default_timezone !== false){ - ini_set("date.timezone", $default_timezone); - date_default_timezone_set($default_timezone); - break; - }else{ - //Bad php.ini value, try another method to detect timezone - $messages[] = "Timezone \"$timezone\" could not be parsed as a valid timezone from php.ini, falling back to auto-detection"; - } - }else{ - date_default_timezone_set($timezone); - break; + public static function init() : void{ + $timezone = ini_get("date.timezone"); + if($timezone !== ""){ + /* + * This is here so that people don't come to us complaining and fill up the issue tracker when they put + * an incorrect timezone abbreviation in php.ini apparently. + */ + if(strpos($timezone, "/") === false){ + $default_timezone = timezone_name_from_abbr($timezone); + if($default_timezone !== false){ + ini_set("date.timezone", $default_timezone); + date_default_timezone_set($default_timezone); + return; } + + //Bad php.ini value, try another method to detect timezone + \GlobalLogger::get()->warning("Timezone \"$timezone\" could not be parsed as a valid timezone from php.ini, falling back to auto-detection"); + }else{ + date_default_timezone_set($timezone); + return; } + } - if(($timezone = self::detectSystemTimezone()) and date_default_timezone_set($timezone)){ - //Success! Timezone has already been set and validated in the if statement. - //This here is just for redundancy just in case some program wants to read timezone data from the ini. - ini_set("date.timezone", $timezone); - break; - } + if(($timezone = self::detectSystemTimezone()) and date_default_timezone_set($timezone)){ + //Success! Timezone has already been set and validated in the if statement. + //This here is just for redundancy just in case some program wants to read timezone data from the ini. + ini_set("date.timezone", $timezone); + return; + } - if($response = Internet::getURL("http://ip-api.com/json") //If system timezone detection fails or timezone is an invalid value. - and $ip_geolocation_data = json_decode($response, true) - and $ip_geolocation_data['status'] !== 'fail' - and date_default_timezone_set($ip_geolocation_data['timezone']) - ){ - //Again, for redundancy. - ini_set("date.timezone", $ip_geolocation_data['timezone']); - break; - } + if($response = Internet::getURL("http://ip-api.com/json") //If system timezone detection fails or timezone is an invalid value. + and $ip_geolocation_data = json_decode($response, true) + and $ip_geolocation_data['status'] !== 'fail' + and date_default_timezone_set($ip_geolocation_data['timezone']) + ){ + //Again, for redundancy. + ini_set("date.timezone", $ip_geolocation_data['timezone']); + return; + } - ini_set("date.timezone", "UTC"); - date_default_timezone_set("UTC"); - $messages[] = "Timezone could not be automatically determined or was set to an invalid value. An incorrect timezone will result in incorrect timestamps on console logs. It has been set to \"UTC\" by default. You can change it on the php.ini file."; - }while(false); - - return $messages; + ini_set("date.timezone", "UTC"); + date_default_timezone_set("UTC"); + \GlobalLogger::get()->warning("Timezone could not be automatically determined or was set to an invalid value. An incorrect timezone will result in incorrect timestamps on console logs. It has been set to \"UTC\" by default. You can change it on the php.ini file."); } public static function detectSystemTimezone(){ diff --git a/src/pocketmine/utils/Utils.php b/src/pocketmine/utils/Utils.php index 33e409a90..f2eeeb26c 100644 --- a/src/pocketmine/utils/Utils.php +++ b/src/pocketmine/utils/Utils.php @@ -407,8 +407,9 @@ class Utils{ } public static function kill($pid) : void{ - if(MainLogger::isRegisteredStatic()){ - MainLogger::getLogger()->syncFlushBuffer(); + $logger = \GlobalLogger::get(); + if($logger instanceof MainLogger){ + $logger->syncFlushBuffer(); } switch(Utils::getOS()){ case "win": diff --git a/tests/plugins/TesterPlugin/src/pmmp/TesterPlugin/Main.php b/tests/plugins/TesterPlugin/src/pmmp/TesterPlugin/Main.php index c008b1498..7b40795b8 100644 --- a/tests/plugins/TesterPlugin/src/pmmp/TesterPlugin/Main.php +++ b/tests/plugins/TesterPlugin/src/pmmp/TesterPlugin/Main.php @@ -44,7 +44,6 @@ class Main extends PluginBase implements Listener{ $this->waitingTests = [ new tests\AsyncTaskMemoryLeakTest($this), - new tests\AsyncTaskMainLoggerTest($this), new tests\AsyncTaskPublishProgressRaceTest($this) ]; } diff --git a/tests/plugins/TesterPlugin/src/pmmp/TesterPlugin/tests/AsyncTaskMainLoggerTest.php b/tests/plugins/TesterPlugin/src/pmmp/TesterPlugin/tests/AsyncTaskMainLoggerTest.php deleted file mode 100644 index 4972ec286..000000000 --- a/tests/plugins/TesterPlugin/src/pmmp/TesterPlugin/tests/AsyncTaskMainLoggerTest.php +++ /dev/null @@ -1,68 +0,0 @@ -getPlugin()->getServer()->getAsyncPool()->submitTask(new class($this) extends AsyncTask{ - - /** @var bool */ - protected $success = false; - - public function __construct(AsyncTaskMainLoggerTest $testObject){ - $this->storeLocal($testObject); - } - - public function onRun() : void{ - ob_start(); - MainLogger::getLogger()->info("Testing"); - if(strpos(ob_get_contents(), "Testing") !== false){ - $this->success = true; - } - ob_end_flush(); - } - - public function onCompletion() : void{ - /** @var AsyncTaskMainLoggerTest $test */ - $test = $this->fetchLocal(); - $test->setResult($this->success ? Test::RESULT_OK : Test::RESULT_FAILED); - } - }); - } - - public function getName() : string{ - return "MainLogger::getLogger() works in AsyncTasks"; - } - - public function getDescription() : string{ - return "Verifies that the MainLogger is accessible by MainLogger::getLogger() in an AsyncTask"; - } - - -}