Skip to content

Commit

Permalink
fix: ensure wing process is killed from vscode (#4124)
Browse files Browse the repository at this point in the history
Fixes #4119

Since wing itself is being run in a child process of vscode, it wasn't handling the kill signal. Detaching it allows it to have its own process tree and be killed more reliably. Special handling is needed for windows, of course.

This also ensures that any process spawned by `wing it` itself is terminated too.

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
MarkMcCulloh authored Sep 11, 2023
1 parent 0be5495 commit d0b321c
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 15 deletions.
31 changes: 27 additions & 4 deletions apps/vscode-wing/src/console/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { spawn } from "child_process";
import { execSync, spawn } from "child_process";
import path from "path";
import fetch from "node-fetch";

Expand Down Expand Up @@ -78,9 +78,29 @@ export class WingConsoleManager {
stdio: ["ignore", "pipe", "pipe"],
windowsHide: true,
shell: false,
detached: process.platform !== "win32",
});

const onDidClose = () => {
if (process.platform === "win32") {
try {
execSync(`taskkill /pid ${cp.pid} /T /F`);
} catch (error) {
this.logger.appendLine(
`Failed to kill the process with taskkill: ${error}`
);

cp.kill();
}
} else {
if (cp.pid) {
process.kill(-cp.pid, "SIGTERM");
}
}
};

cp.on("error", async (err) => {
this.logger.appendLine(err.toString());
if (err) {
this.consoleManager.closeInstance(wingfilePath);
}
Expand All @@ -99,11 +119,14 @@ export class WingConsoleManager {
wingfile,
url,
client: createClient(url),
onDidClose: () => {
cp.kill();
},
onDidClose,
});
});
cp.stdout?.on("data", (data) => {
this.logger.append(data.toString());
});

process.once("exit", onDidClose);
}

public async openFile() {
Expand Down
3 changes: 1 addition & 2 deletions apps/wing-console/console/server/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ export const createConsoleServer = async ({
updater?.removeEventListener("status-change", invalidateUpdaterStatus);
config?.removeEventListener("config-change", invalidateConfig);
await Promise.allSettled([
server.closeAllConnections(),
server.close(),
compiler.stop(),
simulator.stop(),
Expand All @@ -257,8 +258,6 @@ export const createConsoleServer = async ({
}
};

process.on("SIGINT", () => close(() => process.exit(0)));

return {
port,
close,
Expand Down
8 changes: 4 additions & 4 deletions apps/wing/src/commands/run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ test("wing it runs the only .w file", async () => {
wingfile: resolve("foo.w"),
requestedPort: 3000,
hostUtils: expect.anything(),
requireAcceptTerms: true,
requireAcceptTerms: false,
});
expect(open).toBeCalledWith("http://localhost:3000/");
} finally {
Expand Down Expand Up @@ -69,7 +69,7 @@ test("wing it with a file runs", async () => {
wingfile: resolve("foo.w"),
requestedPort: 3000,
hostUtils: expect.anything(),
requireAcceptTerms: true,
requireAcceptTerms: false,
});
expect(open).toBeCalledWith("http://localhost:3000/");
} finally {
Expand All @@ -93,7 +93,7 @@ test("wing it with a nested file runs", async () => {
wingfile: resolve(filePath),
requestedPort: 3000,
hostUtils: expect.anything(),
requireAcceptTerms: true,
requireAcceptTerms: false,
});
expect(open).toBeCalledWith("http://localhost:3000/");
} finally {
Expand Down Expand Up @@ -128,7 +128,7 @@ test("wing it with a custom port runs", async () => {
wingfile: resolve("foo.w"),
requestedPort: 5000,
hostUtils: expect.anything(),
requireAcceptTerms: true,
requireAcceptTerms: false,
});
expect(open).toBeCalledWith("http://localhost:5000/");
} finally {
Expand Down
14 changes: 11 additions & 3 deletions apps/wing/src/commands/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,27 @@ export async function run(entrypoint?: string, options?: RunOptions) {
entrypoint = resolve(entrypoint);
debug("opening the wing console with:" + entrypoint);

const { port } = await createConsoleApp({
const { port, close } = await createConsoleApp({
wingfile: entrypoint,
requestedPort,
hostUtils: {
async openExternal(url) {
async openExternal(url: string) {
await open(url);
},
},
requireAcceptTerms: true,
requireAcceptTerms: !!process.stdin.isTTY,
});
const url = `http://localhost:${port}/`;
if (openBrowser) {
await open(url);
}
console.log(`The Wing Console is running at ${url}`);

const onExit = async (exitCode: number) => {
await close(() => process.exit(exitCode));
};

process.once("exit", async (c) => await onExit(c));
process.once("SIGTERM", async () => await onExit(0));
process.once("SIGINT", async () => await onExit(0));
}
4 changes: 2 additions & 2 deletions libs/wingsdk/src/.gen/versions.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"registry.terraform.io/hashicorp/aws": "4.65.0",
"registry.terraform.io/hashicorp/random": "3.5.1",
"registry.terraform.io/hashicorp/google": "4.63.1",
"registry.terraform.io/hashicorp/azurerm": "3.54.0",
"registry.terraform.io/hashicorp/aws": "4.65.0"
"registry.terraform.io/hashicorp/google": "4.63.1"
}

0 comments on commit d0b321c

Please sign in to comment.