Skip to content

Commit 00f67c4

Browse files
author
wellwei
committed
fix: narrow serial executor reentrancy detection and fix xlings PATH check
- createSerialExecutor: use AsyncLocalStorage-scoped flag instead of global depth counter so only same-async-context reentrant calls bypass the queue - findXlingsExecutable: check PATH entries with existsSync before returning 'xlings', so the wizard's 'install xlings' guidance branch is reachable - add 2 tests: concurrent caller queues correctly, reentrant call bypasses
1 parent deacebd commit 00f67c4

3 files changed

Lines changed: 120 additions & 7 deletions

File tree

src/llvmTools.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,28 @@ export function findXlingsExecutable(): string | undefined {
7979
}
8080
}
8181

82-
// Fall back to PATH — execFile will resolve it
83-
return "xlings";
82+
// Fall back to PATH, but only when "xlings" actually resolves there. Always
83+
// returning "xlings" hid the not-installed case, so callers could never show
84+
// the "xlings 未安装" guidance.
85+
return xlingsResolvableOnPath() ? "xlings" : undefined;
86+
}
87+
88+
function xlingsResolvableOnPath(): boolean {
89+
const names = process.platform === "win32"
90+
? ["xlings.exe", "xlings.cmd", "xlings.bat"]
91+
: ["xlings"];
92+
const pathValue = process.env.PATH ?? "";
93+
for (const dir of pathValue.split(path.delimiter)) {
94+
if (dir.length === 0) {
95+
continue;
96+
}
97+
for (const name of names) {
98+
if (existsSync(path.join(dir, name))) {
99+
return true;
100+
}
101+
}
102+
}
103+
return false;
84104
}
85105

86106
export async function runXlingsCommand(

src/workflow.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { AsyncLocalStorage } from "node:async_hooks";
2+
13
import type { CheckResult, ModuleCapability } from "./analysis";
24

35
export type RefreshOutcomeLevel = "information" | "warning" | "error";
@@ -102,19 +104,30 @@ export function createKeyedSingleFlightReconciler<K, T>(
102104

103105
export function createSerialExecutor(): <T>(operation: () => Promise<T>) => Promise<T> {
104106
let tail: Promise<void> = Promise.resolve();
105-
let depth = 0;
107+
// `running` is scoped to the async context of the in-flight operation via
108+
// AsyncLocalStorage instead of a global counter, so only genuinely reentrant
109+
// calls bypass the queue:
110+
// - a call made from inside an operation's own async chain (e.g.
111+
// wizard → build → reconcile → executor) sees `running` and executes
112+
// immediately — the outer operation is awaiting it, so queuing would hang;
113+
// - an unrelated concurrent caller (a separate command/event that has its
114+
// own async context) does not see it and queues behind the in-flight
115+
// operation, which the previous `depth > 0` check would wrongly let run
116+
// concurrently.
117+
const state = { running: false };
118+
const storage = new AsyncLocalStorage<{ running: boolean }>();
106119

107120
return <T>(operation: () => Promise<T>): Promise<T> => {
108-
if (depth > 0) {
121+
if (storage.getStore() === state && state.running) {
109122
return operation();
110123
}
111124

112125
const wrapped = async (): Promise<T> => {
113-
depth += 1;
126+
state.running = true;
114127
try {
115-
return await operation();
128+
return await storage.run(state, operation);
116129
} finally {
117-
depth -= 1;
130+
state.running = false;
118131
}
119132
};
120133

test/workflow.test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import test from "node:test";
33

44
import * as workflow from "../src/workflow";
55
import {
6+
createSerialExecutor,
67
createSingleFlightReconciler,
78
describeRefreshOutcome,
89
statusCommandForCapability,
@@ -132,6 +133,85 @@ test("serializes operations that share the workspace clangd configuration", asyn
132133
assert.deepEqual(calls, ["a:start", "a:end", "b:start", "b:end"]);
133134
});
134135

136+
test("queues an unrelated concurrent caller behind an in-flight operation", async () => {
137+
let releaseFirst: (() => void) | undefined;
138+
const firstGate = new Promise<void>((resolve) => {
139+
releaseFirst = resolve;
140+
});
141+
const calls: string[] = [];
142+
let maxConcurrent = 0;
143+
let active = 0;
144+
const execute = createSerialExecutor();
145+
146+
const first = execute(async () => {
147+
active += 1;
148+
maxConcurrent = Math.max(maxConcurrent, active);
149+
calls.push("a:start");
150+
await firstGate;
151+
calls.push("a:end");
152+
active -= 1;
153+
return "a";
154+
});
155+
156+
// A second, unrelated invocation arrives while the first is still in flight,
157+
// from a separate async context (a macrotask, like a user command event).
158+
let second: Promise<string> | undefined;
159+
const secondArrived = new Promise<void>((resolve) => {
160+
setTimeout(() => {
161+
second = execute(async () => {
162+
active += 1;
163+
maxConcurrent = Math.max(maxConcurrent, active);
164+
calls.push("b:start");
165+
calls.push("b:end");
166+
active -= 1;
167+
return "b";
168+
});
169+
resolve();
170+
}, 10);
171+
});
172+
await secondArrived;
173+
174+
assert.deepEqual(calls, ["a:start"]);
175+
assert.ok(releaseFirst);
176+
releaseFirst();
177+
assert.ok(second);
178+
assert.deepEqual(await Promise.all([first, second]), ["a", "b"]);
179+
assert.deepEqual(calls, ["a:start", "a:end", "b:start", "b:end"]);
180+
assert.equal(maxConcurrent, 1);
181+
});
182+
183+
test("runs a reentrant call from inside an operation without queuing", async () => {
184+
const calls: string[] = [];
185+
let releaseInner: (() => void) | undefined;
186+
const innerGate = new Promise<void>((resolve) => {
187+
releaseInner = resolve;
188+
});
189+
const execute = createSerialExecutor();
190+
191+
const outer = execute(async () => {
192+
calls.push("outer:start");
193+
await Promise.resolve();
194+
const inner = execute(async () => {
195+
calls.push("inner:start");
196+
await innerGate;
197+
calls.push("inner:end");
198+
return "inner";
199+
});
200+
calls.push("outer:mid");
201+
const value = await inner;
202+
calls.push("outer:end");
203+
return value;
204+
});
205+
206+
// The outer operation's continuation (which makes the reentrant call) runs in
207+
// the microtask queue, before the timer below fires.
208+
await new Promise<void>((resolve) => setTimeout(resolve, 0));
209+
assert.ok(releaseInner);
210+
releaseInner();
211+
assert.equal(await outer, "inner");
212+
assert.deepEqual(calls, ["outer:start", "inner:start", "outer:mid", "inner:end", "outer:end"]);
213+
});
214+
135215
test("retries a queued reconciliation after a transient failure", async () => {
136216
let releaseFirst: (() => void) | undefined;
137217
const firstGate = new Promise<void>((resolve) => {

0 commit comments

Comments
 (0)