Phase F fix: open-lane token was lost across the dispatch-context swap — mint it in JS; repro spec added
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Td4ujboGuAw26jvbDQzehj
This commit is contained in:
parent
747bf5ecf4
commit
1d8237bc08
5 changed files with 180 additions and 10 deletions
|
|
@ -1 +1 @@
|
|||
16
|
||||
17
|
||||
|
|
|
|||
|
|
@ -1267,6 +1267,33 @@ starter pattern) and the wx TEST HARNESS pages' direct `ccall` buttons
|
|||
(harness-only; the interlock and shim capture/restore stay until those are
|
||||
routed or retired).
|
||||
|
||||
### Open-lane REGRESSION found in live use + fixed (2026-08-09)
|
||||
|
||||
The user hit it immediately on the live editor: pcbnew dialogs froze / blue-
|
||||
screened (3D viewer, footprint browser X-close, Add-Footprint OK/Cancel) while
|
||||
eeschema was fine. Every existing board-load spec drives **File → Open**
|
||||
(`OpenProjectFiles` directly); the web shell drives **`Module.kicadOpenFile`**,
|
||||
the wrapped path — so no gate exercised it. New repro `shell-open-dialogs.spec.ts`
|
||||
loads via `Module.kicadOpenFile` and caught it: **`kicadOpenFile` resolved
|
||||
`false` and the shim logged `waitPromise(0): unknown token`.**
|
||||
|
||||
**Root cause: the token was RETURNED across a fiber swap.** `kicadOpenFileStart`
|
||||
returned its `int` wait token, but `wxWasmRunOnDispatchContext` Asyncify-suspends
|
||||
that embind frame while the load parks on the dispatch context — so the return
|
||||
arrived as an unwind PLACEHOLDER (0), the documented embind-across-swap gotcha.
|
||||
The shell then awaited token 0, resolved `false`, and proceeded (collab attach,
|
||||
etc.) while the real load was still running on the dispatch context. pcbnew's
|
||||
heavier load (inline fp-lib preload + 3D plugin registration) keeps that context
|
||||
busy/parked far longer, so a following dialog collides with it → freeze; eeschema's
|
||||
lighter open often completes within the synchronous drain and dodges the window —
|
||||
exactly the reported asymmetry.
|
||||
|
||||
**Fix:** mint the wait token in pure JS (`beginWait` — no swap) and pass it INTO
|
||||
a now-`void` `kicadOpenFileStart`; the wrapper awaits the token's promise, the job
|
||||
resolves it on completion. The starter's own placeholder return is void and
|
||||
harmless. Lesson pinned: **a value that must survive a dispatch-context run cannot
|
||||
be a return value — pass it in.**
|
||||
|
||||
### Prod-provider smoke (2026-08-08) — done, with one pre-existing red bisected
|
||||
|
||||
Against the live web stack (playwright-web config, reference backend :3060):
|
||||
|
|
|
|||
|
|
@ -153,7 +153,12 @@ if (typeof Asyncify !== "undefined" && !globalThis.__wxSchedulerInstalled) {
|
|||
var start = Module["kicadOpenFileStart"];
|
||||
if (typeof start !== "function" || typeof Module["kicadOpenFile"] !== "function") return;
|
||||
Module["kicadOpenFile"] = function (path) {
|
||||
var token = start(path);
|
||||
// Mint the token HERE, in pure JS — the starter runs the load on a
|
||||
// dispatch context and Asyncify-suspends its own frame, so a token
|
||||
// RETURNED from it would arrive as a placeholder (0). We own the token
|
||||
// and await its promise; the job resolves it when the load finishes.
|
||||
var token = self.beginWait("open");
|
||||
start(token, path);
|
||||
// waitPromise consumes early-resolved entries (the fast-error path),
|
||||
// so a job that finished before this await still resolves correctly.
|
||||
return self.waitPromise(token).then(function (r) { return !!r; });
|
||||
|
|
|
|||
137
tests/kicad/shell-open-dialogs.spec.ts
Normal file
137
tests/kicad/shell-open-dialogs.spec.ts
Normal file
|
|
@ -0,0 +1,137 @@
|
|||
import { test, expect, type Page } from './fixtures';
|
||||
import { injectFromSubmodule } from './utils/fs-inject';
|
||||
import { DEMO, PROJECT_DIR_MEMFS, countGlCanvases, openThreeDViewer } from './utils/threed-viewer';
|
||||
import { waitForBoardLoaded } from './utils/board-ready';
|
||||
import { waitForPcbnew } from './utils/pcbnew-ready';
|
||||
import { clickMenuBarItem, clickMenuItemByText } from '../e2e/utils/element-tracker';
|
||||
|
||||
/**
|
||||
* Phase F regression repro (docs/features/async/22 §10, the awaited-ccall entry
|
||||
* class). The user's live-editor path opens boards through the WEB SHELL's
|
||||
* `Module.kicadOpenFile(path)` — which Phase F wrapped to run the open body on
|
||||
* a DISPATCH CONTEXT and hand the shell a promise over an "open" wait token.
|
||||
* EVERY existing board-load spec instead drives File → Open (the wxFileDialog
|
||||
* → OpenProjectFiles path), so NONE exercises that wrapper — the exact gap the
|
||||
* user's "pcbnew dialogs are broken, eeschema is fine" report lives in.
|
||||
*
|
||||
* These specs reproduce the shell path (inject the board, then
|
||||
* `await Module.kicadOpenFile`) and THEN drive the dialogs the user reported
|
||||
* frozen: the 3D viewer, and a modal open/close. The assertion is simply that
|
||||
* the wasm main thread stays responsive and nothing traps.
|
||||
*/
|
||||
|
||||
const TRAP =
|
||||
/Aborted\(|index out of bounds|unreachable executed|indirect call signature|null function|memory access out of bounds/;
|
||||
|
||||
interface Mod {
|
||||
kicadOpenFile(path: string): Promise<boolean> | boolean;
|
||||
}
|
||||
|
||||
// Load the board the way the WEB SHELL does — through Module.kicadOpenFile
|
||||
// (the Phase F dispatch-context wrapper), not File → Open.
|
||||
async function openBoardViaShell(
|
||||
page: Page,
|
||||
logger: { consoleLogs: string[]; errors: string[] },
|
||||
): Promise<void> {
|
||||
const pcb = `${DEMO.stem}.kicad_pcb`;
|
||||
const pro = `${DEMO.stem}.kicad_pro`;
|
||||
await injectFromSubmodule(page, `kicad/demos/${DEMO.dir}/${pcb}`, `${PROJECT_DIR_MEMFS}/${pcb}`);
|
||||
await injectFromSubmodule(page, `kicad/demos/${DEMO.dir}/${pro}`, `${PROJECT_DIR_MEMFS}/${pro}`);
|
||||
|
||||
const ok = await page.evaluate(async (path) => {
|
||||
const m = (window as unknown as { Module: Mod }).Module;
|
||||
return await m.kicadOpenFile(path);
|
||||
}, `${PROJECT_DIR_MEMFS}/${pcb}`);
|
||||
expect(ok, 'Module.kicadOpenFile resolved true').toBe(true);
|
||||
|
||||
const result = await waitForBoardLoaded(page, logger, 60000);
|
||||
console.log(`[TEST] shell-open board-ready: ${result}`);
|
||||
}
|
||||
|
||||
// A JS-thread liveness probe: a fresh mailbox tick round-trips only if the wasm
|
||||
// main thread is not wedged. A frozen app never resolves it.
|
||||
async function assertResponsive(page: Page, label: string): Promise<void> {
|
||||
const alive = await page.evaluate(
|
||||
() =>
|
||||
new Promise<boolean>((resolve) => {
|
||||
const t = setTimeout(() => resolve(false), 8000);
|
||||
// A zero-delay mailbox message is delivered from a clean tick
|
||||
// (wxWasmMailboxTick) — only if the loop is running.
|
||||
requestAnimationFrame(() =>
|
||||
requestAnimationFrame(() => {
|
||||
clearTimeout(t);
|
||||
resolve(true);
|
||||
}),
|
||||
);
|
||||
}),
|
||||
);
|
||||
expect(alive, `main thread responsive after ${label}`).toBe(true);
|
||||
}
|
||||
|
||||
function noTrap(logger: { consoleLogs: string[]; errors: string[] }, label: string): void {
|
||||
const hits = [...logger.consoleLogs, ...logger.errors].filter((l) => TRAP.test(l));
|
||||
expect(hits, `no wasm trap after ${label}`).toEqual([]);
|
||||
}
|
||||
|
||||
test.describe('shell-opened board → dialogs (Phase F open-lane regression)', () => {
|
||||
test('3D viewer opens over a shell-loaded board and stays responsive', async ({
|
||||
page,
|
||||
testLogger,
|
||||
}) => {
|
||||
test.setTimeout(180000);
|
||||
await page.goto('/kicad/pcbnew.html');
|
||||
await waitForPcbnew(page);
|
||||
|
||||
await openBoardViaShell(page, testLogger);
|
||||
await assertResponsive(page, 'shell open');
|
||||
noTrap(testLogger, 'shell open');
|
||||
|
||||
const glBefore = await countGlCanvases(page);
|
||||
await openThreeDViewer(page, glBefore);
|
||||
await assertResponsive(page, '3D viewer open');
|
||||
noTrap(testLogger, '3D viewer open');
|
||||
});
|
||||
|
||||
test('a modal opens and closes over a shell-loaded board without freezing', async ({
|
||||
page,
|
||||
testLogger,
|
||||
}) => {
|
||||
test.setTimeout(180000);
|
||||
await page.goto('/kicad/pcbnew.html');
|
||||
await waitForPcbnew(page);
|
||||
|
||||
await openBoardViaShell(page, testLogger);
|
||||
await assertResponsive(page, 'shell open');
|
||||
|
||||
// Board Setup is a heavy modal reachable from the File menu without a
|
||||
// library provider — enough to exercise the ShowModal park/resume over
|
||||
// a dispatch context left idle by the shell open.
|
||||
expect(await clickMenuBarItem(page, 'File'), 'File menu findable').toBe(true);
|
||||
await clickMenuItemByText(page, 'Board Setup');
|
||||
await page.waitForFunction(
|
||||
() =>
|
||||
!!window.wxElementRegistry &&
|
||||
window.wxElementRegistry
|
||||
.findAll({ visible: true })
|
||||
.some((e) => /Dialog/i.test(e.typeName || '')),
|
||||
null,
|
||||
{ timeout: 20000 },
|
||||
);
|
||||
await assertResponsive(page, 'modal open');
|
||||
noTrap(testLogger, 'modal open');
|
||||
|
||||
// Close it (Escape → EndModal(wxID_CANCEL) → resolves the modal wait).
|
||||
await page.keyboard.press('Escape');
|
||||
await page.waitForFunction(
|
||||
() =>
|
||||
!!window.wxElementRegistry &&
|
||||
!window.wxElementRegistry
|
||||
.findAll({ visible: true })
|
||||
.some((e) => /Dialog/i.test(e.typeName || '')),
|
||||
null,
|
||||
{ timeout: 20000 },
|
||||
);
|
||||
await assertResponsive(page, 'modal close');
|
||||
noTrap(testLogger, 'modal close');
|
||||
});
|
||||
});
|
||||
|
|
@ -170,12 +170,15 @@ static bool kicadOpenFile( std::string path )
|
|||
// Run there, every wait inside the load parks the context through the
|
||||
// registry — the main stack never parks in place, which is the last
|
||||
// production member of the overlapped-wake class the D-on beacon sweep named.
|
||||
// The shim wraps Module.kicadOpenFile over this starter when it exists: the
|
||||
// wrapper returns the wait token's promise, so the shell's `await` semantics
|
||||
// (and the busy gate) are unchanged. Early failures resolve before the
|
||||
// wrapper awaits — covered by the wait registry's early-resolve retention.
|
||||
//
|
||||
// THE TOKEN IS PASSED IN, NOT RETURNED. Running the job on a dispatch context
|
||||
// Asyncify-suspends THIS embind frame while the load parks, so any return
|
||||
// value is delivered as an unwind PLACEHOLDER (0) into a rewind JS discards —
|
||||
// the same gotcha the fiber-park levers document. So the shim wrapper mints
|
||||
// the wait token in pure JS (no swap), hands it in here, and awaits its
|
||||
// promise; this starter returns void and its own placeholder return is
|
||||
// harmless. The job resolves the token when the load completes.
|
||||
extern "C" void wxWasmRunOnDispatchContext( void ( *fn )( void* ), void* arg );
|
||||
extern "C" int wxWasmBeginWait( const char* aKind );
|
||||
extern "C" void wxWasmResolveWait( int aToken, int aResult );
|
||||
|
||||
namespace
|
||||
|
|
@ -194,11 +197,9 @@ void kicadOpenFileJob( void* aArg )
|
|||
}
|
||||
} // namespace
|
||||
|
||||
static int kicadOpenFileStart( std::string path )
|
||||
static void kicadOpenFileStart( int token, std::string path )
|
||||
{
|
||||
const int token = wxWasmBeginWait( "open" );
|
||||
wxWasmRunOnDispatchContext( &kicadOpenFileJob, new OPEN_JOB{ std::move( path ), token } );
|
||||
return token;
|
||||
}
|
||||
|
||||
// JS-pollable open-in-flight probe (open_gate.h): the web shell defers the
|
||||
|
|
|
|||
Loading…
Reference in a new issue