From a753c1342d5c6fe6b092dfcb2abdf6c176b959e9 Mon Sep 17 00:00:00 2001 From: "Dylan K. Taylor" Date: Sun, 24 Mar 2019 17:42:41 +0000 Subject: [PATCH] Clean up Query cache handling, remove useless timeouts the timeout was entirely useless, because: - when shorter than 25.6 seconds (512 ticks) it would cause caches to be needlessly destroyed and regenerated - when longer than 25.6 seconds, just made outdated caches persist for longer, even after the query info was regenerated. This now uses a mark-dirty model to deal with caches, which means that plugin modifications to the query data will be reflected immediately, regardless of when they are made. Previously, modifying the result of Server->getQueryInformation() would have inconsistent results. --- src/pocketmine/Server.php | 7 +-- .../event/server/QueryRegenerateEvent.php | 43 ++++++++++--------- src/pocketmine/network/query/QueryHandler.php | 23 +--------- 3 files changed, 26 insertions(+), 47 deletions(-) diff --git a/src/pocketmine/Server.php b/src/pocketmine/Server.php index d52a1462a..f0a9af271 100644 --- a/src/pocketmine/Server.php +++ b/src/pocketmine/Server.php @@ -1263,7 +1263,7 @@ class Server{ register_shutdown_function([$this, "crashDump"]); - $this->queryRegenerateTask = new QueryRegenerateEvent($this, 5); + $this->queryRegenerateTask = new QueryRegenerateEvent($this); $this->pluginManager->loadPlugins($this->pluginPath); @@ -2008,10 +2008,7 @@ class Server{ } if(($this->tickCounter & 0b111111111) === 0){ - ($this->queryRegenerateTask = new QueryRegenerateEvent($this, 5))->call(); - if($this->queryHandler !== null){ - $this->queryHandler->regenerateInfo(); - } + ($this->queryRegenerateTask = new QueryRegenerateEvent($this))->call(); } if($this->sendUsageTicker > 0 and --$this->sendUsageTicker === 0){ diff --git a/src/pocketmine/event/server/QueryRegenerateEvent.php b/src/pocketmine/event/server/QueryRegenerateEvent.php index a9393a1c6..d7267baac 100644 --- a/src/pocketmine/event/server/QueryRegenerateEvent.php +++ b/src/pocketmine/event/server/QueryRegenerateEvent.php @@ -35,8 +35,6 @@ use function substr; class QueryRegenerateEvent extends ServerEvent{ public const GAME_ID = "MINECRAFTPE"; - /** @var int */ - private $timeout; /** @var string */ private $serverName; /** @var bool */ @@ -68,13 +66,16 @@ class QueryRegenerateEvent extends ServerEvent{ /** @var array */ private $extraData = []; + /** @var string|null */ + private $longQueryCache = null; + /** @var string|null */ + private $shortQueryCache = null; + /** * @param Server $server - * @param int $timeout */ - public function __construct(Server $server, int $timeout = 5){ - $this->timeout = $timeout; + public function __construct(Server $server){ $this->serverName = $server->getMotd(); $this->listPlugins = $server->getProperty("settings.query-plugins", true); $this->plugins = $server->getPluginManager()->getPlugins(); @@ -98,20 +99,9 @@ class QueryRegenerateEvent extends ServerEvent{ } - /** - * Gets the min. timeout for Query Regeneration - * - * @return int - */ - public function getTimeout() : int{ - return $this->timeout; - } - - /** - * @param int $timeout - */ - public function setTimeout(int $timeout) : void{ - $this->timeout = $timeout; + private function destroyCache() : void{ + $this->longQueryCache = null; + $this->shortQueryCache = null; } /** @@ -126,6 +116,7 @@ class QueryRegenerateEvent extends ServerEvent{ */ public function setServerName(string $serverName) : void{ $this->serverName = $serverName; + $this->destroyCache(); } /** @@ -140,6 +131,7 @@ class QueryRegenerateEvent extends ServerEvent{ */ public function setListPlugins(bool $value) : void{ $this->listPlugins = $value; + $this->destroyCache(); } /** @@ -154,6 +146,7 @@ class QueryRegenerateEvent extends ServerEvent{ */ public function setPlugins(array $plugins) : void{ $this->plugins = $plugins; + $this->destroyCache(); } /** @@ -168,6 +161,7 @@ class QueryRegenerateEvent extends ServerEvent{ */ public function setPlayerList(array $players) : void{ $this->players = $players; + $this->destroyCache(); } /** @@ -182,6 +176,7 @@ class QueryRegenerateEvent extends ServerEvent{ */ public function setPlayerCount(int $count) : void{ $this->numPlayers = $count; + $this->destroyCache(); } /** @@ -196,6 +191,7 @@ class QueryRegenerateEvent extends ServerEvent{ */ public function setMaxPlayerCount(int $count) : void{ $this->maxPlayers = $count; + $this->destroyCache(); } /** @@ -210,6 +206,7 @@ class QueryRegenerateEvent extends ServerEvent{ */ public function setWorld(string $world) : void{ $this->map = $world; + $this->destroyCache(); } /** @@ -226,12 +223,16 @@ class QueryRegenerateEvent extends ServerEvent{ */ public function setExtraData(array $extraData) : void{ $this->extraData = $extraData; + $this->destroyCache(); } /** * @return string */ public function getLongQuery() : string{ + if($this->longQueryCache !== null){ + return $this->longQueryCache; + } $query = ""; $plist = $this->server_engine; @@ -274,13 +275,13 @@ class QueryRegenerateEvent extends ServerEvent{ } $query .= "\x00"; - return $query; + return $this->longQueryCache = $query; } /** * @return string */ public function getShortQuery() : string{ - return $this->serverName . "\x00" . $this->gametype . "\x00" . $this->map . "\x00" . $this->numPlayers . "\x00" . $this->maxPlayers . "\x00" . Binary::writeLShort($this->port) . $this->ip . "\x00"; + return $this->shortQueryCache ?? ($this->shortQueryCache = $this->serverName . "\x00" . $this->gametype . "\x00" . $this->map . "\x00" . $this->numPlayers . "\x00" . $this->maxPlayers . "\x00" . Binary::writeLShort($this->port) . $this->ip . "\x00"); } } diff --git a/src/pocketmine/network/query/QueryHandler.php b/src/pocketmine/network/query/QueryHandler.php index 0b620f81c..9092e1546 100644 --- a/src/pocketmine/network/query/QueryHandler.php +++ b/src/pocketmine/network/query/QueryHandler.php @@ -34,7 +34,6 @@ use pocketmine\utils\BinaryDataException; use pocketmine\utils\BinaryStream; use function chr; use function hash; -use function microtime; use function random_bytes; use function strlen; use function substr; @@ -46,12 +45,6 @@ class QueryHandler{ private $lastToken; /** @var string */ private $token; - /** @var string */ - private $longData; - /** @var string */ - private $shortData; - /** @var float */ - private $timeout; public const HANDSHAKE = 9; public const STATISTICS = 0; @@ -74,7 +67,6 @@ class QueryHandler{ $this->regenerateToken(); $this->lastToken = $this->token; - $this->regenerateInfo(); $this->server->getLogger()->info($this->server->getLanguage()->translateString("pocketmine.server.query.running", [$addr, $port])); } @@ -83,13 +75,6 @@ class QueryHandler{ $this->server->getLogger()->debug("[Query] $message"); } - public function regenerateInfo() : void{ - $ev = $this->server->getQueryInformation(); - $this->longData = $ev->getLongQuery(); - $this->shortData = $ev->getShortQuery(); - $this->timeout = microtime(true) + $ev->getTimeout(); - } - public function regenerateToken() : void{ $this->lastToken = $this->token; $this->token = random_bytes(16); @@ -136,15 +121,11 @@ class QueryHandler{ $reply = chr(self::STATISTICS); $reply .= Binary::writeInt($sessionID); - if($this->timeout < microtime(true)){ - $this->regenerateInfo(); - } - $remaining = $stream->getRemaining(); if(strlen($remaining) === 4){ //TODO: check this! according to the spec, this should always be here and always be FF FF FF 01 - $reply .= $this->longData; + $reply .= $this->server->getQueryInformation()->getLongQuery(); }else{ - $reply .= $this->shortData; + $reply .= $this->server->getQueryInformation()->getShortQuery(); } $interface->sendRawPacket($address, $port, $reply);