Add lock file for configs

packages/controller/controller.ts

@@ -127,7 +127,8 @@ void new lib.Gauge(
 
 async function handleBootstrapCommand(
 	args: any,
-	controllerConfig: lib.ControllerConfig
+	controllerConfig: lib.ControllerConfig,
+	controllerConfigLock: lib.LockFile,
 ): Promise<void> {
 	let subCommand = args._[1];
 
@@ -141,6 +142,8 @@ async function handleBootstrapCommand(
 	await userManager.load(path.join(databaseDirectory, "users.json"));
 
 	if (subCommand === "create-admin") {
+		await controllerConfigLock.acquire(); // Also needed to write to the database files
+
 		if (!args.name) {
 			logger.error("name cannot be blank");
 			process.exitCode = 1;
@@ -156,6 +159,7 @@ async function handleBootstrapCommand(
 		admin.roleIds.add(adminRole.id);
 		admin.isAdmin = true;
 		await userManager.save(path.join(controllerConfig.get("controller.database_directory"), "users.json"));
+		await controllerConfigLock.release();
 
 	} else if (subCommand === "generate-user-token") {
 		let user = userManager.getByName(args.name);
@@ -353,6 +357,7 @@ async function initialize(): Promise<InitializeParameters> {
 
 	let controllerConfig;
 	const controllerConfigPath = args.config;
+	const controllerConfigLock = new lib.LockFile(`${controllerConfigPath}.lock`);
 	logger.info(`Loading config from ${controllerConfigPath}`);
 	try {
 		controllerConfig = await lib.ControllerConfig.fromFile("controller", controllerConfigPath);
@@ -378,10 +383,11 @@ async function initialize(): Promise<InitializeParameters> {
 	}
 
 	if (command === "config") {
-		await lib.handleConfigCommand(args, controllerConfig);
-
+		await lib.handleConfigCommand(args, controllerConfig, controllerConfigLock);
 	} else if (command === "bootstrap") {
-		await handleBootstrapCommand(args, controllerConfig);
+		await handleBootstrapCommand(args, controllerConfig, controllerConfigLock);
+	} else if (shouldRun) {
+		await controllerConfigLock.acquire(); // Hold the lock until process exit
 	}
 
 	return {

packages/ctl/ctl.ts

@@ -236,6 +236,7 @@ export async function initialize(
 
 	let controlConfig;
 	const controlConfigPath = args.config;
+	const controlConfigLock = new lib.LockFile(`${controlConfigPath}.lock`);
 	logger.verbose(`Loading config from ${controlConfigPath}`);
 	try {
 		controlConfig = await lib.ControlConfig.fromFile("control", controlConfigPath);
@@ -257,7 +258,7 @@ export async function initialize(
 
 	// Handle the control-config command before trying to connect.
 	if (args._[0] === "control-config") {
-		await lib.handleConfigCommand(args, controlConfig);
+		await lib.handleConfigCommand(args, controlConfig, controlConfigLock);
 		return { args, controlConfig, ctlPlugins, rootCommands, shouldRun: false };
 	}
 

packages/host/host.ts

@@ -169,6 +169,7 @@ async function startHost() {
 
 	let hostConfig;
 	const hostConfigPath = args.config;
+	const hostConfigLock = new lib.LockFile(`${hostConfigPath}.lock`);
 	logger.info(`Loading config from ${hostConfigPath}`);
 	try {
 		hostConfig = await lib.HostConfig.fromFile("host", hostConfigPath);
@@ -186,10 +187,13 @@ async function startHost() {
 	hostConfig.set("host.version", version); // Allows tracking last loaded version
 
 	if (command === "config") {
-		await lib.handleConfigCommand(args, hostConfig);
+		await lib.handleConfigCommand(args, hostConfig, hostConfigLock);
 		return;
 	}
 
+	// From this point on the host should run
+	await hostConfigLock.acquire();
+
 	// If we get here the command was run
 	await fs.ensureDir(hostConfig.get("host.instances_directory"));
 	await fs.ensureDir(hostConfig.get("host.mods_directory"));

packages/lib/index.ts

@@ -31,6 +31,7 @@ export * from "./src/subscriptions";
 export * from "./src/datastore";
 export * from "./src/load_plugin_list";
 export * from "./src/rce_ops";
+export * from "./src/LockFile";
 
 export { default as ExponentialBackoff } from "./src/ExponentialBackoff";
 export { default as ModStore } from "./src/ModStore";

packages/lib/src/LockFile.ts

@@ -0,0 +1,163 @@
+import { FileHandle, open, unlink, readFile } from "fs/promises";
+import { unlinkSync } from "fs";
+import assert from "assert";
+
+/**
+ * Error thrown when the lock file already exists.
+ */
+export class LockFileExistsError extends Error {
+	constructor(
+		readonly filePath: string
+	) {
+		super(`Cannot acquire lock, a valid lock file exists at ${filePath}`);
+		this.name = "LockFileExistsError";
+	}
+}
+
+/**
+ * Error thrown when the acquire or release called during an invalid state.
+ */
+export class LockFileBusyError extends Error {
+	constructor(message: string) {
+		super(message);
+		this.name = "LockFileBusyError";
+	}
+}
+
+type LockState = "idle" | "acquiring" | "acquired" | "releasing";
+
+/**
+ * Cross-platform lock file utility for Node.js.
+ */
+export class LockFile {
+	private handle: FileHandle | null = null;
+	private state: LockState = "idle";
+
+	constructor(
+		private readonly filePath: string
+	) {}
+
+	/**
+	 * Acquire the lock file.
+	 * Throws LockFileExistsError if already locked.
+	 * Throws LockFileBusyError if not in idle state.
+	 */
+	async acquire(): Promise<void> {
+		if (this.state !== "idle") {
+			throw new LockFileBusyError(`Cannot acquire lock, current state is "${this.state}"`);
+		}
+
+		this.state = "acquiring";
+
+		if (await this.isStale()) {
+			await unlink(this.filePath).catch(() => {}); // ignore errors, may already be removed
+		}
+
+		try {
+			this.handle = await open(this.filePath, "wx");
+			await this.handle.write(`${process.pid}\n`);
+			this.attachExitHandlers();
+			this.state = "acquired";
+		} catch (err: any) {
+			this.state = "idle";
+			if (err?.code === "EEXIST") {
+				throw new LockFileExistsError(this.filePath);
+			}
+			throw err;
+		}
+	}
+
+	/**
+	 * Release the lock file.
+	 * Throws LockFileBusyError if not in acquired state.
+	 */
+	async release(): Promise<void> {
+		if (this.state !== "acquired") {
+			throw new LockFileBusyError(`Cannot release lock, current state is "${this.state}"`);
+		}
+
+		this.state = "releasing";
+		this.detachExitHandlers();
+
+		// Ignored: file may have already been removed by another process
+		try { await this.handle?.close(); } catch {}
+		try { await unlink(this.filePath); } catch {}
+
+		this.handle = null;
+		this.state = "idle";
+	}
+
+	/**
+	 * Check whether the lock is currently held by this process.
+	 * True only if the lock is acquired and the PID in the file
+	 * matches the current process.
+	 */
+	async isHeld(): Promise<boolean> {
+		if (this.state !== "acquired") {
+			return false;
+		}
+
+		try {
+			const pidInLock = Number(await readFile(this.filePath, "utf8"));
+			return pidInLock === process.pid;
+		} catch {
+    		// Ignored: file may not exist or be unreadable, so lock is not held
+			return false;
+		}
+	}
+
+	/**
+	 * Determine whether an existing lock is stale.
+	 */
+	async isStale(): Promise<boolean> {
+		try {
+			const pidInLock = Number(await readFile(this.filePath, "utf8"));
+			if (!Number.isFinite(pidInLock)) { return true; }
+			// signal 0 is a special case to test if a process exists
+			process.kill(pidInLock, 0);
+			return false;
+		} catch {
+			return true;
+		}
+	}
+
+	/**
+	 * Attach process handlers, called when transitioning to "acquired"
+	 */
+	private attachExitHandlers(): void {
+		assert(this.state === "acquiring");
+		process.on("SIGINT", this.handleExit);
+		process.on("SIGTERM", this.handleExit);
+		process.on("SIGHUP", this.handleExit);
+		process.on("uncaughtException", this.handleExit);
+		// eslint-disable-next-line node/no-sync
+		process.on("exit", this.handleExitSync);
+	}
+
+	/**
+	 * Detach process handlers, called when transitioning to "idle"
+	 */
+	private async detachExitHandlers(): Promise<void> {
+		assert(this.state === "releasing");
+		process.off("SIGINT", this.handleExit);
+		process.off("SIGTERM", this.handleExit);
+		process.off("SIGHUP", this.handleExit);
+		process.off("uncaughtException", this.handleExit);
+		// eslint-disable-next-line node/no-sync
+		process.off("exit", this.handleExitSync);
+	}
+
+  	// Event handler for async signals / exceptions
+	private handleExit = async (): Promise<void> => {
+		await this.release();
+	};
+
+  	// Event handler for synchronous process exit
+	private handleExitSync = (): void => {
+		try {
+			unlinkSync(this.filePath);
+		} catch (err) {
+      		// Ignored: file may have already been removed by another process
+		}
+	};
+}

packages/lib/src/shared_commands.ts

@@ -8,6 +8,7 @@ import * as libConfig from "./config";
 import * as libFileOps from "./file_ops";
 import { logger } from "./logging";
 import * as libHelpers from "./helpers";
+import { LockFile } from "./LockFile";
 
 
 function print(...content: any[]) {
@@ -130,11 +131,12 @@ export function configCommand(yargs: any) {
  *
  * @param args - yargs args object.
  * @param instance - Config instance.
- * @param configPath - Path to configuration file.
+ * @param lockFile - Lockfile required if attempting to write
  */
 export async function handleConfigCommand(
 	args: Record<string, unknown>,
 	instance: libConfig.Config<any>,
+	lockFile: LockFile,
 ) {
 	let command = (args._ as string[])[1];
 
@@ -155,6 +157,8 @@ export async function handleConfigCommand(
 		}
 
 	} else if (command === "set") {
+		await lockFile.acquire();
+
 		if (args.stdin) {
 			args.value = (await libHelpers.readStream(process.stdin)).toString().replace(/\r?\n$/, "");
 
@@ -172,5 +176,7 @@ export async function handleConfigCommand(
 				throw err;
 			}
 		}
+
+		await lockFile.release();
 	}
 }

test/integration/clusterio.js

@@ -13,7 +13,7 @@ const { wait } = lib;
 const testStrings = require("../lib/factorio/test_strings");
 const {
 	TestControl, TestControlConnector, url, controlToken, slowTest,
-	exec, execCtl, execCtlProcess, execHost, sendRcon, getControl,
+	execCtl, execCtlProcess, execController, execHost, sendRcon, getControl,
 	spawnNode, instancesDir, factorioDir,
 } = require("./index");
 
@@ -109,12 +109,31 @@ describe("Integration of Clusterio", function() {
 	describe("clusteriocontroller", function() {
 		describe("bootstrap generate-user-token", function() {
 			it("work for existing user", async function() {
-				await exec("node ../../packages/controller bootstrap generate-user-token test");
+				await execController("bootstrap generate-user-token test");
 			});
 
 			it("fails if user does not exist", async function() {
 				await assert.rejects(
-					exec("node ../../packages/controller bootstrap generate-user-token invalid")
+					execController("bootstrap generate-user-token invalid")
+				);
+			});
+		});
+
+		describe("bootstrap create-admin", function() {
+			it("refuses to modify the database files", async function() {
+				await assert.rejects(
+					execController("bootstrap create-admin BootstrapAdminTest")
+				);
+			});
+		});
+
+		describe("config", function() {
+			it("can read the config file", async function() {
+				await execController("config list");
+			});
+			it("refuses to modify the config files", async function() {
+				await assert.rejects(
+					execController("confit set controller.name ConfigEditTest")
 				);
 			});
 		});
@@ -143,6 +162,9 @@ describe("Integration of Clusterio", function() {
 				await events.once(connectorB, "resume");
 				await connectorB.close(1000, "");
 			});
+			it("refuses to start a second process", async function() {
+				await assert.rejects(execController("run"));
+			});
 		});
 
 		describe("queryLogRequestHandler", function() {
@@ -245,6 +267,19 @@ describe("Integration of Clusterio", function() {
 				await stopAltHost(hostProcess);
 			}
 		});
+		describe("config", function() {
+			it("can read the config file", async function() {
+				await execHost("config list");
+			});
+			it("refuses to modify the config files", async function() {
+				await assert.rejects(
+					execHost("confit set host.name ConfigEditTest")
+				);
+			});
+		});
+		it("refuses to start a second process", async function() {
+			await assert.rejects(execHost("run"));
+		});
 	});
 
 	describe("clusterioctl", function() {

test/integration/index.js

@@ -173,26 +173,26 @@ let controllerConfigPath = path.join("temp", "test", "config-controller.json");
 let hostConfigPath = path.join("temp", "test", "config-host.json");
 let controlConfigPath = path.join("temp", "test", "config-control.json");
 
-async function exec(command, options = {}) {
+function exec(command, options = {}) {
 	// Uncomment to show commands run in tests
 	// console.log(command);
 	options = { cwd: path.join("temp", "test"), ...options };
-	return await util.promisify(child_process.exec)(command, options);
+	return util.promisify(child_process.exec)(command, options);
 }
 
-async function execController(...args) {
+function execController(...args) {
 	args[0] = `node --enable-source-maps ../../packages/controller ${args[0]}`;
-	return await exec(...args);
+	return exec(...args);
 }
 
-async function execHost(...args) {
+function execHost(...args) {
 	args[0] = `node --enable-source-maps ../../packages/host ${args[0]}`;
-	return await exec(...args);
+	return exec(...args);
 }
 
-async function execCtlProcess(...args) {
+function execCtlProcess(...args) {
 	args[0] = `node --enable-source-maps ../../packages/ctl ${args[0]}`;
-	return await exec(...args);
+	return exec(...args);
 }
 
 let inExecCtl = false;
@@ -394,6 +394,7 @@ module.exports = {
 	TestControl,
 	TestControlConnector,
 	TestHostConnector,
+	execController,
 	execCtlProcess,
 	slowTest,
 	get,

test/lib/LockFile.test.js

@@ -0,0 +1,126 @@
+/* eslint-disable node/no-sync */
+"use strict";
+const assert = require("assert").strict;
+const fs = require("fs-extra");
+const path = require("path");
+
+const { LockFile } = require("@clusterio/lib");
+
+const baseDir = path.join("temp", "test", "lock_file");
+
+describe("class LockFile", function () {
+	/** @type {string} */
+	let filePath;
+	/** @type {LockFile} */
+	let lock;
+
+	before(async function () {
+		await fs.ensureDir(baseDir);
+	});
+
+	beforeEach(function() {
+		filePath = path.join(baseDir, `${Date.now()}-${Math.random()}.lock`);
+		lock = new LockFile(filePath);
+	});
+
+	afterEach(async function () {
+		await fs.unlink(filePath).catch(() => {});
+	});
+
+	describe("acquire()", function () {
+		it("should create the lock file when acquiring", async function () {
+			await lock.acquire();
+			assert.ok(fs.existsSync(filePath));
+		});
+
+		it("should throw LockFileBusyError if called when not idle", async function () {
+			await lock.acquire();
+			await assert.rejects(lock.acquire(), { name: "LockFileBusyError" });
+		});
+
+		it("should remove stale lock file and acquire it", async function () {
+			await fs.writeFile(filePath, "999999\n"); // stale lock
+			await lock.acquire();
+			assert.ok(fs.existsSync(filePath));
+		});
+
+		it("should throw if existing lock file is not stale", async function () {
+			await fs.writeFile(filePath, `${process.pid}\n`);
+			await assert.rejects(lock.acquire(), { name: "LockFileExistsError" });
+		});
+	});
+
+	describe("release()", function () {
+		it("should remove the lock file when releasing", async function () {
+			await lock.acquire();
+			await lock.release();
+			assert.ok(!fs.existsSync(filePath));
+		});
+
+		it("should throw LockFileBusyError if called when not acquired", async function () {
+			await assert.rejects(lock.release(), { name: "LockFileBusyError" });
+		});
+	});
+
+	describe("isHeld()", function () {
+		it("should return true only when lock file contains current PID", async function () {
+			await lock.acquire();
+			const held = await lock.isHeld();
+			assert.equal(held, true);
+		});
+
+		it("should return false when lock file does not exist", async function () {
+			const held = await lock.isHeld();
+			assert.equal(held, false);
+		});
+
+		it("should return false if lock file contains different PID", async function () {
+			await fs.writeFile(filePath, "999999\n"); // simulate another process
+			const held = await lock.isHeld();
+			assert.equal(held, false);
+		});
+	});
+
+	describe("isStale()", function () {
+		it("should return true if lock file does not exist", async function () {
+			const stale = await lock.isStale();
+			assert.equal(stale, true);
+		});
+
+		it("should return true for invalid PID in lock file", async function () {
+			await fs.writeFile(filePath, "abc\n"); // non-numeric PID
+			const stale = await lock.isStale();
+			assert.equal(stale, true);
+		});
+
+		it("should return true for non-existent process PID", async function () {
+			await fs.writeFile(filePath, "999999\n"); // assume PID does not exist
+			const stale = await lock.isStale();
+			assert.equal(stale, true);
+		});
+
+		it("should return false for current process PID", async function () {
+			await fs.writeFile(filePath, `${process.pid}\n`);
+			const stale = await lock.isStale();
+			assert.equal(stale, false);
+		});
+	});
+
+	describe("attach/detach exit handlers", function () {
+		it("should attach exit handler on acquire and detach on release", async function () {
+			await lock.acquire();
+			const listenersBefore = process.listeners("exit").slice();
+			assert.ok(listenersBefore.includes(lock.handleExitSync));
+
+			await lock.release();
+			const listenersAfter = process.listeners("exit").slice();
+			assert.ok(!listenersAfter.includes(lock.handleExitSync));
+		});
+	});
+
+	describe("handleExitSync()", function () {
+		it("should not throw if file already removed", function () {
+			assert.doesNotThrow(() => lock.handleExitSync()); // simulate non-existent file
+		});
+	});
+});