Merge pull request #1151 from bbassie/fix/stop-while-starting

Fix stopping an instance while it is starting

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() {

packages/web_ui/src/components/InstanceControlButton.tsx

@@ -25,8 +25,10 @@ export default function InstanceControlButton(props: InstanceControlButtonProps)
 
 	const instanceId = props.instance.id;
 	const instanceStatus = props.instance.status;
-	const showStart = ["stopped", "stopping"].includes(instanceStatus);
-	const showStop = ["running", "starting"].includes(instanceStatus);
+	// While starting or stopping the button shows the action in progress as loading.
+	const showStart = ["stopped", "starting"].includes(instanceStatus);
+	const showStop = ["running", "stopping"].includes(instanceStatus);
+	const transitioning = ["starting", "stopping"].includes(instanceStatus);
 
 	function actionFinish() {
 		setSwitching(false);
@@ -37,7 +39,7 @@ export default function InstanceControlButton(props: InstanceControlButtonProps)
 
 	const actions: VariableDropdownButtonProps["actions"] = [];
 
-	if (account.hasPermission("core.instance.start") && showStart !== switching) {
+	if (account.hasPermission("core.instance.start") && showStart) {
 		actions.push({
 			key: "start",
 			label: "Start",
@@ -101,7 +103,7 @@ export default function InstanceControlButton(props: InstanceControlButtonProps)
 			{...props}
 			type="primary"
 			actions={actions}
-			loading={switching}
+			loading={switching || transitioning}
 			disabled={!(showStart || showStop)}
 		/>
 		<Popconfirm

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 });

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");
+		});
+	});
 });