Merge pull request #165 from Cryptkeeper/loop-protections

5.4.1 release preview

docs/CHANGELOG.md

@@ -1,3 +1,8 @@
+**5.4.1** *(May 10 2020)*
+- Adds warnings when the system is pinging more frequently than it is getting replies.
+- Replaces the legacy mc-ping-updated dependency with a new library, mcping-js. This fixes some bugs that could cause "zombie" connections and cause stuttering in the ping loops.
+- Fixes potential crash issue when hashing favicons.
+
 **5.4.0** *(May 9 2020)*
 - Favicons are now served over the http server (using a unique hash). This allows the favicons to be safely cached for long durations and still support dynamic updates.
 - Adds "graphDurationLabel" to `config.json` which allows you to manually modify the "24h Peak" label to a custom time duration.

lib/ping.js

@@ -1,4 +1,4 @@
-const minecraftJavaPing = require('mc-ping-updated')
+const minecraftJavaPing = require('mcping-js')
 const minecraftBedrockPing = require('mcpe-ping-fixed')
 
 const logger = require('./logger')
@@ -10,13 +10,15 @@ function ping (serverRegistration, timeout, callback, version) {
   switch (serverRegistration.data.type) {
     case 'PC':
       serverRegistration.unfurlSrv((host, port) => {
-        minecraftJavaPing(host, port || 25565, (err, res) => {
+        const server = new minecraftJavaPing.MinecraftServer(host, port || 25565)
+
+        server.ping(timeout, version, (err, res) => {
           if (err) {
             callback(err)
           } else {
             const payload = {
               players: {
-                online: capPlayerCount(host, parseInt(res.players.online))
+                online: capPlayerCount(serverRegistration.data.ip, parseInt(res.players.online))
               },
               version: parseInt(res.version.protocol)
             }
@@ -28,7 +30,7 @@ function ping (serverRegistration, timeout, callback, version) {
 
             callback(null, payload)
           }
-        }, timeout, version)
+        })
       })
       break
 
@@ -71,6 +73,7 @@ function capPlayerCount (host, playerCount) {
 class PingController {
   constructor (app) {
     this._app = app
+    this._isRunningTasks = false
   }
 
   schedule () {
@@ -113,8 +116,15 @@ class PingController {
   }
 
   startPingTasks = (callback) => {
+    if (this._isRunningTasks) {
+      logger.log('warn', 'Started re-pinging servers before the last loop has finished! You may need to increase "rates.pingAll" in config.json')
+
+      return
+    }
+
+    this._isRunningTasks = true
+
     const results = []
-    let remainingTasks = this._app.serverRegistrations.length
 
     for (const serverRegistration of this._app.serverRegistrations) {
       const version = serverRegistration.getNextProtocolVersion()
@@ -130,7 +140,10 @@ class PingController {
           version
         }
 
-        if (--remainingTasks === 0) {
+        if (Object.keys(results).length === this._app.serverRegistrations.length) {
+          // Loop has completed, release the locking flag
+          this._isRunningTasks = false
+
           callback(results)
         }
       }, version.protocolId)

lib/servers.js

@@ -223,11 +223,7 @@ class ServerRegistration {
 
       // Generate an updated hash
       // This is used by #getFaviconUrl
-      if (!this._faviconHasher) {
+      this.faviconHash = crypto.createHash('md5').update(favicon).digest('hex').toString()
-        this._faviconHasher = crypto.createHash('md5')
-      }
-
-      this.faviconHash = this._faviconHasher.update(favicon).digest('hex').toString()
 
       return true
     }

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "minetrack",
-  "version": "5.4.0",
+  "version": "5.4.1",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
@@ -5495,11 +5495,6 @@
         "object-visit": "^1.0.0"
       }
     },
-    "mc-ping-updated": {
-      "version": "0.1.1",
-      "resolved": "https://registry.npmjs.org/mc-ping-updated/-/mc-ping-updated-0.1.1.tgz",
-      "integrity": "sha512-wYX4B41Jog+RvLFgYPytNvbw+b9mjjsKKqMLJ57a7aH3VgJCNzk6qeWFCTz7FLCWTEk8F81RBqh8kMPCjUVdNw=="
-    },
     "mcpe-ping-fixed": {
       "version": "0.0.3",
       "resolved": "https://registry.npmjs.org/mcpe-ping-fixed/-/mcpe-ping-fixed-0.0.3.tgz",
@@ -5509,6 +5504,11 @@
         "portfinder": "^0.4.0"
       }
     },
+    "mcping-js": {
+      "version": "1.4.0",
+      "resolved": "https://registry.npmjs.org/mcping-js/-/mcping-js-1.4.0.tgz",
+      "integrity": "sha512-0P4fIGzxFrQqUwNG8LqZtrZZ02DNUz/OMyTMOncfPjEISgm+JOQQ2OjYc0K+8piT1e6yzvLJoUVTkDAiTdCV4w=="
+    },
     "md5.js": {
       "version": "1.3.5",
       "resolved": "https://registry.npmjs.org/md5.js/-/md5.js-1.3.5.tgz",

package.json

@@ -1,12 +1,12 @@
 {
   "name": "minetrack",
-  "version": "5.4.0",
+  "version": "5.4.1",
   "description": "A Minecraft server tracker that lets you focus on the basics.",
   "main": "main.js",
   "dependencies": {
     "finalhandler": "^1.1.2",
-    "mc-ping-updated": "0.1.1",
     "mcpe-ping-fixed": "0.0.3",
+    "mcping-js": "^1.4.0",
     "request": "2.88.2",
     "serve-static": "^1.14.1",
     "sqlite3": "4.1.1",