Phase F: restore the doc-19 bounce (footprint chooser dead-app) + regression spec
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Td4ujboGuAw26jvbDQzehj
This commit is contained in:
parent
22fcb766e7
commit
80199f42f4
11 changed files with 274 additions and 3 deletions
|
|
@ -1 +1 @@
|
|||
18
|
||||
19
|
||||
|
|
|
|||
|
|
@ -1323,6 +1323,46 @@ provider (the web-e2e-rot specs that would are `fixme`), so this raced-resolve
|
|||
shape had no automated stager — a synthetic "resolve races a context-wait park"
|
||||
lever is owed as its regression pin.
|
||||
|
||||
### F4 was WRONG — the doc-19 bounce is restored (2026-08-10)
|
||||
|
||||
The gap-1 deferred-wake fix above did NOT fix the footprint chooser; the user
|
||||
retested and it still froze. Reproduced live (the public `demo` board, real
|
||||
fp-lib-table) and read the recorder — the mechanism is NOT gap 1:
|
||||
|
||||
```
|
||||
inplace-park-on-fiber-stack ctx=21 n=175
|
||||
fcs … rf=dynCall_iiii
|
||||
[wx-asyncify] fiber-resume-refused: fiber=2649686808 is asyncify-parked mid-body
|
||||
```
|
||||
|
||||
The footprint chooser is a **quasi-modal with a NESTED event loop, opened from
|
||||
the place-footprint TOOL COROUTINE**. Its nested wait runs on the tool
|
||||
coroutine's fiber stack, where `can_yield_here()` is false (the running star
|
||||
context is dispatch, not the fiber) — so `wxWasmYieldUntil` falls back to an
|
||||
**in-place Asyncify park on the fiber**, the doc-19 disease. On Cancel the
|
||||
resume into that parked-in-place fiber is refused by the quarantine and
|
||||
dropped → the loop never resumes → dead app.
|
||||
|
||||
**This is exactly what the doc-19 mainstack bounce prevented, and F4 removed
|
||||
it.** F4's reasoning ("post-flip the context-park replaces the bounce") was
|
||||
wrong for the tool-fiber case: the context-park only works once the frame is
|
||||
already on the dispatch context, and the bounce is what moves a nested loop
|
||||
opened from a tool fiber ONTO the dispatch context's stack (via `RunMainStack`
|
||||
→ libcontext root, which post-flip IS the running dispatch context — so
|
||||
`can_yield_here()` then holds and the wait context-parks correctly). F4's gate
|
||||
missed it because the footprint chooser has no automated coverage.
|
||||
|
||||
**F4 reverted** (kicad `452eb5260d`, wx `a8d9ece31f`, pcbjam `6a0a20f`): the
|
||||
bounce, `s_mainStackRunner`/`wxWasmRunOnMainStack`, `mainstack.h`,
|
||||
`main_stack_runner.h` + its 5 embind includes, and
|
||||
`RunOnMainStackIfActiveTool` are all back. Verified: footprint chooser
|
||||
Cancel/close green live (frames 2→1, loop keeps ticking) AND headless — the
|
||||
new `tests/kicad/footprint-chooser-close.spec.ts` opens the chooser over a
|
||||
board and cancels it, the regression pin F4 lacked. Gates: kicad 142/1,
|
||||
battery unchanged. The gap-1 deferred-wake queue stays (it is correct and
|
||||
inert here). **Lesson: never delete a doc-19 mitigation without a spec that
|
||||
drives a tool-opened quasi-modal.**
|
||||
|
||||
### Prod-provider smoke (2026-08-08) — done, with one pre-existing red bisected
|
||||
|
||||
Against the live web stack (playwright-web config, reference backend :3060):
|
||||
|
|
|
|||
2
kicad
2
kicad
|
|
@ -1 +1 @@
|
|||
Subproject commit ea4a37d9f81cecc34b1e0a15a6aa619c693824eb
|
||||
Subproject commit 99a7a368a6b36f2cd300d29bb136450e3c935cd7
|
||||
140
tests/kicad/footprint-chooser-close.spec.ts
Normal file
140
tests/kicad/footprint-chooser-close.spec.ts
Normal file
|
|
@ -0,0 +1,140 @@
|
|||
import { test, expect, type Page } from './fixtures';
|
||||
import { loadBoard } from './utils/threed-viewer';
|
||||
import { waitForPcbnew } from './utils/pcbnew-ready';
|
||||
import { clickMenuBarItem, clickMenuItemByText } from '../e2e/utils/element-tracker';
|
||||
|
||||
/**
|
||||
* Repro for the user-reported dead-app: Add Footprint → the footprint chooser
|
||||
* (a quasi-modal running a NESTED event loop, opened from the place-footprint
|
||||
* TOOL COROUTINE) → click Cancel → the whole UI freezes.
|
||||
*
|
||||
* Root cause (recorder, live demo board): the nested loop parks IN PLACE on the
|
||||
* tool coroutine's fiber stack (`inplace-park-on-fiber-stack`), then the resume
|
||||
* that would close it is refused by the stale-fiber quarantine
|
||||
* (`fiber-resume-refused: … asyncify-parked mid-body`) and dropped — the
|
||||
* doc-19 disease. It was masked by the mainstack bounce until Phase F4 removed
|
||||
* it; the footprint chooser has no automated coverage so the removal went
|
||||
* unnoticed. This spec is that coverage.
|
||||
*
|
||||
* Mechanics that matter: the chooser is a wxFrame (not a wxDialog), so detect
|
||||
* it by a second top-level frame + a "nested" scheduler wait; drive the canvas
|
||||
* with synthetic emscripten mouse events (a Playwright click is intercepted by
|
||||
* the wx scrollbar overlay); read the Cancel button's real coords from the
|
||||
* element registry.
|
||||
*/
|
||||
|
||||
const TRAP =
|
||||
/Aborted\(|index out of bounds|unreachable executed|indirect call signature|null function|memory access out of bounds/;
|
||||
|
||||
function frameCount(page: Page): Promise<number> {
|
||||
return page.evaluate(
|
||||
() =>
|
||||
(window.wxElementRegistry?.findAll({ visible: true }) ?? []).filter((e) =>
|
||||
/Frame$/.test(e.typeName || ''),
|
||||
).length,
|
||||
);
|
||||
}
|
||||
|
||||
async function synthClick(page: Page, x: number, y: number): Promise<void> {
|
||||
await page.evaluate(
|
||||
([cx, cy]) => {
|
||||
const c = document.querySelector('#canvas') as HTMLCanvasElement;
|
||||
const opt = (b: number) => ({
|
||||
clientX: cx,
|
||||
clientY: cy,
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
view: window,
|
||||
button: 0,
|
||||
buttons: b,
|
||||
});
|
||||
c.dispatchEvent(new MouseEvent('mousemove', opt(0)));
|
||||
c.dispatchEvent(new MouseEvent('mousedown', opt(1)));
|
||||
c.dispatchEvent(new MouseEvent('mouseup', opt(0)));
|
||||
c.dispatchEvent(new MouseEvent('click', opt(0)));
|
||||
},
|
||||
[x, y],
|
||||
);
|
||||
}
|
||||
|
||||
async function assertResponsive(page: Page, label: string): Promise<void> {
|
||||
const before = await page.evaluate(
|
||||
() => Number((window.__wxAsyncifyDump?.().match(/fcsTotal=(\d+)/) || [])[1] || 0),
|
||||
);
|
||||
await page.waitForTimeout(1500); // eslint-disable-line -- sampling the loop counter across a fixed window
|
||||
const after = await page.evaluate(
|
||||
() => Number((window.__wxAsyncifyDump?.().match(/fcsTotal=(\d+)/) || [])[1] || 0),
|
||||
);
|
||||
if (after === before) {
|
||||
const dump = await page.evaluate(() =>
|
||||
typeof window.__wxAsyncifyDump === 'function' ? window.__wxAsyncifyDump() : 'no dump',
|
||||
);
|
||||
console.log(`[TEST] RECORDER after ${label} (loop STALLED, fcs=${after}):\n${dump}`);
|
||||
}
|
||||
expect(after, `main loop still advancing after ${label}`).toBeGreaterThan(before);
|
||||
}
|
||||
|
||||
test.describe('Add Footprint chooser close (doc-19 dead-app repro)', () => {
|
||||
test('footprint chooser opens over a board and cancels without freezing', async ({
|
||||
page,
|
||||
testLogger,
|
||||
}) => {
|
||||
test.setTimeout(180000);
|
||||
await page.goto('/kicad/pcbnew.html');
|
||||
await waitForPcbnew(page);
|
||||
await loadBoard(page, testLogger);
|
||||
await assertResponsive(page, 'board load');
|
||||
|
||||
const framesBefore = await frameCount(page);
|
||||
|
||||
// Arm Add Footprint, then click the canvas to open the chooser.
|
||||
expect(await clickMenuBarItem(page, 'Place'), 'Place menu findable').toBe(true);
|
||||
await clickMenuItemByText(page, 'Place Footprints');
|
||||
const canvas = await page.locator('#canvas').boundingBox();
|
||||
if (!canvas) throw new Error('canvas not found');
|
||||
await synthClick(page, Math.round(canvas.width * 0.35), Math.round(canvas.height * 0.45));
|
||||
|
||||
// The chooser is a second top-level frame + a "nested" scheduler wait.
|
||||
await page
|
||||
.waitForFunction(
|
||||
(n) =>
|
||||
(window.wxElementRegistry?.findAll({ visible: true }) ?? []).filter((e) =>
|
||||
/Frame$/.test(e.typeName || ''),
|
||||
).length > n,
|
||||
framesBefore,
|
||||
{ timeout: 40000 },
|
||||
)
|
||||
.catch(() => {});
|
||||
const framesOpen = await frameCount(page);
|
||||
console.log(`[TEST] frames: ${framesBefore} → ${framesOpen}`);
|
||||
expect(framesOpen, 'footprint chooser frame opened').toBeGreaterThan(framesBefore);
|
||||
await assertResponsive(page, 'chooser open');
|
||||
|
||||
// Click the real Cancel button (coords from the element registry).
|
||||
const cancel = await page.evaluate(() => {
|
||||
const b = (window.wxElementRegistry?.findAll({ visible: true }) ?? []).find(
|
||||
(e) => /Button/i.test(e.typeName || '') && /cancel/i.test(e.label || ''),
|
||||
);
|
||||
return b ? { x: b.centerX, y: b.centerY } : null;
|
||||
});
|
||||
expect(cancel, 'Cancel button found').not.toBeNull();
|
||||
await synthClick(page, cancel!.x, cancel!.y);
|
||||
|
||||
// The chooser must close AND the app must stay alive. On the broken
|
||||
// build the loop stalls here (the dropped fiber resume).
|
||||
await page
|
||||
.waitForFunction(
|
||||
(n) =>
|
||||
(window.wxElementRegistry?.findAll({ visible: true }) ?? []).filter((e) =>
|
||||
/Frame$/.test(e.typeName || ''),
|
||||
).length <= n,
|
||||
framesBefore,
|
||||
{ timeout: 20000 },
|
||||
)
|
||||
.catch(() => {});
|
||||
await assertResponsive(page, 'chooser cancel');
|
||||
|
||||
const traps = [...testLogger.consoleLogs, ...testLogger.errors].filter((l) => TRAP.test(l));
|
||||
expect(traps, 'no wasm trap after cancel').toEqual([]);
|
||||
});
|
||||
});
|
||||
|
|
@ -60,6 +60,7 @@
|
|||
#include <pcbjam_remote_lock.h>
|
||||
#include "collab_common.h"
|
||||
#include "open_gate.h"
|
||||
#include "main_stack_runner.h"
|
||||
#include "collab_presence_core.h"
|
||||
#include "collab_presence_style.h"
|
||||
#include "pcbjam_theme.h"
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@
|
|||
#include <wx/app.h>
|
||||
#include <wx/string.h>
|
||||
#include "open_gate.h"
|
||||
#include "main_stack_runner.h"
|
||||
|
||||
using namespace emscripten;
|
||||
using json = nlohmann::json;
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@
|
|||
|
||||
#include "pcbjam_libs_reload.h"
|
||||
#include "open_gate.h"
|
||||
#include "main_stack_runner.h"
|
||||
#include "timer_park.h"
|
||||
#include "fiber_park.h"
|
||||
|
||||
|
|
|
|||
86
wasm/bindings/main_stack_runner.h
Normal file
86
wasm/bindings/main_stack_runner.h
Normal file
|
|
@ -0,0 +1,86 @@
|
|||
/*
|
||||
* Main-stack runner: the KiCad half of wx's nested-loop bounce
|
||||
* (pcbjam docs/features/async/19, 20 D3).
|
||||
*
|
||||
* A quasi-modal's nested event loop parks its whole stack for the dialog's
|
||||
* lifetime. When that stack is a TOOL coroutine's, the park suspends the
|
||||
* fiber's body where the fiber layer cannot see it: the stale-fiber guard
|
||||
* quarantines the fiber, then REFUSES its own resume, and the dialog stops
|
||||
* responding to clicks (only the titlebar x still works, because that path is
|
||||
* ungated). That is the Symbol Properties hang.
|
||||
*
|
||||
* wx detects "this nested loop is about to park on a non-main stack" — it can,
|
||||
* cheaply and exactly, by comparing a frame address against
|
||||
* emscripten_stack_get_base()/end() — but it must not know what a coroutine
|
||||
* is. So it calls this runner, and KiCad's TOOL_MANAGER moves the loop onto
|
||||
* the main stack via its own RunMainStack mechanism, which suspends the
|
||||
* coroutine the legitimate way: a fiber swap the layer records
|
||||
* (swap_suspended = true), so no quarantine, no refused resume.
|
||||
*
|
||||
* This lives in pcbjam's binding layer rather than in KiCad or wx precisely
|
||||
* because it is the only place that may know about both.
|
||||
*/
|
||||
#pragma once
|
||||
|
||||
#include <wx/app.h>
|
||||
|
||||
#include <eda_base_frame.h>
|
||||
#include <tool/tool_manager.h>
|
||||
|
||||
#include <wx/wasm/private/mainstack.h>
|
||||
|
||||
namespace pcbjam_main_stack
|
||||
{
|
||||
|
||||
/**
|
||||
* Run aFunc on the main stack if a tool coroutine is currently active.
|
||||
*
|
||||
* Returns 1 when the body has been run (bounced, or run inline because there
|
||||
* was no coroutine to bounce off), 0 when there was nothing to run it with —
|
||||
* no frame or no tool manager — in which case wx parks in place exactly as it
|
||||
* did before this hook existed.
|
||||
*/
|
||||
inline int run_on_main_stack( void ( *aFunc )( void* ), void* aArg )
|
||||
{
|
||||
if( !wxTheApp )
|
||||
return 0;
|
||||
|
||||
auto* frame = dynamic_cast<EDA_BASE_FRAME*>( wxTheApp->GetTopWindow() );
|
||||
|
||||
if( !frame )
|
||||
return 0;
|
||||
|
||||
TOOL_MANAGER* toolMgr = frame->GetToolManager();
|
||||
|
||||
if( !toolMgr )
|
||||
return 0;
|
||||
|
||||
// RunOnMainStackIfActiveTool runs the body inline when no coroutine is
|
||||
// running. Either way the body HAS run, so report it as handled — letting
|
||||
// wx fall through would run the nested loop a second time.
|
||||
bool ran = false;
|
||||
|
||||
toolMgr->RunOnMainStackIfActiveTool(
|
||||
[aFunc, aArg, &ran]()
|
||||
{
|
||||
ran = true;
|
||||
aFunc( aArg );
|
||||
} );
|
||||
|
||||
return ran ? 1 : 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Installs at static-init time. Only stores a function pointer, so it is safe
|
||||
* before wx exists; every lookup above happens lazily per call, since frames
|
||||
* come and go. Included by more than one binding TU — setting the same pointer
|
||||
* twice is idempotent.
|
||||
*/
|
||||
struct INSTALLER
|
||||
{
|
||||
INSTALLER() { wxWasmSetMainStackRunner( &run_on_main_stack ); }
|
||||
};
|
||||
|
||||
inline INSTALLER g_installer;
|
||||
|
||||
} // namespace pcbjam_main_stack
|
||||
|
|
@ -53,6 +53,7 @@
|
|||
#include "collab_common.h"
|
||||
#include "collab_presence_core.h"
|
||||
#include "open_gate.h"
|
||||
#include "main_stack_runner.h"
|
||||
#include "timer_park.h"
|
||||
#include "fiber_park.h"
|
||||
#include "collab_presence_style.h"
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@
|
|||
#include <wx/window.h>
|
||||
#include <nlohmann/json.hpp>
|
||||
#include "open_gate.h"
|
||||
#include "main_stack_runner.h"
|
||||
#include <eda_draw_frame.h>
|
||||
#include <kiid.h>
|
||||
#include <pcbjam_read_only.h>
|
||||
|
|
|
|||
|
|
@ -1 +1 @@
|
|||
Subproject commit aa2cd035473f6367ac1de7e8b9e950ae09c9f7eb
|
||||
Subproject commit 4d479cb3095f6b6cbc9550257c8f0c5b1deb102d
|
||||
Loading…
Reference in a new issue