From eb1e2e2a5e4510e71a02f49ec400ffed7f24d0fd Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Thu, 29 Oct 2026 22:52:09 +0000 Subject: [PATCH] Wait for a start in progress before stopping an instance A stop request sent while the instance was starting ran alongside the start. During save preparation it did nothing and the instance came up running anyway. Once Factorio was launched it put the server into the stopping state, the start then failed with "Expected state running but state is stopping" and its cleanup killed the server mid shutdown. Stop and restart requests now wait for any start, stop or save in progress to finish first, then stop the instance if it is running. Co-Authored-By: Claude Opus 5.5 --- packages/host/src/Instance.ts | 14 ++++++++++-- test/host/Instance.js | 40 +++++++++++++++++++++++++++++++++++ test/integration/instance.js | 22 +++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index f70080d3..6cee6a7b 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -1308,6 +1308,16 @@ end`.replace(/\r?\n/g, " "); } } + /** Stop the instance once a start, stop or save in progress has finished */ + async _stopWhenSettled() { + // Stopping concurrently with these races them and asserts on the server state. + // Checked right before stop() as another waiter may have started stopping. + while (transitoryStatuses.has(this._status)) { + await this._waitForSettledStatus(); + } + await this.stop(); + } + async kill() { if (this._status === "stopped") { return; @@ -1494,7 +1504,7 @@ end`.replace(/\r?\n/g, " "); } async handleInstanceRestartRequest(request: lib.InstanceRestartRequest) { - await this.stop(); + await this._stopWhenSettled(); // Stopping invalidates this instance object, a new one must be created by the host // To do this we simulate a waking an instance and sending a start request const instanceConnection = await this._host._connectInstance(this.id); @@ -1502,7 +1512,7 @@ end`.replace(/\r?\n/g, " "); } async handleInstanceStopRequest() { - await this.stop(); + await this._stopWhenSettled(); } async handleInstanceKillRequest() { diff --git a/test/host/Instance.js b/test/host/Instance.js index 0b106b78..80cca616 100644 --- a/test/host/Instance.js +++ b/test/host/Instance.js @@ -268,6 +268,46 @@ describe("class Instance", function() { }); }); + describe(".handleInstanceStopRequest()", function() { + let calls; + beforeEach(function() { + calls = []; + instance.server._state = "running"; + instance.server.stop = async () => { calls.push("stop"); instance.server._state = "stopping"; }; + instance.server.kill = async () => { calls.push("kill"); }; + instance.sendSaveListUpdate = async () => {}; + }); + + it("should stop once a start in progress finishes", async function() { + instance.notifyStatus("starting"); + let stopped = instance.handleInstanceStopRequest(); + await wait(10); + assert.deepEqual(calls, [], "stopped while starting"); + instance.notifyStatus("running"); + await stopped; + assert.deepEqual(calls, ["stop"]); + }); + + it("should do nothing if a start in progress fails", async function() { + instance.notifyStatus("starting"); + let stopped = instance.handleInstanceStopRequest(); + await wait(10); + instance.notifyStatus("stopped"); + await stopped; + assert.deepEqual(calls, []); + }); + + it("should not kill the server when stopped while stopping", async function() { + instance.notifyStatus("running"); + let first = instance.handleInstanceStopRequest(); + let second = instance.handleInstanceStopRequest(); + await first; + instance.notifyStatus("stopped"); + await second; + assert.deepEqual(calls, ["stop"]); + }); + }); + describe(".checkModPackVersion()", function() { function modPack(factorioVersion) { return lib.ModPack.fromJSON({ name: "pack", factorio_version: factorioVersion }); diff --git a/test/integration/instance.js b/test/integration/instance.js index 25bdf4f0..c12bee72 100644 --- a/test/integration/instance.js +++ b/test/integration/instance.js @@ -474,4 +474,26 @@ describe("start-all and stop-all commands", function() { assert.equal(await getInstanceStatus(testInstId3), "stopped"); }); }); + + describe("stop while starting", function() { + it("stops the instance once it has started", async function() { + this.timeout(30000); + const control = getControl(); + const started = control.sendTo({ instanceId: testInstId1 }, new lib.InstanceStartRequest()); + // Unroutable until the host has connected the instance + for (;;) { + try { + await control.sendTo({ instanceId: testInstId1 }, new lib.InstanceStopRequest()); + break; + } catch (err) { + if (!(err instanceof lib.UnavailableError)) { + throw err; + } + await lib.wait(10); + } + } + await started; + assert.equal(await getInstanceStatus(testInstId1), "stopped"); + }); + }); });