From 99fa6b4b1284920f8565e8fc84e0948a9d263298 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 17 Feb 2021 17:45:43 +0100 Subject: [PATCH 1/3] Set blocking on logging before process.exit On windows the terminal output is asynchronous by default which leads to log lines being lost if text is written to stdout before calling process.exit. Fix by using setBlocking before doing the log call. --- packages/master/master.js | 2 ++ packages/slave/slave.js | 1 + 2 files changed, 3 insertions(+) diff --git a/packages/master/master.js b/packages/master/master.js index 55f0da3a..0ff2f37a 100755 --- a/packages/master/master.js +++ b/packages/master/master.js @@ -1957,6 +1957,7 @@ async function startServer(args) { let secondSigint = false; process.on("SIGINT", () => { if (secondSigint) { + setBlocking(true); logger.fatal("Caught second interrupt, terminating immediately"); // eslint-disable-next-line no-process-exit process.exit(1); @@ -1969,6 +1970,7 @@ async function startServer(args) { let secondSigterm = false; process.on("SIGTERM", () => { if (secondSigterm) { + setBlocking(true); logger.fatal("Caught second termination, terminating immediately"); // eslint-disable-next-line no-process-exit process.exit(1); diff --git a/packages/slave/slave.js b/packages/slave/slave.js index 120df4de..5998c29f 100755 --- a/packages/slave/slave.js +++ b/packages/slave/slave.js @@ -1404,6 +1404,7 @@ class Slave extends libLink.Link { // Clear silly interval in pidfile library. pidusage.clear(); } catch (err) { + setBlocking(true); logger.error(` +--------------------------------------------------------------------+ | Unexpected error occured while shutting down slave, please report | From 4ca092f351f6c55feefd89a442f480a3829f6167 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 17 Feb 2021 17:48:05 +0100 Subject: [PATCH 2/3] Prevent shutdown running more than once Exit immediately if shutdown has already been called. This is due to the possibility of it being called multiple times from combinations of signals and fatal errors. --- packages/master/master.js | 7 ++++++- packages/slave/slave.js | 3 +++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/master/master.js b/packages/master/master.js index 0ff2f37a..4f69a5dc 100755 --- a/packages/master/master.js +++ b/packages/master/master.js @@ -477,11 +477,16 @@ async function saveUsers(databaseDirectory, file) { await fs.outputFile(filePath, JSON.stringify(serialized, null, 4)); } +let shuttingDown = false; /** * Innitiate shutdown of master server */ async function shutdown() { - logger.info("Shutting down"); + if (shuttingDown) { + return; + } + shuttingDown = true; + let exitStartTime = Date.now(); try { logger.info("Saving configs"); diff --git a/packages/slave/slave.js b/packages/slave/slave.js index 5998c29f..3c89dcb7 100755 --- a/packages/slave/slave.js +++ b/packages/slave/slave.js @@ -1384,6 +1384,9 @@ class Slave extends libLink.Link { * Stops all instances and closes the connection */ async shutdown() { + if (this._shuttingDown) { + return; + } this._shuttingDown = true; this.connector.setTimeout(30); From 9660fa0801abe78384ae8b1815e52b1dcf957bbf Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 17 Feb 2021 17:49:54 +0100 Subject: [PATCH 3/3] Handle SIGHUP gracefully Invoke shutdown instead of exiting immediately when SIGHUP is received, and do the same on both slave and master for signals received. --- packages/master/master.js | 8 ++------ packages/slave/slave.js | 6 +++++- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/master/master.js b/packages/master/master.js index 4f69a5dc..dbc1ed1a 100755 --- a/packages/master/master.js +++ b/packages/master/master.js @@ -1985,13 +1985,9 @@ async function startServer(args) { logger.info("Caught termination signal, shutting down"); shutdown(); }); - - // terminal closed process.on("SIGHUP", () => { - // No graceful cleanup, no warning out (stdout is likely closed.) - // Don't close the terminal with the clusterio master in it. - // eslint-disable-next-line no-process-exit - process.exit(1); + logger.info("Terminal closed, shutting down"); + shutdown(); }); await fs.ensureDir(masterConfig.get("master.database_directory")); diff --git a/packages/slave/slave.js b/packages/slave/slave.js index 3c89dcb7..7e474879 100755 --- a/packages/slave/slave.js +++ b/packages/slave/slave.js @@ -1633,7 +1633,7 @@ async function startSlave() { process.on("SIGTERM", () => { if (secondSigterm) { setBlocking(true); - logger.fatal("Caught second termination signal, terminating immediately"); + logger.fatal("Caught second termination, terminating immediately"); // eslint-disable-next-line no-process-exit process.exit(1); } @@ -1642,6 +1642,10 @@ async function startSlave() { logger.info("Caught termination signal, shutting down"); slave.shutdown(); }); + process.on("SIGHUP", () => { + logger.info("Terminal closed, shutting down"); + slave.shutdown(); + }); slaveConnector.once("connect", () => { logger.add(new libLoggingUtils.LinkTransport({ link: slave }));