Skip to content

Commit aacf840

Browse files
borsbors
committed
Auto merge of rust-lang#18281 - darichey:async-subprocess, r=lnicola
Run subprocesses async in vscode extension Extensions should not block the vscode extension host. Replace uses of `spawnSync` with `spawnAsync`, a convenience wrapper around `spawn`. These `spawnSync`s are unlikely to cause a real issue in practice, because they spawn very short-lived processes, so we aren't blocking for very long. That said, blocking the extension host is poor practice, and if they _do_ block for too long for whatever reason, vscode becomes useless.
2 parents 7562b17 + 0460ff7 commit aacf840

File tree

3 files changed

+83
-32
lines changed

3 files changed

+83
-32
lines changed

‎src/tools/rust-analyzer/editors/code/src/bootstrap.ts‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import * as vscode from "vscode";
22
import * as os from "os";
33
import type { Config } from "./config";
4-
import { type Env, log } from "./util";
4+
import { type Env, log, spawnAsync } from "./util";
55
import type { PersistentState } from "./persistent_state";
6-
import { exec, spawnSync } from "child_process";
6+
import { exec } from "child_process";
77
import { TextDecoder } from "node:util";
88

99
export async function bootstrap(
@@ -61,13 +61,12 @@ async function getServer(
6161
// if so, use the rust-analyzer component
6262
const toolchainUri = vscode.Uri.joinPath(workspaceFolder.uri, "rust-toolchain.toml");
6363
if (await hasToolchainFileWithRaDeclared(toolchainUri)) {
64-
const res = spawnSync("rustup", ["which", "rust-analyzer"], {
65-
encoding: "utf8",
64+
const res = await spawnAsync("rustup", ["which", "rust-analyzer"], {
6665
env: { ...process.env },
6766
cwd: workspaceFolder.uri.fsPath,
6867
});
6968
if (!res.error && res.status === 0) {
70-
toolchainServerPath = earliestToolchainPath(
69+
toolchainServerPath = await earliestToolchainPath(
7170
toolchainServerPath,
7271
res.stdout.trim(),
7372
raVersionResolver,
@@ -114,10 +113,8 @@ async function getServer(
114113
}
115114

116115
// Given a path to a rust-analyzer executable, resolve its version and return it.
117-
function raVersionResolver(path: string): string | undefined {
118-
const res = spawnSync(path, ["--version"], {
119-
encoding: "utf8",
120-
});
116+
async function raVersionResolver(path: string): Promise<string | undefined> {
117+
const res = await spawnAsync(path, ["--version"]);
121118
if (!res.error && res.status === 0) {
122119
return res.stdout;
123120
} else {
@@ -126,13 +123,16 @@ function raVersionResolver(path: string): string | undefined {
126123
}
127124

128125
// Given a path to two rust-analyzer executables, return the earliest one by date.
129-
function earliestToolchainPath(
126+
async function earliestToolchainPath(
130127
path0: string | undefined,
131128
path1: string,
132-
raVersionResolver: (path: string) => string | undefined,
133-
): string {
129+
raVersionResolver: (path: string) => Promise<string | undefined>,
130+
): Promise<string> {
134131
if (path0) {
135-
if (orderFromPath(path0, raVersionResolver) < orderFromPath(path1, raVersionResolver)) {
132+
if (
133+
(await orderFromPath(path0, raVersionResolver)) <
134+
(await orderFromPath(path1, raVersionResolver))
135+
) {
136136
return path0;
137137
} else {
138138
return path1;
@@ -150,11 +150,11 @@ function earliestToolchainPath(
150150
// nightly - /Users/myuser/.rustup/toolchains/nightly-2022-11-22-aarch64-apple-darwin/bin/rust-analyzer
151151
// versioned - /Users/myuser/.rustup/toolchains/1.72.1-aarch64-apple-darwin/bin/rust-analyzer
152152
// stable - /Users/myuser/.rustup/toolchains/stable-aarch64-apple-darwin/bin/rust-analyzer
153-
function orderFromPath(
153+
async function orderFromPath(
154154
path: string,
155-
raVersionResolver: (path: string) => string | undefined,
156-
): string {
157-
const raVersion = raVersionResolver(path);
155+
raVersionResolver: (path: string) => Promise<string | undefined>,
156+
): Promise<string> {
157+
const raVersion = await raVersionResolver(path);
158158
const raDate = raVersion?.match(/^rust-analyzer .*\(.* (\d{4}-\d{2}-\d{2})\)$/);
159159
if (raDate?.length === 2) {
160160
const precedence = path.includes("nightly-") ? "0" : "1";
@@ -184,11 +184,10 @@ async function hasToolchainFileWithRaDeclared(uri: vscode.Uri): Promise<boolean>
184184
}
185185
}
186186

187-
export function isValidExecutable(path: string, extraEnv: Env): boolean {
187+
export async function isValidExecutable(path: string, extraEnv: Env): Promise<boolean> {
188188
log.debug("Checking availability of a binary at", path);
189189

190-
const res = spawnSync(path, ["--version"], {
191-
encoding: "utf8",
190+
const res = await spawnAsync(path, ["--version"], {
192191
env: { ...process.env, ...extraEnv },
193192
});
194193

‎src/tools/rust-analyzer/editors/code/src/util.ts‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as vscode from "vscode";
22
import { strict as nativeAssert } from "assert";
3-
import { exec, type ExecOptions } from "child_process";
3+
import { exec, spawn, type SpawnOptionsWithoutStdio, type ExecOptions } from "child_process";
44
import { inspect } from "util";
55
import type { CargoRunnableArgs, ShellRunnableArgs } from "./lsp_ext";
66

@@ -233,3 +233,55 @@ export function expectNotUndefined<T>(input: Undefinable<T>, msg: string): NotUn
233233
export function unwrapUndefinable<T>(input: Undefinable<T>): NotUndefined<T> {
234234
return expectNotUndefined(input, `unwrapping \`undefined\``);
235235
}
236+
237+
interface SpawnAsyncReturns {
238+
stdout: string;
239+
stderr: string;
240+
status: number | null;
241+
error?: Error | undefined;
242+
}
243+
244+
export async function spawnAsync(
245+
path: string,
246+
args?: ReadonlyArray<string>,
247+
options?: SpawnOptionsWithoutStdio,
248+
): Promise<SpawnAsyncReturns> {
249+
const child = spawn(path, args, options);
250+
const stdout: Array<Buffer> = [];
251+
const stderr: Array<Buffer> = [];
252+
try {
253+
const res = await new Promise<{ stdout: string; stderr: string; status: number | null }>(
254+
(resolve, reject) => {
255+
child.stdout.on("data", (chunk) => stdout.push(Buffer.from(chunk)));
256+
child.stderr.on("data", (chunk) => stderr.push(Buffer.from(chunk)));
257+
child.on("error", (error) =>
258+
reject({
259+
stdout: Buffer.concat(stdout).toString("utf8"),
260+
stderr: Buffer.concat(stderr).toString("utf8"),
261+
error,
262+
}),
263+
);
264+
child.on("close", (status) =>
265+
resolve({
266+
stdout: Buffer.concat(stdout).toString("utf8"),
267+
stderr: Buffer.concat(stderr).toString("utf8"),
268+
status,
269+
}),
270+
);
271+
},
272+
);
273+
274+
return {
275+
stdout: res.stdout,
276+
stderr: res.stderr,
277+
status: res.status,
278+
};
279+
} catch (e: any) {
280+
return {
281+
stdout: e.stdout,
282+
stderr: e.stderr,
283+
status: e.status,
284+
error: e.error,
285+
};
286+
}
287+
}

‎src/tools/rust-analyzer/editors/code/tests/unit/bootstrap.test.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ export async function getTests(ctx: Context) {
66
await ctx.suite("Bootstrap/Select toolchain RA", (suite) => {
77
suite.addTest("Order of nightly RA", async () => {
88
assert.deepStrictEqual(
9-
_private.orderFromPath(
9+
await _private.orderFromPath(
1010
"/Users/myuser/.rustup/toolchains/nightly-2022-11-22-aarch64-apple-darwin/bin/rust-analyzer",
11-
function (path: string) {
11+
async function (path: string) {
1212
assert.deepStrictEqual(
1313
path,
1414
"/Users/myuser/.rustup/toolchains/nightly-2022-11-22-aarch64-apple-darwin/bin/rust-analyzer",
@@ -22,9 +22,9 @@ export async function getTests(ctx: Context) {
2222

2323
suite.addTest("Order of versioned RA", async () => {
2424
assert.deepStrictEqual(
25-
_private.orderFromPath(
25+
await _private.orderFromPath(
2626
"/Users/myuser/.rustup/toolchains/1.72.1-aarch64-apple-darwin/bin/rust-analyzer",
27-
function (path: string) {
27+
async function (path: string) {
2828
assert.deepStrictEqual(
2929
path,
3030
"/Users/myuser/.rustup/toolchains/1.72.1-aarch64-apple-darwin/bin/rust-analyzer",
@@ -38,9 +38,9 @@ export async function getTests(ctx: Context) {
3838

3939
suite.addTest("Order of versioned RA when unable to obtain version date", async () => {
4040
assert.deepStrictEqual(
41-
_private.orderFromPath(
41+
await _private.orderFromPath(
4242
"/Users/myuser/.rustup/toolchains/1.72.1-aarch64-apple-darwin/bin/rust-analyzer",
43-
function () {
43+
async function () {
4444
return "rust-analyzer 1.72.1";
4545
},
4646
),
@@ -50,9 +50,9 @@ export async function getTests(ctx: Context) {
5050

5151
suite.addTest("Order of stable RA", async () => {
5252
assert.deepStrictEqual(
53-
_private.orderFromPath(
53+
await _private.orderFromPath(
5454
"/Users/myuser/.rustup/toolchains/stable-aarch64-apple-darwin/bin/rust-analyzer",
55-
function (path: string) {
55+
async function (path: string) {
5656
assert.deepStrictEqual(
5757
path,
5858
"/Users/myuser/.rustup/toolchains/stable-aarch64-apple-darwin/bin/rust-analyzer",
@@ -66,7 +66,7 @@ export async function getTests(ctx: Context) {
6666

6767
suite.addTest("Order with invalid path to RA", async () => {
6868
assert.deepStrictEqual(
69-
_private.orderFromPath("some-weird-path", function () {
69+
await _private.orderFromPath("some-weird-path", async function () {
7070
return undefined;
7171
}),
7272
"2",
@@ -75,10 +75,10 @@ export async function getTests(ctx: Context) {
7575

7676
suite.addTest("Earliest RA between nightly and stable", async () => {
7777
assert.deepStrictEqual(
78-
_private.earliestToolchainPath(
78+
await _private.earliestToolchainPath(
7979
"/Users/myuser/.rustup/toolchains/stable-aarch64-apple-darwin/bin/rust-analyzer",
8080
"/Users/myuser/.rustup/toolchains/nightly-2022-11-22-aarch64-apple-darwin/bin/rust-analyzer",
81-
function (path: string) {
81+
async function (path: string) {
8282
if (
8383
path ===
8484
"/Users/myuser/.rustup/toolchains/nightly-2022-11-22-aarch64-apple-darwin/bin/rust-analyzer"

0 commit comments

Comments
 (0)