From 9082b40abd7fb2c8cfa5675a1d0ff505051b94d0 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 28 Oct 2019 21:13:07 +0100 Subject: [PATCH 1/4] Update rcon-client Update rcon-client to 4.2.0 in order to remove workaround for issue with rcon-client 4.1.0 that has been resolved. --- lib/factorio/server.js | 9 --------- package.json | 2 +- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/lib/factorio/server.js b/lib/factorio/server.js index aaad92b4..d05cf4f3 100644 --- a/lib/factorio/server.js +++ b/lib/factorio/server.js @@ -621,15 +621,6 @@ class FactorioServer extends events.EventEmitter { this._state = "stopping"; if (this._rconClient) { await this._rconClient.end(); - - // Unfortunately version 4.1.0 of rcon-client doesn't send an - // end event on .end() and causes an error if the close operation - // fails. Wait a bit here to ensure it's closed before killing - // Factorio. This is only an issue on Windows where Factorio is - // terminated without cleanup. - await new Promise((resolve, reject) => { - setTimeout(resolve, 100); - }); } // On linux this sends SIGTERM to the process. On windows this diff --git a/package.json b/package.json index 960e0a8b..6b84b2d7 100644 --- a/package.json +++ b/package.json @@ -58,7 +58,7 @@ "node-factorio-api": "^0.3.8", "node-forge": "^0.7.1", "prom-client": "^10.2.3", - "rcon-client": "^4.0.4", + "rcon-client": "^4.2.0", "request": "^2.83.0", "sanitizer": "^0.1.3", "simple-git": "^1.96.0", From a781c2f98869750add423473a7961a04c4829150 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 28 Oct 2019 21:55:36 +0100 Subject: [PATCH 2/4] Wait for RCON connecting on FactorioServer.stop() If the RCON client is currently in the process of connecting to the Factorio server then wait for it to complete the connection to avoid it emmiting out an error connecting in FactorioServer.stop(). --- lib/factorio/server.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/factorio/server.js b/lib/factorio/server.js index d05cf4f3..afae615d 100644 --- a/lib/factorio/server.js +++ b/lib/factorio/server.js @@ -620,6 +620,13 @@ class FactorioServer extends events.EventEmitter { this._state = "stopping"; if (this._rconClient) { + + // If RCON is not yet fully connected that operation needs to + // complete before the RCON connection can be closed cleanly. + if (!this._rconReady) { + await events.once(this, 'rcon-ready'); + } + await this._rconClient.end(); } From 246a8031c1a8a0833cf2269414543a9d99baae9b Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 28 Oct 2019 21:56:57 +0100 Subject: [PATCH 3/4] Add hang detection to FactorioServer.stop() Attempt to detect if the Factorio server has hanged in FactorioServer.stop() and send a second SIGTERM if this is the case. --- lib/factorio/server.js | 39 +++++++++++++++++++++++++++++++++++++- test/integration/server.js | 17 +++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/lib/factorio/server.js b/lib/factorio/server.js index afae615d..8d031f5d 100644 --- a/lib/factorio/server.js +++ b/lib/factorio/server.js @@ -281,6 +281,17 @@ const outputHeuristics = [ } }, + // Message indicating the server is shutting down + { + filter: { + type: 'generic', + message: /^Quitting: / + }, + action: function(output) { + this.emit('_quitting'); + } + }, + // Messages that might be tha cause of an unexpected shutdown. { filter: { @@ -636,7 +647,33 @@ class FactorioServer extends events.EventEmitter { // developers. Rcon does not recognize /quit, stdin is not // recognized, and there's no "send CTRL+C to process" on Windows. this._server.kill(); - await events.once(this._server, 'exit'); + + // There appears to be an race condition where sending SIGTERM + // immediatly before the RCON interface comes online causes the + // Factorio server to hang. It's also possible the server doesn't + // stop if it's stuck with an infinite lua code loop. + if (process.platform !== "win32") { + let hanged = true; + function setAlive(output) { hanged = false; } + this.on('_quitting', setAlive); + + let timeoutId = setTimeout(() => { + if (hanged) { + console.error("Factorio appears to have hanged, sending second SIGTERM"); + this._server.kill(); + } + }, 5000); + + await events.once(this._server, 'exit'); + + clearTimeout(timeoutId); + this.off('_quitting', setAlive); + + // On windows the process is terminated immediately, but to keep + // ordering wait until after the exit event here. + } else { + await events.once(this._server, 'exit'); + } } /** diff --git a/test/integration/server.js b/test/integration/server.js index b1213be2..65d2c169 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -1,4 +1,5 @@ const assert = require("assert").strict; +const events = require("events"); const fs = require("fs-extra"); const path = require("path"); @@ -165,5 +166,21 @@ describe("Integration of lib/factorio/server", function() { assert(pass, "server did not output line from test scenario"); }); }); + + describe(".stop() hang detection", function() { + it("should detect factorio hanging on shutdown", async function() { + slowTest(this); + log(".start() for hang detection"); + + await server.start("test.zip"); + if (!server._rconReady) { + await events.once(server, 'rcon-ready'); + } + server.sendRcon("/c while true do end").catch(() => {}); + await new Promise((resolve) => setTimeout(resolve, 300)); + log(".stop() for hang detection"); + await server.stop(); + }); + }); }); }); From bdf8cd11527e366080f646b2f983782f6ad4be84 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 28 Oct 2019 22:30:57 +0100 Subject: [PATCH 4/4] Kill Factorio process if it's dected to hang Send SIGKILL instead of SIGTERM when it's dectected that Factorio hangs on shutdown. This should hopefully resolve the travis ci hanging issuse. --- lib/factorio/server.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/factorio/server.js b/lib/factorio/server.js index 8d031f5d..70f93d55 100644 --- a/lib/factorio/server.js +++ b/lib/factorio/server.js @@ -651,7 +651,8 @@ class FactorioServer extends events.EventEmitter { // There appears to be an race condition where sending SIGTERM // immediatly before the RCON interface comes online causes the // Factorio server to hang. It's also possible the server doesn't - // stop if it's stuck with an infinite lua code loop. + // stop if it's stuck with an infinite lua code loop. In either + // case there's no recovering from it. if (process.platform !== "win32") { let hanged = true; function setAlive(output) { hanged = false; } @@ -659,8 +660,8 @@ class FactorioServer extends events.EventEmitter { let timeoutId = setTimeout(() => { if (hanged) { - console.error("Factorio appears to have hanged, sending second SIGTERM"); - this._server.kill(); + console.error("Factorio appears to have hanged, sending SIGKILL"); + this._server.kill('SIGKILL'); } }, 5000);