diff --git a/lib/factorio/server.js b/lib/factorio/server.js index aaad92b4..70f93d55 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: { @@ -620,16 +631,14 @@ 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); - }); + // 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(); } // On linux this sends SIGTERM to the process. On windows this @@ -638,7 +647,34 @@ 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. In either + // case there's no recovering from it. + 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 SIGKILL"); + this._server.kill('SIGKILL'); + } + }, 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/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", 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(); + }); + }); }); });