Skip to content

Commit 53d6e5b

Browse files
committed
feat(v7-i03): synchronous trainInFlight lock closes H22 race window
Closes V7-I03 (cppmega-mlx-pi1a): H22 disabled the Train button via the trainInFlight React state, but React commits state updates asynchronously so a 0ms-delay double click could pass the prop-disabled check on both invocations and fire pipeline.run twice. Fix: trainInFlightLockRef (useRef<boolean>) mutated SYNCHRONOUSLY at the top of handleRunPipeline before any await. The second microtask click reads the ref already true and returns early. Released in the finally block so legitimate sequential trains work normally. Tests: - vbgui e2e 79_concurrent_train_race.spec.ts: zero-delay double click via page.evaluate(() => { btn.click(); btn.click(); }) → asserts exactly 1 pipeline.run HTTP POST. 1/1 passing. - vbgui e2e concurrency regression (H03 + H22 + V7-I03): 3/3 passing — no regression in cancel or button-disabled flows.
1 parent 57ba5b9 commit 53d6e5b

2 files changed

Lines changed: 58 additions & 0 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// V7-I03: tightens H22 with a synchronous lock. Two rapid Train
2+
// clicks dispatched in the same microtask (no artificial delay)
3+
// must result in only ONE pipeline.run on the backend.
4+
5+
import { test, expect } from "@playwright/test";
6+
import { gotoApp, selectPreset, closeModal } from "../fixtures";
7+
8+
test("V7-I03: zero-delay double Train click → single pipeline.run",
9+
async ({ page }) => {
10+
test.setTimeout(60_000);
11+
12+
// Count pipeline.run HTTP requests across the test.
13+
let pipelineRunCount = 0;
14+
page.on("request", (req) => {
15+
if (!req.url().endsWith("/rpc") || req.method() !== "POST") return;
16+
try {
17+
const body = JSON.parse(req.postData() ?? "{}");
18+
if (body.method === "pipeline.run") pipelineRunCount += 1;
19+
} catch { /* ignore */ }
20+
});
21+
22+
await gotoApp(page);
23+
await selectPreset(page, "llama3_8b");
24+
await page.getByTestId("run-pipeline-toggle").click();
25+
await page.getByTestId("train-num-steps").fill("2");
26+
// Dispatch two clicks in the SAME microtask via JS so neither
27+
// sees a React render before the second fires.
28+
await page.evaluate(() => {
29+
const btn = document.querySelector<HTMLButtonElement>(
30+
"[data-testid='run-pipeline-train']")!;
31+
btn.click();
32+
btn.click();
33+
});
34+
35+
// Wait for the modal of the (one and only) pipeline run.
36+
await page.getByTestId("run-result-modal").waitFor({ timeout: 60_000 });
37+
await closeModal(page);
38+
39+
// Allow a moment for any straggler request to land.
40+
await page.waitForTimeout(200);
41+
expect(pipelineRunCount).toBe(1);
42+
});

vbgui/src/App.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,12 @@ export function App(): JSX.Element {
107107
const [runReport, setRunReport] = useState<RunReport | null>(null);
108108
const [runError, setRunError] = useState<string | null>(null);
109109
const [trainInFlight, setTrainInFlight] = useState(false);
110+
// V7-I03: synchronous lock. React's setTrainInFlight schedules an
111+
// async commit, so two button clicks within the same microtask
112+
// both read trainInFlight=false and both call rpc.call. The ref
113+
// is mutated synchronously inside handleRunPipeline before any
114+
// await, closing the ~10ms window.
115+
const trainInFlightLockRef = useRef<boolean>(false);
110116
const [trainRunId, setTrainRunId] = useState<string | null>(null);
111117
// H04: most recent successfully-completed Train run_id, used as
112118
// continue_from_run_id when the warm-start checkbox is on. Cleared
@@ -371,8 +377,17 @@ export function App(): JSX.Element {
371377
master_dtype?: "fp32" | "bf16" | "fp16" | "auto";
372378
},
373379
) => {
380+
// V7-I03: synchronous lock check at the very top — before any
381+
// await, before the canvas-empty guard. Two rapid clicks both
382+
// pass through React's stale-prop trainInFlight=false but only
383+
// the first acquires the ref.
384+
if (mode === "train") {
385+
if (trainInFlightLockRef.current) return;
386+
trainInFlightLockRef.current = true;
387+
}
374388
const snap = wireSpecRef.current;
375389
if (snap.nodes.length === 0) {
390+
if (mode === "train") trainInFlightLockRef.current = false;
376391
setRunError("canvas is empty — drop bricks or pick a preset first");
377392
setRunReport(null);
378393
return;
@@ -471,6 +486,7 @@ export function App(): JSX.Element {
471486
if (mode === "train") {
472487
setTrainInFlight(false);
473488
setTrainRunId(null);
489+
trainInFlightLockRef.current = false;
474490
}
475491
}
476492
}, [rpc, trainParquetPath, trainSideChannels, trainTokenizerPath,

0 commit comments

Comments
 (0)