From bccb6527a711c45d519a032ca1c634b455548272 Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Wed, 7 Jan 2026 01:20:40 +0000 Subject: [PATCH] Add lock file for configs --- packages/controller/controller.ts | 14 ++- packages/ctl/ctl.ts | 3 +- packages/host/host.ts | 6 +- packages/lib/index.ts | 1 + packages/lib/src/LockFile.ts | 163 ++++++++++++++++++++++++++++ packages/lib/src/shared_commands.ts | 8 +- test/integration/clusterio.js | 41 ++++++- test/integration/index.js | 17 +-- test/lib/LockFile.test.js | 126 +++++++++++++++++++++ 9 files changed, 361 insertions(+), 18 deletions(-) create mode 100644 packages/lib/src/LockFile.ts create mode 100644 test/lib/LockFile.test.js diff --git a/packages/controller/controller.ts b/packages/controller/controller.ts index c83b3d30..3eabb814 100644 --- a/packages/controller/controller.ts +++ b/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 { 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 { 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 { } 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 { diff --git a/packages/ctl/ctl.ts b/packages/ctl/ctl.ts index bafb43af..6d2816f1 100755 --- a/packages/ctl/ctl.ts +++ b/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 }; } diff --git a/packages/host/host.ts b/packages/host/host.ts index 6a3b1a3f..259fb31c 100755 --- a/packages/host/host.ts +++ b/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")); diff --git a/packages/lib/index.ts b/packages/lib/index.ts index 36e0d492..204cc7d3 100644 --- a/packages/lib/index.ts +++ b/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"; diff --git a/packages/lib/src/LockFile.ts b/packages/lib/src/LockFile.ts new file mode 100644 index 00000000..5cb6ecce --- /dev/null +++ b/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 { + 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 { + 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 { + 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 { + 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 { + 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 => { + 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 + } + }; +} diff --git a/packages/lib/src/shared_commands.ts b/packages/lib/src/shared_commands.ts index e65f925f..d9f6eb09 100644 --- a/packages/lib/src/shared_commands.ts +++ b/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, instance: libConfig.Config, + 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(); } } diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index 948dd556..8532b57a 100644 --- a/test/integration/clusterio.js +++ b/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() { diff --git a/test/integration/index.js b/test/integration/index.js index 529b0c08..7d507040 100644 --- a/test/integration/index.js +++ b/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, diff --git a/test/lib/LockFile.test.js b/test/lib/LockFile.test.js new file mode 100644 index 00000000..7034ecaf --- /dev/null +++ b/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 + }); + }); +});