feat(wasm-eh): migrate the WASM build to native wasm exceptions (+ 3D viewer default-on)
Replace the legacy Emscripten JS-exceptions model with native wasm-EH (legacy encoding) across the whole build, keeping Asyncify coroutines working via a from-source Binaryen --hoist-cpp-catches pre-pass. Net result: native-EH is the only build mode, the 3D viewer is on by default, and pcbnew shrinks substantially. Highlights: - Binaryen submodule everywhere + --hoist-cpp-catches integration in apply-asyncify; post-link Asyncify covers every app wasm (not just standalone test wasm). - Build deps (incl. OpenCASCADE without OCC_CONVERT_SIGNALS) and all KiCad apps with -fwasm-exceptions; emscripten_sleep added to the post-link asyncify-imports. - libcontext fiber entry wired under native exceptions; while-loop main loop + currData shim injected into all wx apps. - Native-EH collab apply fixed: DEBUG-define the embind TU + match all out-of-CMake C++ TUs' ABI flags to the core, fixing the vtable-layout skew / mis-dispatch. - 3D viewer enabled by default (real raytracer linked, not the stub). - Retire the EH-spike scaffolding; flip the asyncify-races ablation pins to shim-redundancy pins (native-EH stays clean with the legacy shims ablated). - Fix the asyncify-races quiescence check to not require Asyncify.currData==0: under the native-EH per-frame-yield top loop the main stack is asyncify-suspended every frame, so currData legitimately churns (a freed-but-not-yet-nulled buffer, not a leak). Refresh the pcbnew toolbar screenshot baseline for the new kicad. - CI: drop the obsolete binaryen_version input/env (the build uses the binaryen submodule fork's wasm-opt, not a version download); key the wasm-output cache on the binaryen submodule SHA instead. Bumps the wxwidgets + binaryen submodules to their squashed feature commits. Validated green: all 7 apps native-EH (real 3D in pcbnew); KiCad e2e 63/63 Firefox + Chromium (3D viewer renders); wx 336; coroutine 34/34 both engines; asyncify 7/7 both engines. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Viktor Vaczi
parent
b8c8dee355
commit
c1ef489cfa
75 changed files with 4976 additions and 680 deletions
|
|
@ -1,5 +1,14 @@
|
|||
# 03 — Toolchain compatibility status (verified 2026-06-10/11)
|
||||
|
||||
> **Partly superseded 2026-06-22 — see [`06-spike-plan.md`](06-spike-plan.md).** Corrections:
|
||||
> (a) the host-side `--asyncify` already runs **Binaryen v130** in CI/publish
|
||||
> (`BINARYEN_VERSION=130`), so "we don't have the partial support locally" understates it — v121
|
||||
> is only the finalize/in-link copy. (b) `--pass-arg=asyncify-ignore-unwind-from-catch` **is**
|
||||
> implemented (shipped v125), but it *silently drops* the suspend — a tripwire-silencer, not a
|
||||
> fix. (c) The encoding is resolved to **legacy** (exnref + Asyncify is unsupported in every
|
||||
> released Binaryen incl. v130, no roadmap), so the "encoding decision forks the asyncify work"
|
||||
> framing in §experiment is closed: legacy + the catch-arm-hoisting pre-pass is the only path.
|
||||
|
||||
## The compatibility matrix
|
||||
|
||||
| Combination | Status |
|
||||
|
|
|
|||
|
|
@ -1,75 +0,0 @@
|
|||
# 04 — The KiCad catch-block audit
|
||||
|
||||
## Why an audit
|
||||
|
||||
Binaryen's asyncify cannot (yet) handle a suspension that begins while execution is inside
|
||||
a wasm catch handler (see 05). Any C++ `catch` whose handler (directly or transitively)
|
||||
opens a modal dialog, touches the async clipboard, etc., is therefore illegal under
|
||||
`-fwasm-exceptions` + ASYNCIFY=1 today. KiCad's standard error pattern is exactly that:
|
||||
|
||||
```cpp
|
||||
catch( const IO_ERROR& ioe )
|
||||
{
|
||||
DisplayErrorMessage( this, ioe.What() ); // → ShowModal → Asyncify suspension
|
||||
}
|
||||
```
|
||||
|
||||
## Method
|
||||
|
||||
[`catch_audit.py`](catch_audit.py): walks `kicad/**/*.cpp` (excluding `thirdparty/`, `qa/`),
|
||||
extracts every catch block with real brace matching (string/comment aware), and classifies
|
||||
each handler body:
|
||||
|
||||
- **direct_suspend** — contains a known suspending call (`DisplayError[Message]`,
|
||||
`DisplayInfoMessage`, `wxMessageBox`, `ShowModal`, `ShowQuasiModal`, `KIDIALOG`,
|
||||
`IsOK(`, clipboard ops, `wxFileDialog`, …).
|
||||
- **infobar** — only `ShowInfoBar*` (non-modal, non-suspending).
|
||||
- **trivial** — only rethrow / capture / format / logging. Note `wxLogError` is *safe*:
|
||||
its GUI display is deferred to the idle-time log flush, outside any catch.
|
||||
`PGM_BASE::HandleException` (`common/pgm_base.cpp:805`) was manually verified — it only
|
||||
`wxLogError`s → benign.
|
||||
- **needs_review** — calls functions not classifiable as benign; requires a transitive look.
|
||||
|
||||
Full output incl. all site locations: [`audit-results.txt`](audit-results.txt).
|
||||
Re-run: `python3 catch_audit.py` (path to the kicad tree is hardcoded at the top).
|
||||
|
||||
## Results (2026-06-10, kicad @ wasm-port head)
|
||||
|
||||
| Category | Count |
|
||||
|---|---|
|
||||
| **direct_suspend** | **85** |
|
||||
| needs_review | 93 (tail looks mostly benign — accessors/file ops; expect ~10–20 to become refactors on inspection) |
|
||||
| trivial/safe | 458 |
|
||||
| infobar-only | 0 |
|
||||
| **total** | **636** |
|
||||
|
||||
Per app (direct/review): eeschema 37/17, pcbnew 32/29, common 9/35, cvpcb 3/1, rest
|
||||
scattered. Concentrated exactly on the file-load error paths the e2e tests exercise:
|
||||
`pcbnew/files.cpp`, `eeschema/files-io.cpp`, `footprint_libraries_utils.cpp`,
|
||||
`symbol_library_manager.cpp`, the design-block utils.
|
||||
|
||||
wxWidgets adds essentially nothing (zero catch-with-dialog sites in its own code).
|
||||
|
||||
## The hand-refactor option (superseded by the fork, kept for the record)
|
||||
|
||||
Mechanical hoist per site:
|
||||
|
||||
```cpp
|
||||
wxString err;
|
||||
try { ... }
|
||||
catch( const IO_ERROR& ioe ) { err = ioe.What(); } // capture only
|
||||
if( !err.IsEmpty() ) DisplayErrorMessage( this, err ); // suspend OUTSIDE the catch
|
||||
```
|
||||
|
||||
Effort: 85 hoists (~15–30 min each; error-UX paths with near-zero test coverage) ≈ 3–5
|
||||
dev-days; 93 reviews ≈ 2–3 dev-days; destructor-during-unwind audit ≈ half day (cleanup
|
||||
pads = `catch_all`; dialogs from destructors expected zero); **plus** permanent
|
||||
upstream-sync policing (every KiCad merge adds new `catch { DisplayError }` sites —
|
||||
`catch_audit.py` as a CI gate automates detection), **plus** the fork-divergence cost
|
||||
against the "stay close to upstream" policy. Total ~2–3 weeks one-time + maintenance tax,
|
||||
with hard-crash failure modes for any missed transitive site (asyncify asserts mode traps
|
||||
deterministically — good in CI, fatal in production).
|
||||
|
||||
**Verdict:** with the catch-arm-hoisting fork (05) this entire refactor becomes
|
||||
unnecessary — all 85 direct sites are C++ catches, which the fork makes legal. Only
|
||||
suspend-inside-`catch_all`-cleanup remains forbidden, which KiCad does not do.
|
||||
267
docs/features/wasm-exceptions/06-spike-plan.md
Normal file
267
docs/features/wasm-exceptions/06-spike-plan.md
Normal file
|
|
@ -0,0 +1,267 @@
|
|||
# 06 — Native wasm-EH: refreshed findings + red-green spike plan (2026-06-22)
|
||||
|
||||
> **Status:** plan / decision record, *in refinement* (no code written yet — this is the
|
||||
> agreed artifact before Phase 0). Produced by a 5-agent research spike on 2026-06-22:
|
||||
> browser support · toolchain · runtime mechanisms · the Binaryen pass · red-green harness.
|
||||
> **Supersedes in part** `README.md`, `03-toolchain-status.md`, and the root
|
||||
> `docs/wasm-exceptions-experiment.md` where called out below. Reads on top of 01–05.
|
||||
|
||||
---
|
||||
|
||||
## RESULTS — Phases 0 & 1 (2026-06-22, current toolchain, NO emsdk bump)
|
||||
|
||||
> **Cross-engine policy:** every spec runs in **all three engines — Firefox, Chrome (V8), and
|
||||
> Safari/WebKit** (`cd tests && npm run test:asyncify:all`). The eh-spike harness is green in all
|
||||
> three. Result notes below that say "V8/Firefox" predate the Safari run and now hold in WebKit too.
|
||||
|
||||
**Phase 0 — PASS.** em 4.0.2's LLVM emits *parseable, runnable* legacy wasm-EH:
|
||||
- A trivial `-fwasm-exceptions -sWASM_LEGACY_EXCEPTIONS=1` throw/catch builds (finalize on
|
||||
bundled v121, **no "popping from empty stack"**) and runs correctly under node
|
||||
(`tests/apps/standalone/eh-spike/eh_probe.cpp`, Makefile `eh-probe` target). So the parked
|
||||
experiment's finalize failure was **scale/OCC-specific, not a general codegen break**.
|
||||
- Binaryen **v130 asyncifies** the legacy-wasm-EH module (asyncify_* exports present); bundled
|
||||
**v121 crashes** (`UNREACHABLE … Asyncify.cpp:1146 — unexpected expression type`). Confirms
|
||||
the v121→v130 split; the post-link asyncify path (already `BINARYEN_VERSION=130` in CI) is the
|
||||
one to use. **No emsdk bump required for the spike.**
|
||||
|
||||
**Phase 1 — PASS (red-green captured).** One source built two ways
|
||||
(`scripts/build-eh-spike.sh`; the wasm-EH variant uses the production stub→post-link-v130 flow).
|
||||
Validated in **both V8 (node) and Firefox** by `tests/asyncify/eh-spike.spec.ts`:
|
||||
|
||||
| case | mechanism | JS-EH | native wasm-EH |
|
||||
|---|---|---|---|
|
||||
| `throw_across_sleep` | EM_ASYNC_JS sleep + EH | PASS | **PASS** |
|
||||
| `fiber_then_throw` | `emscripten_fiber_swap` + EH | PASS | **PASS** |
|
||||
| `suspend_in_catch` | suspend *inside* a catch arm | PASS | **HARD TRAP** — `indirect call to null` / `null function or function signature mismatch` |
|
||||
|
||||
→ asyncify + native wasm-EH **works for the sleep and fiber mechanisms**; the *only* failure is
|
||||
suspend-inside-catch (Binaryen #4470), and it fails **loud and deterministically** (a trap, not a
|
||||
silent drop) in both engines. This is exactly the hole the catch-arm-hoisting pass (05) closes —
|
||||
the spike has now made it concrete and pinned it as a regression test.
|
||||
|
||||
**Artifacts:** `tests/apps/standalone/eh-spike/{eh_probe.cpp,eh_spike_test.cpp}`,
|
||||
`scripts/build-eh-spike.sh`, `tests/asyncify/eh-spike.spec.ts`, the `eh-probe` Makefile target,
|
||||
and `playwright-asyncify.config.ts` `testMatch` widened to include `eh-spike`.
|
||||
|
||||
**Phase 1.5 — PASS (the fix).** The catch-arm-hoisting Binaryen pass
|
||||
(`scripts/binaryen-hoist-pass/HoistCppCatches.cpp`, ~150 LoC + a 39-line registration patch)
|
||||
flips `suspend_in_catch` from a hard trap to **green** under native wasm-EH — validated in
|
||||
**V8 + Firefox**. `tests/asyncify/eh-spike.spec.ts` is now a 3-variant ablation harness:
|
||||
JS-EH green / wasm-EH-no-pass red / wasm-EH+hoist green (pins both disease and fix). The pass
|
||||
outlines a cpp-tag catch arm containing a suspending call to plain code after the try (capture
|
||||
payload→local + set flag + `br` out; in the hoisted handler `pop`→`local.get`, no-match
|
||||
`rethrow`→`throw`), so stock `--asyncify` instruments it for free — exactly the 05 design. It is
|
||||
a PRE-pass (`--hoist-cpp-catches` before `--asyncify`); `Asyncify.cpp` is unchanged. Built
|
||||
reproducibly via `scripts/binaryen-hoist-pass/build-wasm-opt.sh` (drops the file into the v130
|
||||
clone, applies the patch, `ninja`). **MVP scope** (sufficient for the toy): void/unreachable-typed
|
||||
tries, single cpp catch (single-i32 tag), no catch_all, and a DIRECT suspending-import call in the
|
||||
arm; the nested-pop / nested-rethrow cases are handled (via an `ExpressionStackWalker` Try-ancestor
|
||||
guard). **Remaining for real KiCad** (all "fiddly-but-tractable" per 05): concrete-result-typed
|
||||
tries (route the body value through a temp local), catch_all coexistence, and TRANSITIVE suspend
|
||||
detection (catch → DisplayErrorMessage → ShowModal → startModal), which needs the asyncify
|
||||
ModuleAnalyzer rather than the direct-call heuristic — or simply hoist-all-cpp-catches and let
|
||||
`-O2` clean up.
|
||||
|
||||
**Next:** Phase 2 — flip a small wx standalone app to native EH (the real `ShowModal`-from-catch
|
||||
path). Phase 3 — full KiCad + the emsdk bump (OCC only). Before KiCad: generalize the pass
|
||||
(result-typed tries + transitive detection) and file the design on binaryen #4470.
|
||||
|
||||
---
|
||||
|
||||
## 0. What changed since 01–05 (which were authored 2026-06-11/12)
|
||||
|
||||
Three corrections that move the decision:
|
||||
|
||||
1. **The encoding is resolved: use the LEGACY encoding (`-sWASM_LEGACY_EXCEPTIONS=1`).**
|
||||
Not "decide after the emsdk bump" (as README §TL;DR / 03 §experiment / 05 §new-EH frame
|
||||
it). Binaryen's Asyncify supports **only** legacy `try`/`catch` — never
|
||||
`try_table`/`throw_ref`/`exnref` — through the latest **v130** (Jun 2026), with no
|
||||
roadmap, PR, or TODO to change it. The exnref path the parked experiment was forced onto
|
||||
is therefore a **dead end**: even with the OCC bug fixed, an exnref module dies at the
|
||||
`--asyncify` step. Legacy has shipped *unflagged* in all three engines since 2021–22
|
||||
(Chrome 95 / Safari 15.2 / Firefox 100), is still emscripten's own default, and — crucially
|
||||
— the size prize comes from **native-vs-JS EH (dropping `invoke_*`), independent of the
|
||||
encoding** — so legacy costs us nothing. **The "exnref → TryTable variant" fork is closed:
|
||||
legacy + the catch-arm-hoisting pre-pass (05) is the single viable path.**
|
||||
|
||||
2. **Binaryen is no longer a blocker — production already ships v130.** README/03 say "our
|
||||
emsdk bundles v121, we don't even have the partial support locally." That v121 is only the
|
||||
*finalize / test-apps-in-link* copy. **CI and publish pin `BINARYEN_VERSION=130`**
|
||||
(`.github/workflows/ci-ubicloud.yml:44`, `.github/workflows/publish-wasm.yml:33`;
|
||||
resolved by `scripts/common/get-wasm-opt.sh:50`) for the host-side `--asyncify` + `-O2`.
|
||||
v130 is the latest release and carries the v125 partial legacy-EH asyncify support. So the
|
||||
migration plan's "step 1: get a newer wasm-opt for the post-link step" is **already done**.
|
||||
|
||||
3. **`--pass-arg=asyncify-ignore-unwind-from-catch` is implemented now** (shipped in
|
||||
Binaryen v125; `03:23` called it "dead docs"). But it is a **tripwire-silencer, not a
|
||||
fix**: it *silently drops* the suspension inside a catch arm, which is semantically wrong
|
||||
on paths we actually reach (file-load error dialogs, the eeschema **Paste** handler).
|
||||
Correctness still requires the catch-arm-hoisting pass (or a hand-refactor).
|
||||
|
||||
**Net:** the long pole shrinks to a single thing — the **emsdk / LLVM compiler bump** (for
|
||||
*parseable legacy wasm-EH* + the OCC `br_table` miscompile), **not** the Binaryen version.
|
||||
|
||||
---
|
||||
|
||||
## 1. The decision in one line
|
||||
|
||||
> `-fwasm-exceptions -sSUPPORT_LONGJMP=wasm -sWASM_LEGACY_EXCEPTIONS=1`, applied **uniformly**
|
||||
> across deps + wxWidgets + KiCad + test apps; host-side `--asyncify` on Binaryen ≥ v125
|
||||
> (we have v130); the ~85 suspend-in-catch sites fixed by the **catch-arm-hoisting pre-pass**
|
||||
> (05), not by source refactors. Prize unchanged from 02: **64.5 → ~36 MB gz download,
|
||||
> 187 → ~122 MB module**, plus a large `-O2` wall-time drop on the build critical path.
|
||||
|
||||
---
|
||||
|
||||
## 2. What works / what doesn't (verdicts from this spike)
|
||||
|
||||
| | Verdict | Why |
|
||||
|---|---|---|
|
||||
| Native wasm-EH, **legacy** encoding, + Asyncify v130, + catch-arm fix | ✅ **viable path** | Mechanically sound per all five agents; legacy is the only encoding Asyncify can instrument. |
|
||||
| **exnref** (`=0`) + Asyncify | ❌ architecturally broken | Asyncify has zero `TryTable`/`exnref` support in *every* released Binaryen incl. v130; no roadmap. The experiment's `=0` never reached asyncify (blocked earlier at finalize). |
|
||||
| `--asyncify-ignore-unwind-from-catch` as a *fix* | ❌ not a fix | Exists (v125+), but silently drops the suspend → our catch→modal dialogs misbehave. A tripwire-silencer only. |
|
||||
| Bundled Binaryen **v121** + Asyncify + any wasm-EH | ❌ crashes | `Asyncify.cpp:998 UNREACHABLE`. Need ≥ v125 — already satisfied for the host-side pass (v130). |
|
||||
| JSPI instead of Asyncify | ❌ closed for us | Fibers don't exist under JSPI (emscripten #18180); KiCad tools are fibers. (`03`) |
|
||||
|
||||
---
|
||||
|
||||
## 3. The three asyncify mechanisms vs. the EH switch
|
||||
|
||||
All three suspension mechanisms + handle-sleep are **orthogonal** to the C++ EH model — none
|
||||
touches `__cxa_*`/landing pads in its own implementation. (Full detail: this session's
|
||||
mechanism map + `docs/features/async/`.)
|
||||
|
||||
| Mechanism | EH-model dependence | Verdict |
|
||||
|---|---|---|
|
||||
| **Fibers** (`emscripten_fiber_swap`, libcontext/`coroutine.h`) | none — no try/catch in the coroutine layer | just works |
|
||||
| **Main-loop park** (`emscripten_set_main_loop(...,1)` → `throw "unwind"`) | none — it's a **JS string** throw, not `__cxa_throw`; swallowed in JS glue | just works |
|
||||
| **EM_ASYNC_JS sleeps + handle-sleep engine** (modal/nested-loop/clipboard/fonts/DOM-popup/PCBJam fetch) | implementation: none. **Callers** are the risk. | at-risk *only* via the catch-arm caller pattern |
|
||||
|
||||
The §3c trampoline-heal (`inject-dyncall-shims.sh`) and `handlesleep.js` "unwind" catches are
|
||||
**JS-level and EH-independent** — unaffected by the switch.
|
||||
|
||||
**The entire exposure** is the **~85 sites where a C++ `catch` arm opens a modal →
|
||||
asyncify-suspends** (e.g. `pcbnew/files.cpp:674/684/692`; 6 of them on a coroutine/fiber
|
||||
stack including the eeschema Paste handler). This trap has **never fired at runtime** — it is
|
||||
a static conclusion (brace-match audit `04` + Binaryen `AsyncifyFlow` skipping catch bodies
|
||||
`05`). **Making it concrete and proving the fix is the spike's core job.**
|
||||
|
||||
---
|
||||
|
||||
## 4. Blockers, ordered (corrected)
|
||||
|
||||
| # | Blocker | Status / fix | Confidence |
|
||||
|---|---|---|---|
|
||||
| 1 | **emsdk 4.0.2 → 6.0.0 (LLVM 23) compiler bump** | em 4.0.2 emitted legacy wasm-EH that failed Binaryen parse at finalize (`popping from empty stack`) with **both** v121 and official v130 → an **LLVM-output bug, not a Binaryen-version bug**; a newer LLVM should fix it. Also lifts the bundled finalize-binaryen to ~v130. **Changes the JS-EH build too → must revalidate the whole project.** | parse-failure cause **unconfirmed** (Phase 0 resolves) |
|
||||
| 2 | **OpenCASCADE** | (a) invalid `br_table` arity in `ShapeUpgrade_SplitSurface::Build` under wasm-EH — candidate upstream fix is **LLVM PR #123915** (Jan 2025, "add unreachable before catch destinations"), so the LLVM-23 bump likely covers it. (b) Separate `OCC_CONVERT_SIGNALS` setjmp↔exception-in-one-function conflict — **likely already sidestepped** by `-sSUPPORT_LONGJMP=wasm`; verify, else drop the flag. | both **to verify** in Phase 3 |
|
||||
| 3 | **~85 suspend-in-catch sites** | catch-arm-hoisting Binaryen pre-pass — ~400–800 LoC, 1–2 wk, upstreamable, **keeps KiCad pristine** and obsoletes the hand-refactor + CI gate (05, 04). Alternative: hand-refactor (2–3 wk + permanent `catch_audit.py` CI gate + fights our upstream-closeness policy). | design **sound** (05); not yet built |
|
||||
| 4 | **Safari 26.0 regression** (watch) | `-fwasm-exceptions` *legacy* apps transiently crashed at startup on Safari 26.0's initial release (in-place-interpreter bug, emscripten #25365), since patched. Track Safari point releases. | external, **patched** |
|
||||
|
||||
Browser support is otherwise a non-issue: legacy EH ≈ Chrome 95+/Safari 15.2+/Firefox 100+,
|
||||
>96% of traffic, a 4-year tail. The "Chrome problem" (V8 slow to ship *exnref*) doesn't touch
|
||||
us because we never emit exnref.
|
||||
|
||||
---
|
||||
|
||||
## 5. The phased red-green plan
|
||||
|
||||
**Key enabler (agent 5):** the Asyncify×wasm-EH interaction is **toy-testable now, without the
|
||||
emsdk bump** — the bump's blockers are OCC-specific, and the toy has no OCC. So we separate the
|
||||
two risks: *compiler bump* (full-KiCad only) vs *asyncify×wasm-EH semantics* (provable on a toy
|
||||
today). We reuse the existing ablation harness pattern in
|
||||
`tests/apps/standalone/{asyncify-races,coroutine}/` + `tests/asyncify/`.
|
||||
|
||||
### Phase 0 — micro-probe (hours, current toolchain)
|
||||
**Question:** can em 4.0.2's LLVM emit *parseable* legacy wasm-EH on a tiny no-OCC program?
|
||||
- Minimal standalone C++ (no wx, no OCC): a `try { throw } catch(...) {}` + a trivial sleep.
|
||||
- Build `-fwasm-exceptions -sSUPPORT_LONGJMP=wasm -sWASM_LEGACY_EXCEPTIONS=1`, **`-sASYNCIFY=0`
|
||||
at link**; does it link + `wasm-emscripten-finalize` (bundled v121) cleanly? (i.e. reproduce
|
||||
the experiment's `=1` failure, or not).
|
||||
- Then standalone `wasm-opt --asyncify` via `get-wasm-opt.sh` with `BINARYEN_VERSION=130` —
|
||||
does v130's asyncify parse + instrument the legacy-EH module?
|
||||
- **Gate:** both clean → Phase 1 proceeds on current toolchain, bump deferred to Phase 3.
|
||||
Finalize fails the same way → em-4.0.2 legacy codegen is generally broken → **front-load the
|
||||
bump** (Phase 0′) before the toy.
|
||||
|
||||
### Phase 1 — red-green toy (days)
|
||||
**Goal:** prove all three mechanisms survive native wasm-EH, and that **suspend-in-catch is the
|
||||
only failure** (red) — then green once the fix lands.
|
||||
- Add `tests/apps/standalone/eh-spike/eh_spike_test.cpp`, console protocol
|
||||
`[EH_SPIKE] PASS/FAIL/SUMMARY` (model on `races_test.cpp`). Three cases:
|
||||
(a) **throw across a sleep**; (b) **suspend inside a catch** (the #4470 case);
|
||||
(c) **fiber-swap then throw** (include `../coroutine/kicad_coroutine_harness.h`).
|
||||
- One source, two builds via a make var `EH_FLAGS`: `-fexceptions` (green baseline) vs
|
||||
`-fwasm-exceptions … =1`. No-wx/no-OCC (template on the `coroutine-pthread` no-wx
|
||||
`LDFLAGS`), so we skip the wx-EH rebuild and the OCC bug.
|
||||
- **Decouple asyncify**: link `-sASYNCIFY=0`, then standalone `wasm-opt --asyncify`
|
||||
(`get-wasm-opt.sh`, v130) so the in-link v121 doesn't poison case (b). Re-inject shims.
|
||||
- Playwright spec `tests/asyncify/eh-spike.spec.ts` (model on `asyncify-races.spec.ts`,
|
||||
reuse `findSummary`/`crashLines`): JS-EH build **all green**; wasm-EH build **green on (a)/(c),
|
||||
red on (b)**. Build commands: `scripts/build-wasm-test.sh eh-spike-{js,wasm}`; run
|
||||
`cd tests && npx playwright test --config=playwright-asyncify.config.ts --project=firefox …`
|
||||
(widen `testMatch` or name it `asyncify-races-eh.spec.ts`).
|
||||
- **Bonus data:** characterize what (b) does under wasm-EH — hard trap vs silent drop (the
|
||||
`ignore-unwind-from-catch` behavior). Informs decision-point #1.
|
||||
|
||||
### Phase 1.5 — the catch-arm-hoisting pass (1–2 wk; only if Phase 1 confirms (b) is the sole gap)
|
||||
- Implement `src/passes/HoistCppCatches.cpp` + 2 registration lines in a Binaryen fork branch
|
||||
(05 §transform). Wire via `get-wasm-opt.sh` `BINARYEN_BUILD_FROM_SOURCE=1` pointed at the
|
||||
fork branch (`get-wasm-opt.sh:59-90`; one-line URL/branch change).
|
||||
- Prove case (b) goes **green** on the toy. File the design on **binaryen #4470** first
|
||||
(upstreamability).
|
||||
|
||||
### Phase 2 — wx standalone (days)
|
||||
- Flip a small wx test app to native EH (wx native-EH rebuild via the `KICAD_WASM_EH` gate;
|
||||
flag set patch-ready in the experiment appendix — but with `=1`, **not** the experiment's
|
||||
`=0`). Exercises the **real** `ShowModal`-from-catch suspend in miniature; confirms the pass
|
||||
handles the actual wx modal, not just a toy sleep.
|
||||
|
||||
### Phase 3 — full KiCad (weeks)
|
||||
- emsdk bump → 6.0.0 (LLVM 23): **revalidate the JS-EH build first** (whole-project risk).
|
||||
- OCC: verify `br_table` fixed; verify (and if needed remove) `OCC_CONVERT_SIGNALS`.
|
||||
- Apply the experiment appendix patch (`KICAD_WASM_EH=1`) **corrected to `=1`**; uniform flag
|
||||
flip across deps/wx/kicad/tests; **drop `env.invoke_*` from `ASYNCIFY_IMPORTS`**
|
||||
(`apply-asyncify.sh` — README cites `:33`, current read ~`:88`; verify).
|
||||
- Build host-side asyncify with the hoisting pass (fork). e2e audit under
|
||||
`-sASYNCIFY_ASSERTIONS` to flush any missed suspend-in-catch. Measure the gz/module win + the
|
||||
`-O2` wall-time drop.
|
||||
|
||||
---
|
||||
|
||||
## 6. Open decision points (to refine together)
|
||||
|
||||
1. **Catch-arm fix strategy:** hoisting pass (rec — pristine KiCad, upstreamable) vs
|
||||
hand-refactor 85 sites (CI gate, fights upstream-closeness) vs ship-with-`ignore-flag` and
|
||||
accept degraded error dialogs (fast, semantically wrong). *Phase 1 data informs this.*
|
||||
2. **emsdk target:** 6.0.0 (latest, LLVM 23) vs a more conservative 5.0.x — 6.0.0 carries
|
||||
other breaking changes (startup `async/await`, compiler-rt naming) needing JS-EH-side
|
||||
revalidation.
|
||||
3. **Binaryen fork hosting:** real git submodule (sibling to kicad/wxwidgets, pinned) vs a
|
||||
lighter `BINARYEN_BUILD_FROM_SOURCE` branch URL. (Fork is a *build tool*, not conveyed code
|
||||
— no GPLv3 `BUILD_SHA` treatment needed.)
|
||||
4. **Run Phase 1 on current em 4.0.2** (if Phase 0 passes) **vs bump first regardless.**
|
||||
5. **Sequencing vs the Asyncify-arbiter work** (`docs/features/async/`): README §relationship
|
||||
says arbiter-first (it fixes shipping bugs, needed under either EH model). Still the priority?
|
||||
|
||||
---
|
||||
|
||||
## 7. Risks / unknowns (honest)
|
||||
|
||||
- The `popping from empty stack` cause is **unconfirmed** — Phase 0 is the cheap decider.
|
||||
- Whether case (b) under wasm-EH **traps vs silently drops** — Phase 1 characterizes.
|
||||
- Whether **LLVM 23 actually fixes** our OCC `br_table` (PR #123915 is a strong candidate, not
|
||||
confirmed against our exact OCC code) — Phase 3 verifies.
|
||||
- Safari 26.0 legacy-EH regression — patched upstream, but a reminder to track point releases.
|
||||
- File:line refs in 03/05/README and the experiment appendix may have **drifted** — re-verify
|
||||
before editing (e.g. `apply-asyncify.sh` `ASYNCIFY_IMPORTS` line, build-script EH-flag lines).
|
||||
|
||||
---
|
||||
|
||||
## 8. Provenance
|
||||
|
||||
5-agent spike, 2026-06-22 — browser support, toolchain, runtime mechanisms, the pass design,
|
||||
red-green harness. Key external refs: WebAssembly/binaryen **#4470** (open) / **#5475** (merged
|
||||
v125), **LLVM PR #123915**, **emscripten #25365** (Safari 26.0), webassembly.org **Wasm 3.0**
|
||||
(Sep 2025). Internal: this dossier 01–05, `docs/wasm-exceptions-experiment.md`,
|
||||
`docs/features/async/`, `docs/features/perf/README.md` (lever #9), and memory
|
||||
`wasm-eh-migration-assessment`.
|
||||
244
docs/features/wasm-exceptions/07-spike-results-and-opinion.md
Normal file
244
docs/features/wasm-exceptions/07-spike-results-and-opinion.md
Normal file
|
|
@ -0,0 +1,244 @@
|
|||
# 07 — Native wasm-EH spike: results and engineering opinion (2026-06-22)
|
||||
|
||||
> What the spike actually built and proved, then a candid opinion on whether to pursue the
|
||||
> `-fwasm-exceptions` migration and how. Companion to [`06-spike-plan.md`](06-spike-plan.md)
|
||||
> (the plan + Phase 0/1/1.5 result log). Verdict up front, evidence and caveats after.
|
||||
|
||||
## Verdict
|
||||
|
||||
**The migration is viable on the current toolchain, the single blocking limitation is real
|
||||
and narrow, and a bounded Binaryen pass fixes it. I recommend pursuing it** — but the
|
||||
schedule risk is the **emsdk bump for OpenCASCADE**, not the exception machinery, and the
|
||||
pass still needs generalization before full KiCad. This was a genuine de-risking: the core
|
||||
"can Asyncify and native wasm-EH coexist?" question is now answered **yes, empirically**,
|
||||
in **all three engines — Chrome (V8), Firefox (SpiderMonkey), and Safari (WebKit)** — with no
|
||||
emsdk bump required to prove it. (Project policy, set this session: every spec is run in all
|
||||
three browsers — `cd tests && npm run test:asyncify:all`.)
|
||||
|
||||
## What was proven (with evidence, not estimates)
|
||||
|
||||
Everything below ran on the **current pinned toolchain** (emscripten 4.0.2, Binaryen v130 via
|
||||
`BINARYEN_VERSION=130`) — **no emsdk bump**.
|
||||
|
||||
1. **em 4.0.2 emits runnable legacy wasm-EH.** A trivial `-fwasm-exceptions
|
||||
-sWASM_LEGACY_EXCEPTIONS=1` throw/catch builds (finalize on the bundled v121, *no* "popping
|
||||
from empty stack") and runs correctly under node. So the parked experiment's finalize
|
||||
failure was scale/OCC-specific, not a general codegen break.
|
||||
(`tests/apps/standalone/eh-spike/eh_probe.cpp`.)
|
||||
2. **Binaryen v130 asyncifies legacy-wasm-EH; v121 cannot.** v130 instruments the module
|
||||
(asyncify_* exports appear); the emsdk-bundled v121 crashes (`UNREACHABLE …
|
||||
Asyncify.cpp:1146`). We already ship v130 for the post-link asyncify, so this costs nothing.
|
||||
3. **The three Asyncify mechanisms split exactly as predicted.** A red-green toy
|
||||
(`eh_spike_test.cpp`) exercises sleep-across-throw, fiber-swap-then-throw, and
|
||||
suspend-inside-catch. Under native wasm-EH: **sleep ✅, fiber ✅, suspend-in-catch ❌
|
||||
(hard trap: `indirect call to null`).** Identical in V8 (node) and Firefox. So the *only*
|
||||
failure mode is the documented one (Binaryen #4470: AsyncifyFlow skips catch bodies), and
|
||||
it fails **loudly and deterministically** — not a silent corruption.
|
||||
4. **A ~150-line Binaryen pass closes it.** `--hoist-cpp-catches`
|
||||
(`binaryen/src/passes/HoistCppCatches.cpp`, our fork) flips suspend-in-catch to **green**
|
||||
in both engines. `tests/asyncify/eh-spike.spec.ts` is a 3-variant ablation harness pinning
|
||||
JS-EH-green / wasm-EH-red / wasm-EH+hoist-green. Rebuilt and re-verified end-to-end from the
|
||||
tracked submodule.
|
||||
|
||||
## The artifacts (all reproducible)
|
||||
|
||||
| Thing | Where |
|
||||
|---|---|
|
||||
| Red-green toy (3 mechanisms) | `tests/apps/standalone/eh-spike/eh_spike_test.cpp` |
|
||||
| Phase-0 probe | `tests/apps/standalone/eh-spike/eh_probe.cpp` (+ `eh-probe` Makefile target) |
|
||||
| 3-variant build (stub→post-link-v130, +hoist) | `scripts/build-eh-spike.sh` |
|
||||
| The Binaryen pass | `binaryen/` submodule (fork, branch `wasm-port` = `version_130` + the pass) |
|
||||
| Fork build wrapper | `scripts/binaryen-hoist-pass/build-wasm-opt.sh` |
|
||||
| Red-green-fixed spec | `tests/asyncify/eh-spike.spec.ts` |
|
||||
|
||||
## How the pass works (one paragraph)
|
||||
|
||||
For a `try` whose cpp-tag `catch` arm contains a suspending call, it rewrites the arm to just
|
||||
*capture the exception payload into a local and set a flag*, and **hoists the real handler to
|
||||
plain straight-line code after the try**, guarded by the flag. In the hoisted handler the
|
||||
payload `pop` becomes a `local.get`, and the personality no-match `rethrow` becomes an explicit
|
||||
`throw` of the cpp tag with the captured payload. Stock `--asyncify` then instruments the
|
||||
hoisted handler like any other code — the upstream "no pause/resume inside catchBodies"
|
||||
invariant becomes true *by construction*. It is a **pre-pass**; `Asyncify.cpp` is unchanged,
|
||||
which is why it's a clean ~150-line addition and genuinely upstreamable.
|
||||
|
||||
## Generalization (follow-up, same session)
|
||||
|
||||
The pass was generalized from the MVP (one direct-suspend catch) to **hoist-all-cpp-catches** and
|
||||
tested against a richer toy covering the real KiCad/wx shapes. **All 7 shapes are green in all
|
||||
three engines** (Firefox + Chrome + Safari/WebKit):
|
||||
|
||||
| shape | covered |
|
||||
|---|---|
|
||||
| direct suspend in catch | ✅ |
|
||||
| transitive (catch → helper → … → suspend) | ✅ (hoist-all; direct detection would miss it) |
|
||||
| value-returning try/catch | ✅ (LLVM keeps the value in a local → void try, no result routing needed) |
|
||||
| suspend-in-catch on a fiber/coroutine stack (eeschema Paste) | ✅ |
|
||||
| nested suspend-in-catch tries | ✅ |
|
||||
| **catch nested in a catch_all cleanup** (try body has a local with a destructor) | ✅ (escape past the outermost try; see below) |
|
||||
|
||||
**The one gap — catch_all-wrapped catches.** When the try body holds a local with a non-trivial
|
||||
destructor, LLVM lowers the C++ catch *nested inside* the cleanup `catch_all`
|
||||
(`catch_all { ~g; try { rethrow } catch $cpp { sleep } }`). Hoisting the cpp catch leaves the sleep
|
||||
inside the `catch_all`; legacy `catch_all` gives no payload to capture/re-raise, so the sleep can
|
||||
only be freed by hoisting **past the outermost enclosing try** — have the cpp catch capture the
|
||||
payload and `br` to a `$done` block placed after that try. **Prototyped this session and
|
||||
reverted:** the escape transform *validates* and fixes the catch_all case *in isolation*, but the
|
||||
`block` + `br` + flag-dispatch control-flow shape it produces is **not asyncify-rewindable**
|
||||
(rewind traps with `null function`) and it regressed the simple cases too. So the real work is
|
||||
finding an asyncify-friendly escape shape — the per-try inline `br_if`-skip form (handler inline
|
||||
right after the try) rewinds fine; a `br` out to a separate dispatch does not. That's the
|
||||
fix is now landed (see "catch_all-escape: LANDED" below); all 7 shapes are green. How often it
|
||||
bites KiCad depends on whether the specific catch's try body constructs a destructible
|
||||
local/temporary (e.g. a `wxString`); a `try { ptr = Load(fn); } catch(IO_ERROR&)` with a pointer
|
||||
result has no cleanup pad and is already covered. (`HOIST_ONLY_SUSPEND` switches off hoist-all back
|
||||
to direct-suspend-only, useful for narrowing blast radius while debugging.)
|
||||
|
||||
### catch_all-escape: LANDED (2026-06-22)
|
||||
|
||||
**Fixed — all 7 shapes green in Firefox + Chrome + Safari.** When the try body holds a local with a
|
||||
non-trivial destructor, LLVM lowers the C++ catch *nested inside* the cleanup `catch_all`; the pass
|
||||
hoists it PAST the outermost enclosing try. Confirmed to occur in real KiCad (`pcbnew/files.cpp:670`
|
||||
builds `std::map<std::string, UTF8> props` in the try, so its IO/format/bad_alloc catches are
|
||||
catch_all-nested). Landing it took a from-source Binaryen build + a minimal *multi-function* repro
|
||||
(`/tmp/eh_min2.cpp`, `/tmp/eh_min3.cpp`); two bugs, both invisible on a single function and only live
|
||||
once several shapes inline together:
|
||||
|
||||
1. **Over-eager deferral → `null function`.** A nested cpp catch is deferred to its ancestor escape
|
||||
target, but the test matched ANY ancestor catch body — so a cpp catch in a *regular* catch body
|
||||
(the `__cxa_end_catch` cleanup tries LLVM emits everywhere) was deferred to a target that never
|
||||
hoisted it; its suspend was dropped and rewind trapped. Fix: defer only when the catch sits in an
|
||||
ancestor's `catch_all` cleanup pad (`hasCatchAll() && catchBodies.back() == child`).
|
||||
2. **Trailing catch_all code → `unreachable`.** The rewritten minimal arm completes, but the
|
||||
catch_all body has trailing `(unreachable)` after the nested try (it assumed the handler
|
||||
diverged). Fix: wrap the escape target in a `block $esc`; the arm `br $esc`s after capturing,
|
||||
landing fall-through just before the dispatch (so Asyncify still rewinds the handler).
|
||||
|
||||
The pass is in the `binaryen` submodule (`src/passes/HoistCppCatches.cpp`). The earlier
|
||||
"not asyncify-rewindable" worry was wrong — Asyncify rewinds the escape form fine; the blockers were
|
||||
ordinary IR bugs, exactly as the multi-function-debug plan predicted.
|
||||
|
||||
### value-typed (concrete-result) tries: LANDED (2026-06-22)
|
||||
|
||||
The pass also handles an escape target whose try yields a *value* (i32/i64/…), not just
|
||||
void/unreachable — it routes the body/handler value through a `$result` local (the no-exception
|
||||
body value is captured inside `block $esc`; a caught arm br's out and each per-arm dispatch writes
|
||||
`$result`; the block yields `local.get $result`). Non-defaultable result types are still skipped.
|
||||
These tries don't arise from normal C++ EH lowering (LLVM keeps catch values in locals →
|
||||
void/unreachable tries), so they're covered by hand-written modules in
|
||||
`scripts/binaryen-hoist-pass/tests/` (`run.sh`): `--fuzz-exec` confirms the pass preserves the
|
||||
result value across the exception / no-exception / payload paths, and a real asyncify unwind+rewind
|
||||
through a value-typed *suspending* catch yields the correct value (50).
|
||||
|
||||
#### Debugging history (superseded)
|
||||
|
||||
The notes below trace the path to the fix; their "blocked" conclusions are superseded by the
|
||||
landing above.
|
||||
|
||||
##### catch_all-escape: confirmed real; the fix is asyncify-SOUND, not a wall (2026-06-22)
|
||||
|
||||
> **Correction (later same session):** the "not asyncify-rewindable" conclusion below was
|
||||
> **disproven**. Diffing the *asyncified* output of the per-try vs escape forms on a minimal
|
||||
> single-function suspend-in-catch (`/tmp/eh_min.cpp`) shows them **structurally identical** — all
|
||||
> 22 diff hunks are pure local-index renumbering — and **both run cleanly in node**. So Asyncify
|
||||
> rewinds the escape form fine. The real blocker is ordinary structural bugs in the escape pass on
|
||||
> the complex *inlined* toy (one found: the skip-to-escape-target coordination drops a catch when
|
||||
> its escape target is value-typed; fixing that surfaced a load-time trap, so there's ≥1 more).
|
||||
> That is tractable engineering — methodical per-function isolation like the eh_min repro — **not**
|
||||
> an asyncify-internals wall. WIP + partial fix preserved in the escape-wip file below.
|
||||
|
||||
> **Further localization (same session):** the breakage is **not** fiber-specific (cases 1/3/4/5
|
||||
> with no fibers still trap) and **not** one bug. It is a **layout-sensitive structural corruption**
|
||||
> the escape restructure introduces on MULTI-function modules — `null function` / wrong
|
||||
> `call_indirect`, which V8 then mis-compiles unpredictably (the trap point *moves* with module
|
||||
> composition). Single-function repros (`eh_min`) work; the corruption only appears once several
|
||||
> functions/cases compile together. So the next step is NOT more single-function isolation but a
|
||||
> small **multi-function** repro under a Binaryen **debug build (assertions)** + `--fuzz-exec`, to
|
||||
> catch the exact expression the restructure corrupts. Deferred to dedicated debugging.
|
||||
|
||||
**Confirmed we DO hit the gap.** A spot-check of the audited sites found destructible locals in the
|
||||
try bodies: e.g. `pcbnew/files.cpp:670` declares `std::map<std::string, UTF8> props;` in the try,
|
||||
so its three `catch (… ) { DisplayErrorMessage(…) }` arms are lowered nested inside a cleanup
|
||||
`catch_all`. The file-load sites generally construct `wxString`/`std::map`/smart-pointer locals, so
|
||||
this is not academic — a real subset of the ~85 sites is affected.
|
||||
|
||||
**The fix was attempted extensively and is blocked.** The escape-target restructure (hoist the cpp
|
||||
catch — own or nested — past the outermost enclosing try, dispatching handlers after it) **validates**
|
||||
in every variant but is **not asyncify-rewindable**: it traps with `null function` even on the simple
|
||||
cases the per-try form handles. Tried: inline flag-dispatch (`if (flag==n)` — asyncify skips `if`
|
||||
bodies on rewind), a bare single handler, `br_if`-skip guards, and `ReFinalize` (for stale `Try`
|
||||
types). None worked at the time — the actual root causes (over-eager deferral + trailing catch_all code)
|
||||
were found later with a multi-function repro; the fix landed in the submodule pass (see above).
|
||||
|
||||
**Open options:** (1) diff the *asyncified* IR of the working per-try form vs the escape form on one
|
||||
simple case, to pinpoint exactly what Asyncify mis-handles; (2) hand-refactor the affected KiCad
|
||||
sites (move the destructible local out of the try body) — a targeted subset, not all 85; (3) the new
|
||||
`exnref` EH encoding gives `catch_all` a payload (a clean fix) but Asyncify has no `exnref` support.
|
||||
The per-try pass (6/7 shapes) is the shipped state.
|
||||
|
||||
## Opinions (the part you asked for)
|
||||
|
||||
**1. Do it — the size/perf prize is real and the risk is now bounded.** −44% download / −35%
|
||||
module (measured, see 02) plus a large `-O2` build-time drop. The thing everyone feared
|
||||
(Asyncify ⊥ wasm-EH) is disproven. I would not have said this before the spike; I say it now
|
||||
because the toy actually runs.
|
||||
|
||||
**2. Switch the pass from "selective" to "hoist-all-cpp-catches" before KiCad.** My MVP only
|
||||
hoists arms with a *direct* suspending-import call. KiCad's real pattern is **transitive** —
|
||||
`catch (IO_ERROR&) { DisplayErrorMessage(...); }` → `ShowModal` → `startModal` — so direct
|
||||
detection would miss most of the 85 audited sites. The design doc already recommends hoist-all
|
||||
+ let `-O2` prune the no-op hoists, and having now written the selective version I agree: it
|
||||
removes the call-graph analysis entirely, is robust to transitivity, and the only cost is
|
||||
transforming more tries (which `-O2` coalesces). Selective was the right call for *proving the
|
||||
concept with minimal blast radius*; hoist-all is the right call for *shipping*.
|
||||
|
||||
**3. The remaining pass work is small (~1–2 days), and I know exactly what it is.** (a)
|
||||
Concrete-result-typed tries — route the body value through a temp local (the toy already
|
||||
forced me to handle `unreachable`-typed; `i32`/others are the same shape). (b) `catch_all`
|
||||
coexistence on the same try (cpp catch + cleanup pad). (c) hoist-all gating. None are research;
|
||||
all are mechanical Binaryen-IR work. The two real-IR gotchas are already solved in the MVP:
|
||||
**nested-catch pops** (don't clobber a nested catch's payload — fixed with a Try-ancestor guard)
|
||||
and **nested suspend-in-catch tries** (KiCad will have these; the toy already did, and hoisting
|
||||
*both* was required).
|
||||
|
||||
**4. The schedule risk is the emsdk bump, not exceptions.** Everything above avoided the bump.
|
||||
Full KiCad cannot: OpenCASCADE miscompiles a `br_table` under wasm-EH on em 4.0.2 (candidate
|
||||
LLVM fix exists; em 6.0.0 = LLVM 23 should cover it), and the bump changes the compiler for the
|
||||
*JS-EH build too* → whole-project revalidation. That is the multi-week, cross-cutting item.
|
||||
Budget the migration as "1–2 days pass + N weeks emsdk-bump-and-revalidate," not the reverse.
|
||||
|
||||
**5. Keep the legacy encoding; ignore the exnref/Chrome noise.** Asyncify can only instrument
|
||||
legacy `try/catch` (no roadmap to change through v130), and legacy ships unflagged everywhere
|
||||
since 2021. The size win is native-vs-JS EH, independent of the encoding — so legacy is free
|
||||
and correct. The exnref "Chrome problem" never touches us.
|
||||
|
||||
**6. Honest caveat — the toy is small; scale is unproven.** This spike de-risks the *semantic*
|
||||
interaction, not KiCad-scale behavior. Two known scale hazards remain untested under wasm-EH:
|
||||
V8's per-function locals limit on huge asyncified functions
|
||||
([[chrome-asyncify-rewind-crash]]) and unwind-time landing-pad reliability
|
||||
([[asyncify-eh-unwind-landing-pads-unreliable]] — which *might improve* under wasm-EH, worth
|
||||
re-checking). The Safari 26.0 transient legacy-EH crash (emscripten #25365, since patched) is a
|
||||
reminder that even legacy can break on a fresh engine. None of these are blockers; all are
|
||||
"verify at scale," and the recommended order (toy → wx app → full KiCad) is designed to surface
|
||||
them cheaply.
|
||||
|
||||
## Recommended path forward
|
||||
|
||||
1. **Generalize the pass** — DONE: hoist-all, catch_all-escape, and value-typed/concrete-result
|
||||
tries are all handled and verified (the value-typed path via `scripts/binaryen-hoist-pass/tests/`).
|
||||
No further generalization is needed for the 7 KiCad shapes.
|
||||
2. **Phase 2 — a wx standalone app** flipped to native EH: the first *real* `ShowModal`-from-catch
|
||||
path, and the forcing function for transitive hoisting.
|
||||
3. **File the design on binaryen #4470** (the pass is a pure addition; upstreaming collapses our
|
||||
fork back into stock wasm-opt eventually).
|
||||
4. **Phase 3 — full KiCad**, gated on the emsdk bump (the real work) + the generalized pass +
|
||||
the uniform flag flip + dropping `env.invoke_*` from `apply-asyncify.sh`, with an e2e audit
|
||||
under `-sASYNCIFY_ASSERTIONS`.
|
||||
|
||||
## Status of the tracked changes (for review)
|
||||
|
||||
- `binaryen/` submodule added (fork `emergence-engineering/binaryen`), branch **`wasm-port`** at
|
||||
`version_130 + 1` (`58f25ebb2`) — the pass is **committed in the submodule but not pushed**.
|
||||
Pushing the branch to the fork (and committing the parent gitlink) is the user's call.
|
||||
- Parent-repo changes are **uncommitted**, pending review: `.gitmodules` + the `binaryen`
|
||||
gitlink, the spike toy/scripts/spec, and these dossier docs.
|
||||
175
docs/features/wasm-exceptions/08-wx-app-render-rootcause.md
Normal file
175
docs/features/wasm-exceptions/08-wx-app-render-rootcause.md
Normal file
|
|
@ -0,0 +1,175 @@
|
|||
# 08 — Native wasm-EH: the wx application (Phase 2) — render-failure root cause & fix (2026-06-22)
|
||||
|
||||
> Phase 2 of the plan in [`07-spike-results-and-opinion.md`](07-spike-results-and-opinion.md):
|
||||
> flip a real wx standalone app (the `dialog` test) to `-fwasm-exceptions` end-to-end and see if
|
||||
> it runs. It builds and links clean, but rendered **blank**. This documents the deep-debug that
|
||||
> found *why*, the one-line-of-reasoning root cause, the fix, and an **honest account of what is
|
||||
> proven vs. still open** — including a render/screenshot discrepancy that is not yet resolved.
|
||||
|
||||
## Status (read this first)
|
||||
|
||||
- **Proven, C++-level:** the app was **destroying its own main window during startup** under
|
||||
native wasm-EH. Root cause identified with certainty (instrumented build), and the fix makes
|
||||
the destruction **stop** (the `~wxNonOwnedWindow` destructor no longer fires). That specific bug
|
||||
is fixed, and the *why* is understood and re-derivable.
|
||||
- **Proven, in my checks:** after the fix, a headless-Chromium load of `dialog_test.html` showed a
|
||||
full render — `#canvas` present and visible, **5 buttons**, the description text, the event-log
|
||||
control, the status bar; `canvases=1`, `traps=0`; and the screenshot I captured showed the
|
||||
complete dialog UI.
|
||||
- **OPEN / unresolved:** the screenshot is reported **empty** on inspection. My headless ad-hoc
|
||||
check and that observation **disagree**, and I have not reconciled them. **Do not treat the app
|
||||
as "verified rendering" yet.** See [§Open: the empty-screenshot discrepancy](#open-the-empty-screenshot-discrepancy).
|
||||
- **Not yet done:** the real e2e spec in all three browsers; modal dialogs (which now nest Asyncify
|
||||
one level deeper); the rest of the wx suite; cleanup/commit of the Phase-2 changes.
|
||||
|
||||
## The symptom
|
||||
|
||||
`dialog` built and linked under `-fwasm-exceptions -sWASM_LEGACY_EXCEPTIONS=1` (libwx + app, with
|
||||
the post-link hoist+asyncify pipeline). At runtime: boots, prints its startup logs, **no JS error,
|
||||
no wasm trap, main thread responsive** — but `#window-container` empty, no visible canvas, the
|
||||
e2e `waitForApp` (waits for a visible `#canvas`) would time out. A silent non-render.
|
||||
|
||||
## How it was found (the debug chain)
|
||||
|
||||
Each step ruled out a hypothesis and narrowed the next. All via injected logging in the built
|
||||
glue + instrumented libwx rebuilds (the browser symbolizes wasm frames only as `wasm-function[N]`,
|
||||
so callstack mapping was a dead end on a release build — direct source instrumentation was the
|
||||
reliable tool).
|
||||
|
||||
1. **It's not a trap or asyncify/indirect-call corruption.** Calling a wasm export (`ProcessEvents`)
|
||||
from JS post-boot returns cleanly. The "table index out of bounds" seen earlier was an artifact
|
||||
of my own `Module.Asyncify` probe, not the app.
|
||||
2. **`main` "throws `unwind`"** — but that is **normal**: it's `emscripten_exit_with_live_runtime`'s
|
||||
sentinel, caught and swallowed by `handleException` (glue line ~5066). A red herring on its own.
|
||||
3. **The main window is created, then destroyed.** `createWindow(id=-1 → cssId 0)` runs in the frame
|
||||
ctor; then `destroyWindow(0)` runs — the DOM window is torn down. `wxNonOwnedWindow::~wxNonOwnedWindow`
|
||||
is the caller (it `EM_ASM`s `destroyWindow(m_cssId)`). So **the frame's own destructor runs during
|
||||
startup**, leaving the app with no window.
|
||||
4. **The destruction is deliberate, not an exception unwind.** Instrumented `~wxNonOwnedWindow` to log
|
||||
`std::uncaught_exceptions()` → **0**. So no C++ exception is in flight; this is a normal destructor
|
||||
call. (This momentarily looked like it ruled out the landing-pad hazard — it didn't; see root cause.)
|
||||
5. **It happens *after* `OnInit` fully completes.** Logged `OnInit`: "frame created" → "Show done,
|
||||
returning true" both print *before* the destructor. So the teardown is in **`OnRun`**, not OnInit.
|
||||
6. **It happens *inside* `emscripten_set_main_loop`.** Bracketed `wxGUIEventLoop::DoRun`'s
|
||||
`SetSize`/`Refresh`/`set_main_loop` with logs. Order: "before set_main_loop" → **then** the
|
||||
`~wxNonOwnedWindow`. So the frame dies *during* the `emscripten_set_main_loop(ProcessEvents, 0, 1)`
|
||||
call.
|
||||
|
||||
## Root cause (one paragraph)
|
||||
|
||||
`emscripten_set_main_loop(fn, fps, simulate_infinite_loop=1)` implements "loop forever" by **throwing
|
||||
a JS `"unwind"` exception to abandon the C++ stack** — the code after it never runs; the browser drives
|
||||
`fn` thereafter. That `"unwind"` propagates out through every C++ frame between `set_main_loop` and
|
||||
`callMain`. Under **native wasm-EH**, the compiler emits `catch_all` **cleanup** landing pads (for
|
||||
destructors/RAII) that **reliably catch any in-flight exception — including a foreign JS one** — run
|
||||
their cleanup, and rethrow. As the `"unwind"` passes back through `wxEntry`/`OnRun`, those cleanup pads
|
||||
fire and **destroy `wxTopLevelWindows.front()` — the main frame** — before the browser ever calls
|
||||
`ProcessEvents` to paint it. `uncaught_exceptions()==0` is consistent: the `"unwind"` is a *JS*
|
||||
exception, invisible to the C++ exception machinery, so the cleanup-pad destructors see no C++ unwind
|
||||
in progress.
|
||||
|
||||
This is the **inverse** of the documented hazard
|
||||
[`asyncify-eh-unwind-landing-pads-unreliable`]: under legacy `-fexceptions` the cleanup landing pads
|
||||
fire **unreliably**, and that *accidentally* spared the frame (the destroy that should run, didn't).
|
||||
Native wasm-EH makes them reliable — so the latent "abandon-the-stack vs. run-the-cleanup" conflict
|
||||
finally bites. The JS-EH build never rendered-correctly-by-design here; it rendered correctly **by a
|
||||
landing-pad bug canceling a stack-abandon assumption.**
|
||||
|
||||
## The fix
|
||||
|
||||
> **Superseded form (2026-06-23):** the fix described in this section is the *interim* **option A** (`wxWasmRunNestedLoop` / `setTimeout` pump). The final form is the **rAF pump** `wxWasmParkMainLoop` (keeps `requestAnimationFrame`, drops `emscripten_set_main_loop` entirely) — see [`09`](09-event-loop-deparking-plan.md). Both share the root insight (suspend, don't `throw`); the **root cause above is unchanged**. Note the de-park **regresses the coroutine suite** (Asyncify-nesting wall), fixed by [`../async/12`](../async/12-design-b-asyncify-implementation-plan.md) + [`../async/13`](../async/13-design-b-engineering-spec.md).
|
||||
|
||||
Drive the **top-level** event loop via **Asyncify** instead of `set_main_loop`'s
|
||||
abandon-the-stack `"unwind"` — i.e. the **same mechanism the nested/quasi-modal loops already use**
|
||||
(`wxWasmRunNestedLoop`, an `EM_ASYNC_JS` that suspends via Asyncify and pumps `ProcessEvents` from a
|
||||
`setTimeout` loop). Asyncify suspends with a **return-based** unwind that **saves** the stack rather
|
||||
than abandoning it: no `"unwind"` JS exception is thrown, so no `catch_all` cleanup pad fires, so the
|
||||
frame survives. `ProcessEvents` is then driven by the JS `setTimeout(17ms)` pump instead of
|
||||
`requestAnimationFrame`.
|
||||
|
||||
`src/wasm/evtloop.cpp`:
|
||||
- `wxGUIEventLoop::DoRun` — the first (top-level) `DoRun` no longer falls through to
|
||||
`emscripten_set_main_loop(ProcessEvents, 0, 1)`; it does the initial top-window `SetSize`/`Refresh`
|
||||
and then calls `wxWasmRunNestedLoop()`, exactly like a nested loop. Both levels now share one path.
|
||||
- `wxGUIEventLoop::ScheduleExit` — always `wxWasmExitNestedLoop()` (resolve the innermost pump);
|
||||
dropped the top-level `emscripten_cancel_main_loop()` branch (there is no `set_main_loop` to cancel).
|
||||
|
||||
### Why this fix and not the alternatives
|
||||
|
||||
- **`simulate_infinite_loop=0`** (don't throw): then `DoRun` *returns*, `OnRun` returns, and `wxEntry`
|
||||
runs its **normal** teardown (deletes the TLWs) and exits — same dead frame, plus the app exits.
|
||||
Doesn't help.
|
||||
- **Suppress/avoid the cleanup pads:** they're compiler-generated; you can't selectively disable the
|
||||
one that catches `"unwind"`. Not actionable.
|
||||
- **Asyncify the top loop:** it's the existing, tested suspension primitive in this codebase, it
|
||||
*saves* the stack (no abandon → no foreign-exception propagation through cleanup pads), and it
|
||||
unifies top-level and nested loops on one mechanism. This is the minimal, principled change, and it
|
||||
lives in the wasm port layer (`src/wasm/`), per the "fix in the wasm layer" policy.
|
||||
|
||||
## Evidence
|
||||
|
||||
- **Before fix:** `~wxNonOwnedWindow cssId=0 uncaught=0` fires right after "before set_main_loop";
|
||||
`#window-container` empty.
|
||||
- **After fix:** `~wxNonOwnedWindow` **no longer fires** at startup (definitive C++-level signal the
|
||||
frame survives); headless load reports `canvas:true, canvasVisible:true, buttons:5`, body text =
|
||||
"wxDialog and wxMessageBox Test…", `canvases=1`, `traps=0`; rebuilt clean (debug logging removed)
|
||||
and re-checked → same.
|
||||
|
||||
## Open: the empty-screenshot discrepancy
|
||||
|
||||
**My headless-Chromium screenshot showed the full dialog; on inspection the screenshot is reported
|
||||
empty. These disagree and I have not reconciled them.** Until resolved, the app is **not** confirmed
|
||||
rendering. Candidate explanations, to check in order:
|
||||
|
||||
1. **Stale image** — an earlier (pre-fix) empty capture vs. the post-fix one. Cheapest to rule out.
|
||||
2. **Headless vs. headed / real engine** — my check was headless Chromium; a real/headed browser
|
||||
(esp. WebKit/Firefox) may differ. The whole point of project policy is **all three engines**;
|
||||
I only spot-checked one, headless.
|
||||
3. **Ad-hoc load vs. the real e2e spec** — my load waits a fixed 6 s; the spec has its own
|
||||
`waitForApp`/timing and asserts against **tracked baseline screenshots**. The spec is the
|
||||
authoritative render check and I have **not** run it yet.
|
||||
4. **A separate, still-present rendering issue** — the frame-destruction fix is proven, but a
|
||||
*different* paint/canvas problem could remain (e.g. the canvas drawing path, or DOM-widget vs.
|
||||
canvas content). The C++ signal (destructor no longer firing) proves the *frame* lives; it does
|
||||
**not** prove every pixel paints.
|
||||
|
||||
**Immediate next step:** run `tests/.../dialog` through the real e2e spec in **Firefox + Chrome +
|
||||
Safari (WebKit)** and compare to the baseline screenshots — that reconciles the discrepancy and is
|
||||
the real Phase-2 acceptance gate.
|
||||
|
||||
## Build-system decisions made for Phase 2 (for review)
|
||||
|
||||
All gated so the default (JS-EH) build is unchanged; native EH is opt-in via `WX_NATIVE_EH=1`.
|
||||
|
||||
- **`scripts/build-wx-wasm.sh`** — `WX_NATIVE_EH=1` swaps `-fexceptions` for
|
||||
`-fwasm-exceptions -sSUPPORT_LONGJMP=wasm -sWASM_LEGACY_EXCEPTIONS=1` in C/CXXFLAGS. (The whole
|
||||
libwx + every app must share one EH model — EH ABI is all-or-nothing.)
|
||||
- **`tests/apps/Makefile.wasm`** — `WX_NATIVE_EH` adds the same EH flags + `-sDYNCALLS=1` to app
|
||||
CXXFLAGS/LDFLAGS.
|
||||
- **`scripts/build-wasm-test.sh`** — under `WX_NATIVE_EH`, **stubs the emsdk-bundled `wasm-opt`**
|
||||
(v121 crashes asyncifying wasm-EH) so the in-link Asyncify no-ops, then post-link runs the real
|
||||
pipeline on Binaryen v130 over each freshly-linked app + injects the dyncall shims.
|
||||
- **`scripts/common/hoist-and-asyncify.sh`** (new) — the post-link pipeline:
|
||||
`--hoist-cpp-catches` (our fork pass) → `--asyncify` → `-O2`, all on v130. `HOIST_KEEP_NAMES=1`
|
||||
preserves the names section through `-O2` (added for the callstack debugging here).
|
||||
- **`src/wasm/evtloop.cpp`** — the loop fix above (the only behavioral wx-port change).
|
||||
|
||||
## Implications beyond `dialog`
|
||||
|
||||
- **Modals now nest Asyncify two levels deep.** Previously: top = `set_main_loop` (no Asyncify
|
||||
suspend on the main stack), modal = Asyncify (1 level). Now: top = Asyncify, modal = Asyncify
|
||||
(2 levels). This leans harder on the nested-`currData` save/restore in `handlesleep.js`
|
||||
([`asyncify-park-throw-root-cause`]). **Must be tested** (open the Custom/Input dialogs).
|
||||
- **KiCad uses the same `evtloop.cpp`.** If this fix holds for wx apps, it's the same fix KiCad
|
||||
needs under native EH — and it means the `set_main_loop`-`"unwind"` conflict is a **general**
|
||||
wasm-EH×wx-DOM-port interaction, not a `dialog`-specific quirk. This is exactly the kind of
|
||||
"scale hazard" 07 §6 flagged ("unwind-time landing-pad reliability … might *change* under wasm-EH")
|
||||
— it changed, and here's the concrete consequence + remedy.
|
||||
|
||||
## Honest verdict
|
||||
|
||||
The deep-debug **succeeded at the hard part**: a silent blank-render is now a fully understood,
|
||||
evidence-backed root cause with a minimal, principled fix, and the specific bug (frame self-destruct)
|
||||
is provably gone. But Phase 2's acceptance bar — *the app verifiably renders and is interactive in all
|
||||
three browsers via the e2e spec* — is **not met yet**, and the empty-screenshot observation is an
|
||||
unresolved flag against it. Next action is reconciliation via the real spec, not more root-causing.
|
||||
160
docs/features/wasm-exceptions/09-event-loop-deparking-plan.md
Normal file
160
docs/features/wasm-exceptions/09-event-loop-deparking-plan.md
Normal file
|
|
@ -0,0 +1,160 @@
|
|||
# 09 — Event-loop de-parking: one EH-agnostic main loop (plan + verification) (2026-06-23)
|
||||
|
||||
> Plan to replace the wx top-level event loop's stack-abandoning `throw "unwind"` with an
|
||||
> **Asyncify de-park**, written **in C++ (`evtloop.cpp`), not as a post-link shim**, so a **single
|
||||
> code path works under both `-fexceptions` (JS-EH) and `-fwasm-exceptions` (native EH)**.
|
||||
> Companion to [`08-wx-app-render-rootcause.md`](08-wx-app-render-rootcause.md) (root cause) and to
|
||||
> the async dossier's de-park analysis (`docs/features/async/`). Ends with the **old-vs-new
|
||||
> test+screenshot verification matrix**.
|
||||
|
||||
## Decision (verdict up front)
|
||||
|
||||
The top-level `wxGUIEventLoop::DoRun` will stop using `emscripten_set_main_loop(..., simulate_infinite_loop=1)` (which `throw "unwind"`s to abandon the C++ stack). Instead:
|
||||
|
||||
```cpp
|
||||
// top-level DoRun:
|
||||
wxWasmParkMainLoop(); // suspend the C++ stack + drive ProcessEvents from an rAF loop
|
||||
// that calls it via the ASYNC ccall (Asyncify-aware).
|
||||
// NOT emscripten_set_main_loop — see the Correction note below.
|
||||
```
|
||||
|
||||
- **No `throw`** → nothing for native wasm-EH's `catch_all` cleanup pads to catch → the main frame is not destroyed (the 08 bug).
|
||||
- **No `throw`** under JS-EH either → behaves exactly like today minus the (JS-EH-harmless) throw.
|
||||
- **One source, both models** — no `#ifdef WX_NATIVE_EH`, no shim, no `--js-library`. Lives in the wx wasm port where the loop already lives.
|
||||
- **Keeps `requestAnimationFrame`** — `wxWasmParkMainLoop` drives `ProcessEvents` from an rAF loop (not `setTimeout`), vsync-aligned like the original. (Option A used `setTimeout`; this supersedes it.)
|
||||
|
||||
## Background (recap of the 08 bug)
|
||||
|
||||
`emscripten_set_main_loop(ProcessEvents, 0, 1)` registers the rAF loop and then `throw "unwind"` to abandon the C++ stack (so the code after it never runs and the browser drives `ProcessEvents`). Under `-fwasm-exceptions`, the compiler emits real `catch_all` cleanup landing pads; as the foreign `"unwind"` JS exception propagates out of `main`, those pads **catch it and run destructors**, tearing down `wxTopLevelWindows.front()` (the main frame) before first paint → blank render. Under JS-EH the same throw is harmless because legacy `-fexceptions` landing pads fire *unreliably* and `noExitRuntime=true` means the throw is swallowed by `handleException` with no destructors run.
|
||||
|
||||
## The mechanism in detail
|
||||
|
||||
The throw bundles two jobs: (1) register the rAF loop, (2) abandon the stack. `simulate_infinite_loop` is a parameter, so we split them: pass `0` (register + return, no throw), then keep the stack alive ourselves with a **bare park**.
|
||||
|
||||
`wxWasmParkMainLoop` is `wxWasmRunNestedLoop` **minus its `setTimeout` pump** — because here `emscripten_set_main_loop`'s rAF already drives `ProcessEvents`, so a pump would double-drive it. It only `await`s a Promise registered on the existing `Module._wxNestedLoopExit` LIFO:
|
||||
|
||||
```cpp
|
||||
EM_ASYNC_JS(void, wxWasmParkMainLoop, (), {
|
||||
Module._wxNestedLoopExit = Module._wxNestedLoopExit || [];
|
||||
await new Promise(function (resolve) {
|
||||
var finish = function () {
|
||||
var idx = Module._wxNestedLoopExit.indexOf(finish);
|
||||
if (idx !== -1) Module._wxNestedLoopExit.splice(idx, 1);
|
||||
resolve();
|
||||
};
|
||||
Module._wxNestedLoopExit.push(finish);
|
||||
// no pump: emscripten_set_main_loop's rAF drives ProcessEvents
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
`DoRun` becomes:
|
||||
```cpp
|
||||
int wxGUIEventLoop::DoRun() {
|
||||
bool topLevel = (s_wxRunDepth++ == 0);
|
||||
if (topLevel) {
|
||||
// initial sizing as today
|
||||
...SetSize/Refresh on wxTopLevelWindows.front()...
|
||||
emscripten_set_main_loop(ProcessEvents, 0, 0); // rAF, no throw
|
||||
wxWasmParkMainLoop(); // suspend until exit
|
||||
} else {
|
||||
wxWasmRunNestedLoop(); // nested: unchanged (setTimeout pump)
|
||||
}
|
||||
--s_wxRunDepth;
|
||||
return 0;
|
||||
}
|
||||
```
|
||||
|
||||
`ScheduleExit` cancels rAF for the top level before resolving (so no stray rAF tick calls `ProcessEvents` on the app being torn down), then resolves the innermost loop:
|
||||
```cpp
|
||||
void wxGUIEventLoop::ScheduleExit(int) {
|
||||
m_shouldExit = true;
|
||||
if (s_wxRunDepth == 1) emscripten_cancel_main_loop(); // top-level: stop rAF
|
||||
wxWasmExitNestedLoop(); // resolve park (top) or pump (nested)
|
||||
}
|
||||
```
|
||||
|
||||
**Nested/modal loops are unchanged** — they still use `wxWasmRunNestedLoop` (rAF isn't available while nested). Only the top level changes, and only from "abandon-via-throw" to "register-rAF + suspend-via-park."
|
||||
|
||||
## Correction: rAF pump, not `emscripten_set_main_loop`
|
||||
|
||||
The first implementation used `emscripten_set_main_loop(ProcessEvents, 0, 0)` to register rAF + a *bare* park (as "The mechanism in detail" above describes). **It renders white/blank.** `set_main_loop`'s rAF callback (`MainLoop.runIter`) calls `ProcessEvents` **synchronously**, and a synchronous call cannot drive the runtime while `main` is Asyncify-**parked** — the loop stalls after ~6 frames (measured: rafCount **6** vs **~348** for a live loop), so the window never gets its first `Paint` and stays browser-white. (Clicks still work via direct DOM→wx handlers; the first modal's `setTimeout` pump then paints, which masked it in shots 02-05.)
|
||||
|
||||
**Fix — the form now in `evtloop.cpp`:** `wxWasmParkMainLoop` is a hand-rolled `requestAnimationFrame` pump that calls `ProcessEvents` via the **async** `ccall(..., {async:true})` (Asyncify-aware → works on the parked runtime). `DoRun` calls only `wxWasmParkMainLoop()` (no `set_main_loop`); `ScheduleExit` is only `wxWasmExitNestedLoop()` (the pump's `finish()` sets `stopped=true`, stopping it before teardown). It differs from `wxWasmRunNestedLoop` only in rAF vs `setTimeout`.
|
||||
|
||||
```cpp
|
||||
EM_ASYNC_JS(void, wxWasmParkMainLoop, (), {
|
||||
var stopped = false, finish = null;
|
||||
var pump = function () {
|
||||
if (stopped) return;
|
||||
requestAnimationFrame(async function () {
|
||||
if (stopped) return;
|
||||
try { await ccall('ProcessEvents', 'void', [], [], { async: true }); }
|
||||
catch (e) { if (finish) finish(); return; }
|
||||
if (!stopped) pump();
|
||||
});
|
||||
};
|
||||
Module._wxNestedLoopExit = Module._wxNestedLoopExit || [];
|
||||
await new Promise(function (resolve) {
|
||||
finish = function () { stopped = true; /* splice from LIFO */ resolve(); };
|
||||
Module._wxNestedLoopExit.push(finish);
|
||||
pump();
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
Confirmed (native-EH): rafCount 348 (continuous), `dialog-01-loaded` **byte-identical** to baseline, all 5 dialog tests pass incl. modals.
|
||||
|
||||
## Relationship to the async dossier
|
||||
|
||||
The agent review (`docs/features/async/`) classifies this precisely: it is the dossier's **"de-parking (Option C — park main in an unresolved `EM_ASYNC_JS` sleep)"** (`02-asyncify-internals.md:265`, `07-decisions-and-outcome.md:60`), the "natural step one of Design B." The dossier **deferred/rejected** de-parking (`07/D4`) because, under `-fexceptions`, the throw provably "runs no destructors" (`10-resolution-menubar-uaf.md:63`, `08-dom-port-regression.md:305-317`) — so there was no reason to take it on. **That premise is exactly what `-fwasm-exceptions` inverts** (catch_all pads now *do* run destructors), and the dossier never considered native EH (grep-confirmed: zero mentions). So we are adopting the dossier's own deferred design, now made *necessary* by the toolchain change — consistent with its long-term direction (Design B), against its near-term decision (D4), for a reason D4 didn't know about.
|
||||
|
||||
## Teardown on exit — correct, not a bug
|
||||
|
||||
The old throw *abandons* the stack, so `wxEntryCleanupReal` (delete app + all TLWs) **never runs** — leaked on exit. The park lets `DoRun` resume on exit and return into that cleanup, which is correct (and frees the leak). No use-after-free: `ScheduleExit` cancels rAF first, and the park's resolver runs before `DoRun` resumes, so nothing calls `ProcessEvents` on freed state. (The dossier's general de-park warning is about the `simulate_infinite_loop=0`-and-*return* form where a still-registered rAF fires on the freed app; our suspend-then-cancel form avoids it.) Residual: a pre-existing wx window-close crash (`async/01:92-94`) could surface only if a real clean exit is triggered — rare in a browser; pre-existing, not introduced here.
|
||||
|
||||
## Remaining caveat (the real one to verify)
|
||||
|
||||
This makes the top-level an **always-live Asyncify-suspended context** for the app's lifetime. The known nesting wall (`async/11-asyncify-nesting-raytracer.md`: `emscripten_sleep` can't nest on an unwinding context) is the thing to watch — at KiCad scale, especially the 3D viewer. Reasoning suggests it's *not* worsened (the park is a dormant, separate saved stack, not in the active modal→sleep chain; ProcessEvents runs fresh from rAF), but that's analysis, not measurement. Since this is now one path for both EH models, the JS-EH build gets the park too, so the scale check covers both.
|
||||
|
||||
## Verification matrix (the old-vs-new test + screenshot proof)
|
||||
|
||||
Goal: prove (a) native EH renders correctly, and (b) JS-EH has **no regression**, by comparing the full wx e2e screenshots against the committed baselines (which were generated from the **old JS-EH** build). `scripts/compare-screenshots.sh` does byte-exact `cmp` of `tests/test-results/` vs `tests/baseline-screenshots/`.
|
||||
|
||||
| # | Config | EH model | `evtloop.cpp` | Purpose | Expected |
|
||||
|---|---|---|---|---|---|
|
||||
| 1 | **OLD** | JS-EH (`-fexceptions`) | original (throw) | reference / baseline-is-current sanity | matches committed baseline |
|
||||
| 2 | **NEW-native** | native (`-fwasm-exceptions`) | de-park (this plan) | migration target | matches baseline |
|
||||
| 3 | **NEW-js** | JS-EH (`-fexceptions`) | de-park (this plan) | no-regression | matches baseline |
|
||||
|
||||
- **Scope:** the full wx app suite (`menu, clipboard, filedialog, layout, aui, toolbar, grid, dialog, timer, tree`), all their `*-NN-*.png` shots.
|
||||
- **Pass bar:** configs 2 and 3 produce screenshots **byte-identical** (or trivially-different, e.g. caret-blink) to the baseline, same set config 1 produces.
|
||||
- **Browsers:** byte-compare is **chromium** (baselines are chromium). Firefox + WebKit are run for **pass/render** confirmation (their pixels won't byte-match a chromium baseline), per the all-three-engines policy.
|
||||
- **Build order (minimize clean rebuilds; only EH-model switches need `--clean`):** start from current (native-EH + interim option-A) → implement de-park → **(2)** native-EH de-park (incremental) → **(3)** JS-EH de-park (clean EH switch) → **(1)** JS-EH original (revert `evtloop.cpp`, incremental).
|
||||
|
||||
### Results (to fill in)
|
||||
|
||||
| # | Config | identical / different / fail | notes |
|
||||
|---|---|---|---|
|
||||
| 1 | OLD JS-EH | suite: **316 pass / 0 fail** / 1 skip | **Baseline is STALE** — OLD JS-EH itself differs 1–8% from the committed baseline on many main-app shots (03-after-load…aui…calendar…clipboard), so byte-compare *vs baseline* is unreliable; use config-vs-config. **0 failures here ⇒ config 2's 21 ARE regressions** from native-EH and/or de-park (not pre-existing). |
|
||||
| 2 | NEW native | suite: 295 pass / 21 fail / 1 skip. dialog standalone 3 identical + 2 caret. Apps render (main-app snapshot OK). Fails: main-app assertions (boot/wxwidgets/dialogs-tab/grid-tab, 10) + coroutine/threading/raytracer (11) | old-baseline (config 1) pending to classify the 21 as pre-existing vs regression |
|
||||
| 3 | NEW JS-EH | suite: **310 pass / 6 fail** / 1 skip — all 6 are **coroutine** (`coroutine`, `coroutine-nested`, `coroutine-pthread`) | de-park renders **identically** to config 1 (config1-vs-config3 byte-diffs = event-log timestamps + caret only; verified pixel-identical on 04-controls-tab) |
|
||||
|
||||
### Isolation (the verdict)
|
||||
|
||||
| failures | config 1 (no de-park) | config 3 (de-park, JS-EH) | config 2 (de-park, native-EH) | attribution |
|
||||
|---|---|---|---|---|
|
||||
| coroutine / coroutine-nested / coroutine-pthread (6) | pass | **FAIL** | FAIL | **the de-park** (top-level Asyncify park × coroutine fibers — the doc-11 nesting wall, now real) |
|
||||
| coroutine-raytrace (5) + main tabbed app: boot/wxwidgets/dialogs-tab/grid-tab (10) | pass | pass | **FAIL** | **native-EH** (migration coverage gaps; the "scale unproven" caveat) |
|
||||
|
||||
**Key conclusions:**
|
||||
1. The de-park is **visually clean** — no rendering change (config1≡config3 modulo timestamps), standalone apps all pass, dialog byte-identical.
|
||||
2. **The de-park regresses the 6 coroutine/threading tests under BOTH EH models.** Under JS-EH this is a **net loss** (the original `throw` form passes them) — so "one de-park for both" is not free: it costs JS-EH its coroutines. This is exactly the Asyncify-nesting hazard `async/11` + the async dossier flagged; the dossier's answer is **Design B** (fiber/arbiter runtime), of which de-park is "step one."
|
||||
3. native-EH independently breaks 15 more (raytracer + the big app) — broader migration work, separate from the loop change.
|
||||
4. The **committed screenshot baseline is stale** (old JS-EH itself is 1–8% off); and byte-compare is unreliable here anyway (event-log timestamps). A perceptual diff + a baseline refresh are needed for a real screenshot gate.
|
||||
|
||||
**Open decision:** (a) gate the de-park to native-EH only (JS-EH keeps `throw` + coroutines; not "one solution", needs `#ifdef`); (b) do Design B so de-park coexists with coroutines (bigger); (c) ship de-park for both now, coroutine-nesting as tracked follow-up.
|
||||
|
||||
## Status
|
||||
|
||||
Plan agreed. Implementing the `evtloop.cpp` de-park, then running the matrix above. The interim `option-A` edit (top-level via `wxWasmRunNestedLoop`'s `setTimeout` pump — loses rAF) is **superseded** by this and will be reverted in favor of the `set_main_loop(0)` + `wxWasmParkMainLoop` form.
|
||||
339
docs/features/wasm-exceptions/10-pthreads-native-eh.md
Normal file
339
docs/features/wasm-exceptions/10-pthreads-native-eh.md
Normal file
|
|
@ -0,0 +1,339 @@
|
|||
# Native wasm-EH × pthreads — findings and test coverage
|
||||
|
||||
> **Status:** native wasm-EH (`-fwasm-exceptions`) is the **default build**; the pthread test suite is
|
||||
> green in Firefox + Chrome. Authored 2026-06-24, updated 2026-06-25. The exception-handling-side
|
||||
> companion to the mechanism-deep [`../threading/README.md`](../threading/README.md) (the 3-layer
|
||||
> thread model, the deadlock mechanics, the three failure modes). **Scope:** what native wasm-EH does
|
||||
> to pthreads, and the test coverage that proves which patterns work.
|
||||
|
||||
## Why this exists
|
||||
|
||||
KiCad-WASM uses native WebAssembly exceptions instead of Emscripten JS exceptions for the bundle-size
|
||||
win (pcbnew ~64.5 → ~36 MB gz). **Native-EH is the default build** — `build-wx-wasm.sh`,
|
||||
`build-wasm-test.sh`, and `tests/apps/Makefile.wasm` compile `-fwasm-exceptions -sSUPPORT_LONGJMP=wasm
|
||||
-sWASM_LEGACY_EXCEPTIONS=1` (single-sourced from `scripts/common/env.sh`). It is the only build mode —
|
||||
the legacy `-fexceptions` path has been removed. CI builds native. The threading question this doc answers: which pthread patterns work under native-EH, what is
|
||||
the one exception-related risk it removes, and what is the (optional) upstreamable follow-up.
|
||||
|
||||
## TL;DR
|
||||
|
||||
- **Native-EH wx suite: green** in Firefox + Chrome (316 / 1 skipped / 0 failed, chromium — matching
|
||||
JS-EH). WebKit is blocked for *all* pthread apps by a separate, pre-existing COEP worker-load
|
||||
limitation (§2a), so the pthread specs run FF + Chrome.
|
||||
- **The one native-EH-relevant risk is mode-c** — a C++ exception thrown on a pthread worker. Under
|
||||
`-fexceptions` the throw drives Asyncify on the worker and crashes (`"func is not a function"`);
|
||||
native-EH lowers exceptions to native wasm instructions, so a throwing worker task is safe. Every
|
||||
threading pattern below is green under native-EH; the throwing ones are green **only** under native-EH.
|
||||
- **The real `BS::thread_pool` runs 16-core under native-EH**, including a task that throws on a worker
|
||||
(`threadpool-real`, §6) — the decisive proof that the `detach_task` single-thread shim can be dropped.
|
||||
- **On-demand (non-warm) Worker creation works without editing KiCad** (`pthread-ondemand`, §6): the
|
||||
`nanosleep` override (§2b) makes a main-thread `sleep_for` join Asyncify-yield, so the event loop
|
||||
services the new-Worker handshake. This is the threading-doc **mode-(a) deadlock** cure, in the wasm
|
||||
layer.
|
||||
- **A nested `emscripten_sleep` is legal** (§3): code dispatched by a wx modal pump's `ProcessEvents`
|
||||
runs at Asyncify `state == Normal`, so a worker-join that yields via `emscripten_sleep` inside an
|
||||
open modal suspends-and-resumes normally (`raytrace-modal`, §6). The threading-doc **mode-(b)** does
|
||||
**not** arise for this case, so no JS-land scheduler ("Design B") is needed for it.
|
||||
- **The KiCad-10 `std::async` library preload is safe under native-EH** (`async-preload`, §6/§7): the
|
||||
worker parses S-expr (a throw = mode-c) and proxies its async fetch to main; native-EH makes the
|
||||
parse safe and the lazy join keeps main free to service the proxy.
|
||||
- **The fork stays pristine.** The pool's `detach_task` shim is the original, unmodified KiCad code;
|
||||
the tests un-shim it via a build-generated header (§6/§D), so the KiCad submodule carries no
|
||||
wasm-specific change. The later, *optional* upstreamable step is the raw-threads→pool refactor (§4).
|
||||
|
||||
---
|
||||
|
||||
## 1. Suite status under native-EH
|
||||
|
||||
The native-EH wx app suite is **316 / 1 skipped / 0 failed** (chromium), matching JS-EH. Reaching it
|
||||
required two things: clearing a set of build-pipeline gaps that surfaced as native-EH test failures
|
||||
(committed this session, summarized below), and the §2 asyncify-imports fix for the raytracer cluster.
|
||||
|
||||
The build-pipeline gaps (all committed):
|
||||
|
||||
- **Post-link Asyncify find too narrow** — the loop matched only `standalone/*/*_test.wasm`, silently
|
||||
skipping `apps/minimal_test.wasm` and the coroutine-pthread repros / wxpt. Those linked but were
|
||||
never asyncify-instrumented → `asyncify_start_unwind not found`. Broadened to all freshly-linked app
|
||||
wasm.
|
||||
- **Repro apps mixed EH models** — the coroutine-pthread `*_repro` apps hardcoded JS-EH in their link
|
||||
recipes while their compile inherited native-EH → `undefined symbol: __cpp_exception`. Made them
|
||||
EH-aware so they follow the default (native) and only carry `-fexceptions` under `WX_LEGACY_EH`.
|
||||
- **`build-wasm-test.sh` swallowed make failures** — it continued to the post-link after a failed
|
||||
make, leaving apps half-instrumented (read as mass test failures). Now aborts loudly.
|
||||
|
||||
After those, the only real native-EH-specific signal was the raytracer threading cluster, fixed in §2.
|
||||
|
||||
---
|
||||
|
||||
## 2. The asyncify-imports fix (`emscripten_sleep`)
|
||||
|
||||
`coroutine-raytrace.spec.ts` aborted with `Aborted(invalid state: 1)`. The mechanism:
|
||||
|
||||
- `invalid state: 1` is `Asyncify.handleSleep` aborting because the state is **Unwinding** — a second
|
||||
suspend starting before the first rewinds. Logging every `handleSleep`, the state sequence at the
|
||||
abort is exactly **`0,1`**: two `emscripten_sleep`s back-to-back with **no rewind between**. So a
|
||||
function calls `emscripten_sleep`, the unwind arms (state→Unwinding), and the **same function calls
|
||||
`emscripten_sleep` again before returning**. A correctly Asyncify-instrumented function has a
|
||||
post-call "if Unwinding, save locals and return" check after every suspend point; this one doesn't →
|
||||
**Asyncify never instrumented it.**
|
||||
- `main()` is **not** re-entered (a `[MAINCALL]` probe fired exactly once; the abort stack only *shows*
|
||||
`main`'s frames because Asyncify's unwind/rewind runs inside a `setTimeout`-driven `doRewind` that
|
||||
keeps the JS stack live).
|
||||
- **Why un-instrumented:** binaryen's Asyncify instruments only functions that can reach a *listed*
|
||||
async import. The post-link list was curated for the wx apps, which yield via **fibers**
|
||||
(`startModal, js_*, invoke_*, __asyncjs__*, emscripten_fiber_swap`). It **omitted `emscripten_sleep`**,
|
||||
which the raytracer yields via. `env.emscripten_sleep` *is* a wasm import, so binaryen can match it —
|
||||
it just wasn't told to.
|
||||
|
||||
### The exact JS-EH ↔ native-EH difference
|
||||
|
||||
Under **JS-EH**, Asyncify runs **in-link** and emcc **auto-adds** `emscripten_sleep` (+
|
||||
`idb_*`/`wget`/`scan_registers`/`lazy_load`) to the imports. Under **native-EH** we run Asyncify
|
||||
**post-link by hand**, with an explicit list that dropped those auto-imports. That is the entire
|
||||
difference — not a fundamental native-EH × pthread incompatibility, and not a handleSleep-vs-arbiter
|
||||
question (the `currData` shim was never involved).
|
||||
|
||||
### The fix
|
||||
|
||||
Added `env.emscripten_sleep` (+ `scan_registers`, `lazy_load_code`, `wget`, `wget_data`, `idb_*`) to
|
||||
the post-link asyncify-imports — now the shared **`scripts/common/asyncify-imports.txt`**, consumed by
|
||||
the unified **`apply-asyncify.sh`** that both the wx-test and KiCad builds call (the two near-duplicate
|
||||
scripts were folded into one; the old `hoist-and-asyncify.sh` is gone). So the KiCad list gets
|
||||
`emscripten_sleep` too, pre-empting the identical latent bug when its threading is un-shimmed.
|
||||
|
||||
| Check | Result |
|
||||
|---|---|
|
||||
| Full wx suite, Chromium | **316 / 1 skipped / 0 failed** |
|
||||
| `coroutine-raytrace.spec.ts` — all 6 (B1/B2/B1-local/B3 + speedup + A neg-control) | **6/6 pass** |
|
||||
| multi-core speedup | **serial 1342 ms → parallel 142 ms = 9.45× on 16 cores** |
|
||||
| raytrace `#m=5` default (drains pool → on-demand creation) / `#m=1`, Chromium + Firefox | **SUCCESS, workersRan=16** |
|
||||
|
||||
The default `m=5` — which *drains* the pre-warmed pool and forces on-demand Worker creation —
|
||||
succeeds, so the fix also resolves the threading-doc **mode-(a) deadlock**: the `sleep_for` join now
|
||||
yields via an instrumented `emscripten_sleep` instead of busy-spinning and starving the worker
|
||||
handshake.
|
||||
|
||||
### 2a. The WebKit issue (separate, pre-existing)
|
||||
|
||||
In WebKit the asyncify side runs (threads spawn) but the **pthread worker `.js` load is refused on
|
||||
COEP** (`Refused to load worker because of Cross-Origin-Embedder-Policy`) even with COOP + COEP + CORP
|
||||
all served and `crossOriginIsolated:true`. It is a WebKit/playwright-headless COEP-worker strictness
|
||||
issue affecting **all** pthread apps, unrelated to EH. Tracked separately; the pthread specs run
|
||||
FF + Chrome only.
|
||||
|
||||
### 2b. The `nanosleep` override (the on-demand cure)
|
||||
|
||||
`wasm/shims/nanosleep_yield.c` is a **strong `nanosleep` definition** that shadows musl's archive
|
||||
member (`-Wl,--wrap=nanosleep` is not usable — it segfaults wasm-ld in
|
||||
`lld::wasm::ImportSection::addImport`). On the **main thread** it yields via an `EM_ASYNC_JS` await
|
||||
(= `emscripten_sleep` semantics, already in the post-link asyncify-imports); on a **worker** it stays a
|
||||
real blocking `emscripten_thread_sleep`. So an *unmodified* KiCad `sleep_for` join on the main thread
|
||||
pumps the event loop instead of busy-spinning, which lets on-demand Worker creation complete with no
|
||||
KiCad edit (§6 `pthread-ondemand`).
|
||||
|
||||
---
|
||||
|
||||
## 3. Patterns that work — pool-vs-raw, and the nested-sleep case
|
||||
|
||||
Two earlier wx apps, plus the §6 additions:
|
||||
|
||||
| App / test | Thread pattern | native-EH |
|
||||
|---|---|---|
|
||||
| `threadpool_test.cpp` (`threadpool.spec.ts`) | create `hwc` `std::thread`s into the **pre-warmed** pool, short body, **`join()`** each | **PASS** |
|
||||
| `raytrace_threads_test.cpp` (`coroutine-raytrace.spec.ts`) | raw detached/persistent `std::thread`, sleep/busy-wait join, default **drains** the pool → on-demand creation | **PASS** (after §2) |
|
||||
|
||||
Both raw and pool patterns work under native-EH; raw threads are **not** fundamentally broken. The
|
||||
`threadpool` create-and-`join()` never calls `emscripten_sleep`, so it never tripped the missing
|
||||
import; the raytracer yields via `emscripten_sleep`, so it did — which §2 closed.
|
||||
|
||||
**The nested-sleep case (mode-b is not a live blocker).** A worker-join that yields via
|
||||
`emscripten_sleep` *inside an open `ShowModal` dialog* is legal. The modal pump runs `ProcessEvents`
|
||||
via `ccall(async:true)`, so work it dispatches runs in a **fresh managed Asyncify entry at
|
||||
`state == Normal`** — not nested inside an already-Unwinding frame. So the inner `emscripten_sleep`
|
||||
suspends-and-resumes normally. `raytrace-modal` (§6) probes and logs `Asyncify.state == 0` to confirm
|
||||
this; the threading-doc mode-(b) "Asyncify can't nest" only bites a *genuine* second unwind, which the
|
||||
modal pump does not produce. **No JS-land scheduler ("Design B") is required for the "render inside a
|
||||
modal" case.**
|
||||
|
||||
---
|
||||
|
||||
## 4. Optional follow-up (upstreamable): KiCad raw-threads → the pool
|
||||
|
||||
This is a *later, optional* cleanup — not required, since the wasm layer (native-EH + the nanosleep
|
||||
override) already makes the threading patterns work on pristine KiCad. Per
|
||||
[`../threading` §8](../threading/README.md), upstream KiCad has migrated only **1 of 7** raytracer
|
||||
parallel sections to `GetKiCadThreadPool()` (`renderTracing`, and that one accidentally); the other six
|
||||
are untouched **2018 OpenMP-translation** raw-thread code:
|
||||
|
||||
| Site | Pass | Today |
|
||||
|---|---|---|
|
||||
| `render_3d_raytrace_base.cpp:764` `shadeWorker` | post-process shading | raw `std::thread` + busy-wait, `#ifdef`'d serial in WASM |
|
||||
| `render_3d_raytrace_base.cpp:835` `blurWorker` | blur/finish | same |
|
||||
| `render_3d_raytrace_base.cpp:1456` `previewWorker` | preview | same |
|
||||
| `image.cpp:525` `filterWorker` | `EfxFilter` AA/blur | same |
|
||||
| `create_layer_items.cpp:848` `zoneWorker` | zone-fill geometry | same |
|
||||
| `create_layer_items.cpp:1311` `simplifyWorker` | polygon simplify | same |
|
||||
|
||||
**The refactor = migrate these six to `submit_task()` + `multi_future::wait()`** (the `renderTracing`
|
||||
shape, refined by upstream `bccf36538` to wait on *own* tasks only), and delete the
|
||||
`#ifdef __EMSCRIPTEN__` serial fallbacks. Why it is the right *eventual* move:
|
||||
|
||||
- **Upstreamable, not a wasm hack** — precedent in the same file, a filed upstream issue
|
||||
([GitLab #20911](https://gitlab.com/kicad/code/kicad/-/issues/20911), "ray tracing high system
|
||||
load"), and it removes dead OpenMP-era code. If accepted upstream, our fork carries **zero**
|
||||
divergence here.
|
||||
- **Less divergence, not more** — it lets us drop the raytracer `#ifdef`s; combined with native-EH
|
||||
letting us drop the `detach_task` shim, net fork divergence goes *down* while threads come *on*.
|
||||
|
||||
Its prerequisite — that the real `BS::thread_pool` (persistent workers + `submit_task` +
|
||||
`multi_future::wait()`) survives native-EH — **is proven** by §6 `threadpool-real` (16-core, including
|
||||
a throwing worker task). So the refactor is de-risked; it is scheduled **after** the EH port's suite is
|
||||
otherwise green, and remains optional because the wasm-layer fixes already deliver multi-core.
|
||||
|
||||
---
|
||||
|
||||
## 5. The 3D viewer
|
||||
|
||||
The 3D viewer is **live and single-threaded**: the raytracer's six raw-thread passes are `#ifdef`'d to
|
||||
serial fallbacks in WASM, which is what ships. A separate multi-threaded spike exists (the
|
||||
`WASM_RAYTRACE_POOL` work, ~6–7×) but is not the active path.
|
||||
|
||||
Two zero-KiCad-edit routes turn the live viewer multi-threaded:
|
||||
|
||||
- **The nanosleep override (§2b)** makes the existing `sleep_for` joins yield, so the raw-thread passes
|
||||
run multi-core without on-demand-creation deadlock and without main-thread jank — no KiCad change.
|
||||
- **The §4 pool refactor** is the *upstream-clean* alternative: pool-based, drops the `#ifdef`s, and
|
||||
carries zero fork divergence if accepted upstream.
|
||||
|
||||
---
|
||||
|
||||
## 6. pthread test coverage
|
||||
|
||||
All four apps below compile the **real KiCad** thread-pool source and run on **pristine** KiCad/wx-core.
|
||||
The specs are named `coroutine-*` so `playwright-coroutine.config.ts` runs them in Firefox + Chrome
|
||||
(WebKit excluded — §2a).
|
||||
|
||||
| Spec | App | What it proves | native-EH |
|
||||
|---|---|---|---|
|
||||
| `coroutine-threadpool-real.spec.ts` | `threadpool-real` | the **real `GetKiCadThreadPool()`** in every mode — submit / loop / blocks / detach / fanout / lifecycle, and a task that **throws** on a worker — 16-core, throw caught | **PASS** (throw mode green *only* under native-EH = mode-c) |
|
||||
| `coroutine-pthread-ondemand.spec.ts` | `pthread-ondemand` | real pool drains the pre-warmed Workers, then raw fly-threads force **on-demand** creation; the nanosleep override yields the join → on-demand Workers boot → multi-core (control: a non-yielding busy-wait deadlocks) | **PASS** |
|
||||
| `coroutine-raytrace-modal.spec.ts` | `raytrace-modal` | a worker-join run **inside an open `ShowModal`** — both a busy-wait join and an `emscripten_sleep` yield-join complete multi-core; the app probes `Asyncify.state == 0` to show the modal pump dispatches at Normal | **PASS** (mode-b does not arise) |
|
||||
| `coroutine-async-preload.spec.ts` | `async-preload` | the KiCad-10 `std::async` library-preload shape: a worker parses S-expr (throws = mode-c) and proxies its fetch to main via `emscripten_proxy_sync_with_ctx`; modes simple / throw / shutdown / modal-during-preload | **PASS** (mode-c safe; 36 proxy round-trips through a modal, no crash) |
|
||||
|
||||
These also cover the older `coroutine-pthread.spec.ts` (fiber + pthread across activation paths) and
|
||||
`threadpool.spec.ts` (raw create+join), both green. Together they exercise: the real pool API, raw
|
||||
create+join, raw detached/persistent + sleep/busy-wait, on-demand creation, a worker-side throw, a
|
||||
nested yield inside a modal, and a proxied async fetch off a worker — the full set of shapes the KiCad
|
||||
threading uses.
|
||||
|
||||
### D. How the tests un-shim the pool without editing KiCad
|
||||
|
||||
KiCad's `bs_thread_pool.hpp` keeps its original `#ifdef __EMSCRIPTEN__` `detach_task` shim (which runs
|
||||
pool tasks inline → single-threaded). To exercise the *real* pool, the test build **generates** an
|
||||
un-shimmed copy: `tests/apps/Makefile.wasm`'s `POOL_UNSHIMMED` rule `sed`s `#ifdef __EMSCRIPTEN__` →
|
||||
`#if 0` into `standalone/_pool_unshimmed/bs_thread_pool.hpp` (gitignored) and `-I`'s it ahead of the
|
||||
KiCad header. So the KiCad submodule stays pristine; only the test compile sees the un-shimmed pool.
|
||||
|
||||
---
|
||||
|
||||
## 7. Library preload: `std::async` + the PCBJAM proxy under native-EH (the KiCad-10 bump)
|
||||
|
||||
> The path that turns the native-EH migration from a size win into a **prerequisite for tracking
|
||||
> upstream**. Verified by `async-preload` (§6): works under native-EH.
|
||||
|
||||
**What changed upstream.** KiCad 10 (`d8ae50a667`, 2026-06-08, fixes GitLab #23872) (a) added an
|
||||
*eager* library preload on board open — `if( Kiface().IsSingle() ) Kiface().PreloadLibraries()` in
|
||||
`pcbnew/files.cpp` (`OpenProjectFiles`), and `IsSingle()` is exactly our standalone-webapp case; and
|
||||
(b) changed `IFACE::PreloadLibraries`'s dispatch from `tp.submit_task( preload )` (our base,
|
||||
`pcbnew/pcbnew.cpp:666`) to `std::async( std::launch::async, preload )` (KiCad-10 `pcbnew.cpp` ~1121).
|
||||
**`std::async` spawns a real pthread worker that the `detach_task` pool shim does not cover** — the
|
||||
shim only neutralizes the *pool*.
|
||||
|
||||
**The plugin gotcha (don't be fooled by the upstream loader).** Upstream library reads are synchronous
|
||||
(`KICAD_SEXPR` plugin → `fopen`/`FILE_LINE_READER`). **Our fork is not on that path.** The webapp
|
||||
writes the lib-table rows as `(type "PCBJAM")` / `(type "PCBJAM_FP")`
|
||||
(`web/standalone/src/wasm/libs/source.ts:124,143`), so the runtime plugin is our **custom async bridge**
|
||||
(`kicad/eeschema/sch_io/pcbjam_lib/sch_io_pcbjam_lib.cpp`, `kicad/pcbnew/pcb_io/pcbjam_fp/pcb_io_pcbjam_fp.cpp`).
|
||||
A surface read of the upstream loader will wrongly conclude "pure sync, safe" — **verify by the
|
||||
lib-table row `type`, not the generic plugin.** The PCBJAM dispatch is dual-path
|
||||
(`sch_io_pcbjam_lib.cpp:158`):
|
||||
|
||||
```cpp
|
||||
if( emscripten_is_main_runtime_thread() )
|
||||
return pcbjam_libs_request_js(...); // main: EM_ASYNC_JS → Asyncify suspend (works on main)
|
||||
std::lock_guard lk( g_pcbjamProxyMutex ); // worker: serialize, then
|
||||
emscripten_proxy_sync_with_ctx( queue, main, … ); // proxy the fetch to MAIN + futex-block the worker
|
||||
```
|
||||
|
||||
**The two-level architecture (and how the pool shim warps it).**
|
||||
- **Outer:** `std::async(preload)` = one real background worker running a watchdog loop
|
||||
(`sleep_for(150ms)` + poll `AsyncLoadProgress()`). Bypasses the shim.
|
||||
- **Inner:** `adapter->AsyncLoad()` (`FOOTPRINT_LIBRARY_ADAPTER`) `submit_task`s N enumerate jobs to
|
||||
the pool → caught by the shim → run inline → so in our fork they execute *serially on the outer
|
||||
worker*. (Inner parallelism returns once the shim is dropped — also native-EH-gated.)
|
||||
|
||||
**What runs where, on the `std::async` worker:**
|
||||
|
||||
| Step | Suspends Asyncify on the worker? |
|
||||
|---|---|
|
||||
| `sleep_for(150ms)` watchdog | **No** — real worker sleep (`nanosleep`/Atomics.wait), not `emscripten_sleep`. |
|
||||
| PCBJAM fetch of library bytes | **No** — proxied to main + futex-block; the `EM_ASYNC_JS` runs on *main*. |
|
||||
| S-expr **parse** of the bytes (throws `IO_ERROR`) | **Yes under `-fexceptions`** → mode-c crash. **No under native-EH.** |
|
||||
| modals / clipboard / fonts | Not reachable from non-UI parsing. |
|
||||
|
||||
**The join is lazy — which defuses the deadlock.** There is **no eager `.get()`**: `CancelPreload(true)`
|
||||
calls `m_libraryPreloadReturn.wait()` but has **no callers**; `ProjectChanged()` only sets the abort
|
||||
flag; the `std::async` future's **blocking destructor** fires only on **IFACE teardown** (shutdown,
|
||||
main thread); and re-entry is guarded by `m_libraryPreloadInProgress` (so the future is never
|
||||
*reassigned* mid-flight). So in normal operation **main never blocks on the preload future** → it stays
|
||||
in its event loop → it services the PCBJAM proxy queue → the worker's fetches complete. No
|
||||
normal-operation deadlock.
|
||||
|
||||
**Verified.** `async-preload` (§6) runs this shape under native-EH: the worker parse throws and is
|
||||
caught (no mode-c crash), the proxy round-trips, and a modal opened during preload survives 36 proxy
|
||||
round-trips with no crash (the `g_pcbjamProxyMutex` / "table index out of bounds" reentrancy hazard
|
||||
does not fire). So the **KiCad-10 bump can keep `std::async` as-is under native-EH** — it does **not**
|
||||
need a fork patch reverting to `tp.submit_task` (which would make preload block board-open on the main
|
||||
thread). Residual: a real-shutdown ordering check (the blocking destructor while a load is in flight)
|
||||
is covered by the `async-preload` shutdown mode but not yet under a live IFACE teardown.
|
||||
|
||||
**Contrast with the raytracer (§4):** the raytracer's raw threads are legacy OpenMP-era and
|
||||
upstreamable to the pool. This `std::async` is a **deliberate** upstream choice (a dedicated preload
|
||||
thread, off the compute pool), so "upstream it to the pool" is **not** the play — native-EH is.
|
||||
|
||||
---
|
||||
|
||||
## 8. Next steps (ordered)
|
||||
|
||||
1. **DONE — `coroutine-raytrace` root-caused and fixed (§2):** the post-link asyncify-imports list
|
||||
omitted `emscripten_sleep`. Suite 316/0; raytracer multi-core (9.45×).
|
||||
2. **DONE — pthread coverage closed (§6):** the real-pool, on-demand, modal-nested, and `std::async`
|
||||
library-preload shapes are all green under native-EH on pristine KiCad/wx-core.
|
||||
3. **Drop the `detach_task` shim for real** — `threadpool-real` proves the pool survives native-EH, so
|
||||
the next concrete step is enabling the un-shimmed pool in a KiCad build (DRC / zone-fill /
|
||||
connectivity on real Workers) and validating the docker build (the shared asyncify-imports change is
|
||||
untested there).
|
||||
4. **Resolve the §2a WebKit COEP worker-load limitation** for pthread apps (currently the reason the
|
||||
pthread specs skip WebKit).
|
||||
5. **Optional, later — the §4 refactor:** migrate the six raw-thread raytracer sections to the pool,
|
||||
delete the `#ifdef`s, upstream it.
|
||||
6. **Track-only:** `PROXY_TO_PTHREAD` (DOM-bound GUI can't leave the main thread) and JSPI
|
||||
(incompatible with our main-loop architecture) — see [`../threading` §6–7](../threading/README.md).
|
||||
|
||||
## Cross-references
|
||||
|
||||
- [`../threading/README.md`](../threading/README.md) — the 3-layer model, deadlock mechanics, three
|
||||
failure modes, the full raw-thread inventory, and the upstream pool-migration analysis.
|
||||
- [`../async/11-asyncify-nesting-raytracer.md`](../async/11-asyncify-nesting-raytracer.md),
|
||||
[`../async/12`](../async/12-design-b-asyncify-implementation-plan.md),
|
||||
[`../async/13`](../async/13-design-b-engineering-spec.md) — Asyncify nesting + the Design B scheduler
|
||||
(not required for the modal-pump case, §3).
|
||||
- Apps + specs: `tests/apps/standalone/{threadpool-real,pthread-ondemand,raytrace-modal,async-preload,coroutine-pthread,threadpool,raytrace-threads}/`,
|
||||
`tests/e2e/coroutine-{threadpool-real,pthread-ondemand,raytrace-modal,async-preload,pthread,raytrace}.spec.ts`,
|
||||
`tests/e2e/threadpool.spec.ts`; the pool un-shim in `tests/apps/Makefile.wasm` (`POOL_UNSHIMMED`),
|
||||
the on-demand cure in `wasm/shims/nanosleep_yield.c`.
|
||||
- **Library preload (§7):** `kicad/pcbnew/pcbnew.cpp:593` (`PreloadLibraries`),
|
||||
`kicad/eeschema/sch_io/pcbjam_lib/sch_io_pcbjam_lib.cpp` +
|
||||
`kicad/pcbnew/pcb_io/pcbjam_fp/pcb_io_pcbjam_fp.cpp` (the async PCBJAM IO plugins),
|
||||
`web/standalone/src/wasm/libs/source.ts` (lib-table rows typed `PCBJAM`/`PCBJAM_FP`); upstream
|
||||
KiCad-10 `std::async` change `d8ae50a667` (GitLab #23872).
|
||||
438
docs/features/wasm-exceptions/11-collab-apply-native-eh.md
Normal file
438
docs/features/wasm-exceptions/11-collab-apply-native-eh.md
Normal file
|
|
@ -0,0 +1,438 @@
|
|||
# Collaborative editing under native wasm-EH — the virtual-call mis-dispatch: root cause & fix
|
||||
|
||||
> **Finalization note:** the `futex_yield.c` shim and the `vcall_*` / `pool-callafter` investigation
|
||||
> repros referenced below were **removed** during feature finalization — native-EH needs none of them.
|
||||
> This doc is retained as the root-cause record.
|
||||
|
||||
> **Status: RESOLVED 2026-06-28.** Fixed with a one-line build flag.
|
||||
> **One-line:** native-EH pcbnew's collab **apply** hung at virtual method calls because the **embind
|
||||
> translation unit was compiled without `-DDEBUG` while the core TU had it**. A `#if defined(DEBUG)`
|
||||
> virtual (`EDA_ITEM::Show`) takes a vtable slot, so the two TUs' vtable layouts differed by one slot;
|
||||
> every embind virtual call past that slot read the wrong slot and `call_indirect`-trapped on a
|
||||
> signature mismatch — swallowed by the apply coroutine's `catch_all` → silent loop = "hang." **Fix:**
|
||||
> define `DEBUG` for the embind TU in Debug builds (`scripts/kicad/build-kicad-target.sh`). No
|
||||
> devirtualization; the A/B decision in Part 5 is moot. See [Resolution](#resolution--root-cause--fix).
|
||||
>
|
||||
> *Parts 1–5 below are the investigation as it unfolded; it concluded the `vii` correlation was a
|
||||
> "confound" and weighed an A/B decision. That was right that the dispatch **mechanism** wasn't broken
|
||||
> — but it stopped one step short of the dispatch **input**: the vtable **slot offset** the embind
|
||||
> computed was wrong. A runtime vtable probe + a named-binary offset check (the user's "check the
|
||||
> offsets, find the wrong one") closed it. Kept as the reasoning trail; the Resolution is the answer.*
|
||||
>
|
||||
> Companion to [`01-background-two-eh-models.md`](01-background-two-eh-models.md) (EH models),
|
||||
> [`10-pthreads-native-eh.md`](10-pthreads-native-eh.md) (pthreads), and the `currData` dossier in
|
||||
> [`docs/features/async/`](../async/).
|
||||
|
||||
---
|
||||
|
||||
## Resolution — root cause & fix
|
||||
|
||||
**Root cause (verified 2026-06-28).** The embind TU (`wasm/bindings/pcbnew_embind.cpp`, compiled
|
||||
*outside* CMake in `build-kicad-target.sh` step 7) and the core/vtable-emitting TU disagreed on the
|
||||
`PCB_TRACK` vtable layout by exactly one slot:
|
||||
|
||||
- `EDA_ITEM::Show(int, std::ostream&)` is declared `#if defined(DEBUG)` (`kicad/include/eda_item.h:471`).
|
||||
- The **core** is built Debug → CMake `add_compile_definitions($<$<CONFIG:Debug>:DEBUG>)`
|
||||
(`kicad/CMakeLists.txt:351`) defines `DEBUG` → `Show` occupies vtable **slot 35** → `PCB_TRACK::SetWidth`
|
||||
lands at **byte offset 320**.
|
||||
- The **embind TU** was compiled **without `-DDEBUG`** → no `Show` slot → it computed `SetWidth` at
|
||||
**offset 316**, which in the emitted vtable is `BOARD_CONNECTED_ITEM::GetEffectiveNetClass()` (wasm
|
||||
type `ii`). Dispatching it as `vii` = a **`call_indirect` signature-mismatch trap**, swallowed by the
|
||||
apply COROUTINE's `catch_all` → silent retry loop = the observed "hang."
|
||||
- Asyncify state at the park was **Normal** (confirmed via the real `asyncify_get_state()` export) — a
|
||||
trap, never a suspend. Every embind virtual call past slot 35 (`SetWidth`/`GetPosition`/the rebaseline
|
||||
snapshot getters) mis-dispatched; `Type`/`GetClass` (slots < 35) and core-TU calls
|
||||
(`commit.Modify`'s `Clone`) worked — which is what made it *look* signature-specific (`vii` fails,
|
||||
`ii` works).
|
||||
|
||||
**Fix.** `scripts/kicad/build-kicad-target.sh`: `EMBIND_CONFIG_DEFINES="-DDEBUG"` in the Debug branch
|
||||
(empty in Release — Release defines no `DEBUG` in either TU, so the layouts already match), added to the
|
||||
embind `em++` compile. Both TUs now agree on the layout; **all** embind virtual calls dispatch
|
||||
correctly. The per-site devirtualizations tried during the hunt were reverted (unnecessary). *Hygiene
|
||||
follow-up (recommended, not yet applied):* also give the embind TU `-DKICAD_USE_PLATFORM_WASM=1` and the
|
||||
`-include char_traits_uint16_workaround.h` force-include, so its preprocessor/ABI environment matches
|
||||
the core's exactly and this class of skew can't recur.
|
||||
|
||||
**How it was found.** A runtime probe (a fresh `PCB_TRACK` constructed at the park) showed
|
||||
`tr.vtbl == fresh.vtbl` — the vtable *pointer* was correct — yet the fresh object hung identically,
|
||||
ruling out a dead/wrong instance and pointing at the *slot offset*. `wasm-dis` of the named debug
|
||||
binary then showed offset 316 holds `GetEffectiveNetClass`, not `SetWidth`. Exactly the user's
|
||||
instruction: *"check the offsets of all instances, find the wrong one."*
|
||||
|
||||
**Independently-real fix kept** (not caused by the skew): the **COROUTINE** in `kicadCollabApply`
|
||||
(the `commit.Modify`→`Clone` trap needs the fiber) and `-Xclang -fno-pch-timestamp`.
|
||||
|
||||
**`wasm/shims/futex_yield.c` — NOT needed; kept-but-not-compiled.** It was added during the hunt under
|
||||
the (mistaken) theory that the apply hung on the thread-pool futex; the real cause was the vtable skew.
|
||||
A **no-futex build passes collab 8/8** (Firefox + Chromium) — the connectivity recompute is bounded by
|
||||
the pre-warmed pthread pool (`PTHREAD_POOL_SIZE = hardwareConcurrency`), so it never needs an on-demand
|
||||
Worker, so there's no main-thread futex deadlock to fix here. The shim file + the `pool-callafter` repro
|
||||
are kept as a *documented, validated* fix for the on-demand-Worker futex deadlock **if it ever surfaces**
|
||||
(heavy board / cold pool hanging at `commit.Push`'s `RecalculateRatsnest`); the re-enable steps live in
|
||||
`scripts/kicad/build-kicad-target.sh` (the "AVAILABLE BUT NOT COMPILED" block) and the shim's header.
|
||||
|
||||
---
|
||||
|
||||
## 0. How to read this document
|
||||
|
||||
This is written to be understood without prior knowledge of WebAssembly internals, C++ dynamic
|
||||
dispatch, Asyncify, fibers, or futexes. **Part 1** explains every concept from scratch. **Part 2**
|
||||
walks the actual call chains. **Part 3** lists the four bugs we found and the fixes that work.
|
||||
**Part 4** is the investigation that proved the headline bug is a confound. **Part 5** is the A/B
|
||||
decision and the concrete effort estimate for A. Skim the TL;DR, then dive into whatever you want.
|
||||
|
||||
### TL;DR
|
||||
|
||||
- KiCad's collaborative editing broadcasts each local edit to peers; a peer **applies** the change
|
||||
by running the same `BOARD_COMMIT` machinery a native edit uses.
|
||||
- Under **native wasm-EH** (our target), that apply **hangs** at a C++ **virtual** method call
|
||||
(`SetWidth`, `GetPosition`, …). The same calls work fine under the old **legacy JS-EH** build.
|
||||
- We found and fixed three real sub-bugs (a dispatch *trap*, a thread-pool *futex deadlock*, a fiber
|
||||
*finalization* hang). The fourth — the virtual-call hang — looked signature-specific (calls whose
|
||||
wasm signature is `vii` hang; `ii`/`viii` don't), but **five controlled repros prove `vii` dispatch
|
||||
is not actually broken**. It only hangs inside the real, huge module.
|
||||
- **Devirtualizing** the call (telling the compiler the exact function so it emits a direct call
|
||||
instead of an indirect one) makes the hang vanish at that site — but that's treating a symptom, and
|
||||
it has to be repeated at ~10–15 call sites and re-done whenever new code adds a by-value getter.
|
||||
|
||||
---
|
||||
|
||||
# Part 1 — The concepts
|
||||
|
||||
## 1.1 Two ways C++ exceptions become WebAssembly
|
||||
|
||||
WebAssembly can't just "throw" like native code. Emscripten offers two lowerings:
|
||||
|
||||
- **Legacy JS exceptions (`-fexceptions`)** — every call that might throw is wrapped in a JavaScript
|
||||
helper called `invoke_<sig>` that does a JS `try/catch` around a `dynCall_<sig>` into wasm. The
|
||||
control flow for exceptions detours *through JavaScript*. Big and slow, but battle-tested.
|
||||
- **Native wasm exceptions (`-fwasm-exceptions`)** — uses the WebAssembly exception-handling
|
||||
instructions (`try`/`catch`/`throw`) directly in wasm. No `invoke_*`, no JS detour. Smaller and
|
||||
faster — this is what we're migrating to. (See [`01-background-two-eh-models.md`](01-background-two-eh-models.md).)
|
||||
|
||||
This distinction matters later: under native-EH there are **no `invoke_*`/`dynCall_*` wrappers around
|
||||
ordinary calls**, so the JS-side "self-heal" tricks that exist for legacy-EH don't apply.
|
||||
|
||||
## 1.2 Virtual method calls, vtables, and `call_indirect`
|
||||
|
||||
When you write `item->GetPosition()` and `GetPosition()` is declared `virtual`, the compiler does
|
||||
**not** know which function to run. A `PCB_TRACK` returns its start point; a `PCB_VIA` returns its
|
||||
centre; a `FOOTPRINT` returns its origin. The decision is made **at runtime** based on the object's
|
||||
real type. This is **dynamic dispatch**, and it works via a **vtable**:
|
||||
|
||||
```
|
||||
A PCB_TRACK object in memory The PCB_TRACK vtable (one per class)
|
||||
┌───────────────────────────┐ ┌────────────────────────────────────┐
|
||||
│ vptr ───────────────────────────► │ slot 0: &PCB_TRACK::Type │
|
||||
│ m_Start = (10, 20) │ │ slot 1: &PCB_TRACK::GetPosition │
|
||||
│ m_End = (50, 20) │ │ slot 2: &PCB_TRACK::SetWidth │
|
||||
│ m_width = 200000 │ │ ... │
|
||||
└───────────────────────────┘ └────────────────────────────────────┘
|
||||
```
|
||||
|
||||
Every object of a polymorphic class starts with a hidden pointer (`vptr`) to its class's vtable. A
|
||||
virtual call compiles to: *load the vptr → load the function pointer in the right slot → call it.*
|
||||
|
||||
In WebAssembly there are no raw function pointers; instead there is a single **function table** (an
|
||||
array of functions) and a `call_indirect N` instruction that means "call the function at table index
|
||||
N." So the C++ virtual call becomes, in wasm:
|
||||
|
||||
```wat
|
||||
local.get $item ;; the object pointer
|
||||
i32.load offset=0 ;; load vptr (the vtable address)
|
||||
i32.load offset=8 ;; load the function index from the SetWidth slot
|
||||
;; ... push the call args ...
|
||||
call_indirect (type $vii) ;; call the function at that table index, expecting signature "vii"
|
||||
```
|
||||
|
||||
`(type $vii)` is the *expected signature* baked into the instruction. **Signature notation:** the
|
||||
first letter is the return type, the rest are arguments. `i`=i32, `v`=void.
|
||||
- `ii` = `(i32) -> i32` — e.g. `KICAD_T Type()` (takes the hidden `this`, returns an int-like value).
|
||||
- `vii` = `(i32, i32) -> void` — e.g. `void SetWidth(int)` (`this`, the int; returns nothing).
|
||||
- `viii` = `(i32, i32, i32) -> void` — e.g. `view->Update(item, flags)`.
|
||||
|
||||
**A subtlety that matters here — struct returns (the "sret" ABI).** A method that *returns a small
|
||||
struct by value*, like `VECTOR2I GetPosition()`, can't return two ints in one wasm value. The
|
||||
compiler rewrites it so the caller passes a hidden pointer to a return slot, and the function writes
|
||||
through it and returns nothing: `void GetPosition(this, VECTOR2I* out)`. That's also signature `vii`.
|
||||
So **all of `GetPosition`, `GetClass`, `GetText`, `GetTextSize` (struct/string returns) AND
|
||||
`SetWidth`, `SetPosition` (void setters) are `vii`** — which is why the bug *looked* signature-specific.
|
||||
|
||||
## 1.3 Devirtualization (the fix technique, and its trade-off)
|
||||
|
||||
If you class-qualify the call — `static_cast<PCB_TRACK*>(item)->PCB_TRACK::GetPosition()` — you tell
|
||||
the compiler *exactly* which function to run. It no longer needs the vtable; it emits a plain `call`
|
||||
(a direct call to a known function), not a `call_indirect`. We already do this elsewhere: see
|
||||
`itemLayer()` / `itemPosition()` / `itemClass()` in `wasm/bindings/pcbnew_embind.cpp`.
|
||||
|
||||
**Why it helps the bug:** the hang fires *at the instrumented `call_indirect`*. A direct `call`
|
||||
isn't wrapped the same way, so the symptom doesn't appear there.
|
||||
|
||||
**The trade-off / why it's not free:** class-qualifying picks *one* class's version. For a generic
|
||||
getter like `GetPosition` whose answer depends on the type, you must dispatch on the type yourself
|
||||
(`switch (item->Type()) { case PCB_TRACE_T: ...; case PCB_VIA_T: ...; }`) and class-qualify each arm
|
||||
— otherwise you call the wrong override and get wrong geometry. That's why devirtualization here is a
|
||||
**helper per generic getter** plus a hand edit per setter, and why it's "finite but spread."
|
||||
|
||||
## 1.4 Asyncify — making synchronous C++ pause and resume
|
||||
|
||||
KiCad's C++ is written **synchronously**: it calls `sleep`, it *blocks* waiting for worker threads,
|
||||
it pops up a modal dialog and waits for the user. In a browser you **cannot block the main thread** —
|
||||
if you do, the page freezes (no rendering, no input, no timers). The reconciliation is **Asyncify**, a
|
||||
Binaryen transform that rewrites the wasm so a deep synchronous call stack can be **suspended**
|
||||
(unwound back to the JS event loop) and later **rewound** (rebuilt exactly where it left off).
|
||||
|
||||
Mechanically, Asyncify instruments functions so that:
|
||||
- on **unwind**, each function saves its locals + a "where was I" call-index into a memory buffer and
|
||||
returns up the stack until it reaches the event loop; the buffer pointer is `Asyncify.currData`.
|
||||
- on **rewind**, each function restores its locals and jumps back to the saved call-index, rebuilding
|
||||
the stack until execution resumes at the suspend point.
|
||||
|
||||
There is **one** `currData` slot at a time. If a second suspend starts while the first's buffer is
|
||||
still occupied, things collide — that's the "nested currData contention" family
|
||||
([`docs/features/async/`](../async/)). The three states are **Normal (0)**, **Unwinding (1)**,
|
||||
**Rewinding (2)** (`Asyncify.state` / the wasm export `asyncify_get_state()`).
|
||||
|
||||
Why we can't avoid Asyncify: native edits already rely on it (tool interactions suspend mid-drag, the
|
||||
event loop yields each frame). The collab apply runs the same `BOARD_COMMIT` code, so it inherits the
|
||||
same instrumentation.
|
||||
|
||||
## 1.5 Coroutines and libcontext fibers (KiCad's `COROUTINE`)
|
||||
|
||||
A **fiber** is a *second call stack* you can switch to and from cooperatively (no OS thread). KiCad
|
||||
ships its own `COROUTINE` (built on `libcontext`'s `jump_fcontext`) and runs **tool interactions** on
|
||||
a fiber so a tool can "yield" in the middle of an operation and be resumed later. The fiber has its
|
||||
own stack memory; switching is just swapping the stack pointer.
|
||||
|
||||
Why it's in the collab apply: KiCad-WASM has a long-standing rule that the heavy edit machinery
|
||||
(`BOARD_COMMIT::Modify` → `item->Clone()`, the GAL `view->Add`) **only dispatches correctly when run
|
||||
on the tool-coroutine fiber stack** — running it from a bare `CallAfter`/`ccall` trapped with
|
||||
"indirect call signature mismatch." So `kicadCollabApply` wraps `doApply` in a `COROUTINE`. (This is
|
||||
fix #1 below; it's real and necessary.)
|
||||
|
||||
## 1.6 Futexes — and why they deadlock on the browser main thread
|
||||
|
||||
A **futex** ("fast userspace mutex") is the low-level OS primitive a thread uses to **wait until
|
||||
another thread signals it**. `std::mutex`, `std::condition_variable`, and `std::future::get()` are all
|
||||
built on it. The pattern: thread A wants thread B's result, so A does `futex_wait(addr, val)` — "sleep
|
||||
until the value at `addr` changes" — and B does `futex_wake(addr)` when it's done.
|
||||
|
||||
KiCad's **connectivity recompute** (rebuilding the ratsnest/net graph after an edit, in
|
||||
`commit.Push`) is parallelised across a thread pool. The main thread submits work and then
|
||||
`futex_wait`s for the workers' results (`std::future::wait_for` → `pthread_cond_wait` →
|
||||
`emscripten_futex_wait`).
|
||||
|
||||
**The browser problem:** on the **main browser thread**, `Atomics.wait` (the real blocking wait) is
|
||||
*forbidden* — blocking it would freeze the page. Emscripten's fallback is to **busy-spin** in
|
||||
`futex_wait_main_browser_thread()`, calling `_emscripten_yield()` — but that only services the
|
||||
internal proxy queue, it **never returns to the JS event loop**. So if the worker the main thread is
|
||||
waiting on still needs the event loop to run (e.g. an **on-demand Web Worker** has to finish its
|
||||
`loaded → run` handshake), it never gets to — and the busy-spin spins forever. **Deadlock.**
|
||||
|
||||
```
|
||||
main thread: submit work ──► futex_wait(result) ──► busy-spin _emscripten_yield() ──► (spins forever)
|
||||
│ never pumps the JS event loop
|
||||
worker boot: 'loaded' ─X─► 'run' (needs the event loop, which never runs) ──► never produces result
|
||||
```
|
||||
|
||||
**Our solution — `wasm/shims/futex_yield.c`** (fix #2 below): a *strong override* of
|
||||
`emscripten_futex_wait` that, **on the main thread only**, polls the futex word and between polls does
|
||||
an **Asyncify yield** (`await setTimeout(0)`) instead of busy-spinning. Yielding pumps the JS event
|
||||
loop, so the on-demand Worker boots, finishes, wakes the futex, and the wait returns. (On worker
|
||||
threads it keeps the real blocking `memory.atomic.wait32`.) It's the sibling of the existing
|
||||
`nanosleep_yield.c`, which covers the `sleep_for`/`nanosleep` path but not the futex path.
|
||||
|
||||
## 1.7 The function table and `-sDYNCALLS=1` (one caveat to retire a red herring)
|
||||
|
||||
There is a single wasm function table; `call_indirect` indexes it; the type section lists the
|
||||
distinct signatures (`ii`, `vii`, `viii`, …). With `-sDYNCALLS=1` emscripten also exports per-signature
|
||||
`dynCall_<sig>` trampolines for JS↔wasm calls. A known hazard (`dyncall-binding.js.tmpl`) is that
|
||||
*post-asyncify+O2 a `dynCall_<sig>` JS trampoline can carry a stale expected type* — but that is a
|
||||
**legacy-EH** mechanism (the `invoke_*`→`dynCall` path). Under native-EH a C++ virtual call is a
|
||||
**raw `call_indirect`**, not a `dynCall`, so that hazard does not apply. (Verified by disassembly — see
|
||||
Part 4.)
|
||||
|
||||
---
|
||||
|
||||
# Part 2 — The collab apply call chains
|
||||
|
||||
A peer edit arrives as JSON and is applied like this:
|
||||
|
||||
```
|
||||
JS: window.Module.kicadCollabApply(jsonDelta)
|
||||
└─ kicadCollabApply(std::string) [pcbnew_embind.cpp]
|
||||
└─ parse JSON → fr->CallAfter([...]) (defer to the wx main-loop drain)
|
||||
└─ COROUTINE cor([]{ doApply(fr, delta); }) (run on a libcontext fiber — §1.5)
|
||||
└─ cor.Call(0)
|
||||
└─ doApply(frame, delta) [pcbnew_embind.cpp]
|
||||
├─ for removed: commit.Remove(item)
|
||||
├─ for changed: commit.Modify(item) ──► item->Clone() (virtual, fix #1)
|
||||
│ applyChanged(item, j)
|
||||
│ └─ tr->SetStart/SetEnd (non-virtual, fine)
|
||||
│ └─ tr->SetWidth(w) ◄── VIRTUAL "vii" ★ THE HANG
|
||||
├─ for added: makeItem(...) → commit.Add(item)
|
||||
└─ commit.Push("Collaborative edit") [board_commit.cpp]
|
||||
└─ connectivity->RecalculateRatsnest(...)
|
||||
└─ thread-pool results.get() ──► emscripten_futex_wait (fix #2)
|
||||
└─ (back on the main stack, after cor.Call returns)
|
||||
└─ rebaseline() (fix #3 — moved out of the fiber)
|
||||
└─ snapshotByUuid(board)
|
||||
└─ for each item: itemToJson(item)
|
||||
└─ itemPosition/itemClass/GetText… ◄── VIRTUAL "vii" ★ more of THE HANG
|
||||
```
|
||||
|
||||
Three things in this chain are independently load-bearing, and each was a distinct bug:
|
||||
|
||||
1. `item->Clone()` (inside `commit.Modify`) and `view->Add` (inside `commit.Push`) **must** run on the
|
||||
fiber or they trap → **fix #1 (COROUTINE)**.
|
||||
2. `commit.Push`'s connectivity recompute **futex-deadlocks** on the main thread → **fix #2
|
||||
(`futex_yield.c`)**.
|
||||
3. `rebaseline()` (the post-apply snapshot) at the end of `doApply` **must** run after the fiber
|
||||
finalizes, not inside it → **fix #3 (move to main stack)**.
|
||||
|
||||
And then the headline: the `vii` virtual calls (`SetWidth`, the snapshot getters) **hang**.
|
||||
|
||||
---
|
||||
|
||||
# Part 3 — The four bugs and the fixes that work
|
||||
|
||||
| # | Bug | Symptom | Fix | Status |
|
||||
|---|-----|---------|-----|--------|
|
||||
| 1 | `Clone`/`view->Add` dispatch off the fiber | "indirect call signature mismatch" **trap** | run `doApply` in a `COROUTINE` (`kicadCollabApply`) | ✅ validated |
|
||||
| 2 | connectivity recompute futex on main thread | busy-spin **deadlock** (Worker can't boot) | `wasm/shims/futex_yield.c` (Asyncify-yield the main-thread futex wait) | ✅ validated (red→green in the `pool-callafter` repro) |
|
||||
| 3 | `rebaseline()` inside the fiber after a suspend | fiber **never finalizes**, blocks the next apply | move `rebaseline()` to the main stack after `cor.Call` | ✅ validated |
|
||||
| 4 | `vii` virtual calls (`SetWidth`, snapshot getters) | **hang** (asyncify suspend-without-resume) | devirtualize the call (class-qualify) **OR** … (see Part 4) | ⚠️ confound; see below |
|
||||
|
||||
Fixes 1–3 are real and should be kept regardless of the Part-5 decision. Build-plumbing fix to keep
|
||||
too: `-Xclang -fno-pch-timestamp` in `build-kicad-target.sh` (a PCH-staleness workaround).
|
||||
|
||||
---
|
||||
|
||||
# Part 4 — The `vii` hang is a confound (the investigation)
|
||||
|
||||
### The symptom and the obvious (wrong) theory
|
||||
The apply hangs at `SetWidth` (a `vii` call). Devirtualize it → the apply progresses to the next
|
||||
`vii` call (`GetPosition` in the snapshot) → devirtualize that → the next `vii` (`GetClass`, then the
|
||||
text getters) … Meanwhile value-returning `ii` virtuals (`Type`, `GetLayer`, `GetWidth`, `Clone`) and
|
||||
3-arg-void `viii` virtuals (`view->Update`) work. **Obvious theory: the `vii` signature is broken.**
|
||||
|
||||
### Five controlled repros — all pass
|
||||
We built a minimal libcontext-fiber app
|
||||
(`tests/apps/standalone/coroutine-pthread/vcall_fiber_repro.cpp`) that calls all four signatures on a
|
||||
non-devirtualizable polymorphic object (99 genuine `call_indirect`s, confirmed not optimized away),
|
||||
native-EH + asyncify, and progressively added every suspected ingredient:
|
||||
|
||||
| Repro | Added ingredient | Result |
|
||||
|-------|------------------|--------|
|
||||
| `vcall_fiber_repro` | fiber + 4 signatures | **all pass** |
|
||||
| + interleaved suspend | `emscripten_sleep` before each call | **all pass** |
|
||||
| `vcall_mainloop_repro` | rAF `set_main_loop` → `dynCall_v` → COROUTINE | **all pass** |
|
||||
| `vcall_ehloop_repro` | `try`/`catch_all` + RAII dtor + suspend-in-try + loop | **all pass** |
|
||||
| FIX-D test (real pcbnew) | save+clear `Asyncify.currData` before the apply | `currData` was **null**; still hangs |
|
||||
|
||||
A `vii` `call_indirect` dispatches correctly under *every* condition we could isolate — fiber,
|
||||
suspend/rewind, the rAF/`dynCall_v` boundary, even nested native-EH `try/catch_all` (the
|
||||
HoistCppCatches regime) with a suspend inside the try. The signature is **not** the cause.
|
||||
|
||||
### Two binary-level investigations — dispatch ruled out
|
||||
Disassembling the actual `pcbnew.wasm` and the repro:
|
||||
- The asyncify pass is **type-agnostic** (`binaryen/src/passes/Asyncify.cpp`): the void path is the
|
||||
*simpler* subset; `vii` and `viii` get a byte-for-byte identical guard.
|
||||
- The function **table is not reordered/re-indexed** (the only fork pass, `HoistCppCatches`, is
|
||||
intra-function; `-O2` `directize` preserves index→function bindings).
|
||||
- The **embind-vs-core compile flags match** (same EH model, `-O`, RTTI, struct-ABI) — no ABI
|
||||
divergence that could move a vtable slot or change a `call_indirect` type.
|
||||
- The parking `SetWidth` call site has the **same asyncify guard** as the repro's working `setVii`;
|
||||
no trampoline, no `i64` legalization, no stale type.
|
||||
|
||||
The single structural difference round-2 could point to was that the real call sits inside `doApply`'s
|
||||
**deeply nested native-EH `try`/`catch_all`** (48 `try` / 47 `catch_all` — every throwing JSON access
|
||||
has RAII cleanup), in a **loop**, with the suspend landing inside hoisted catch scopes — but the
|
||||
`vcall_ehloop_repro` reproduced exactly that and **passed**.
|
||||
|
||||
### Conclusion
|
||||
The `vii` correlation is a **confound**: devirtualizing removes the instrumented `call_indirect` and
|
||||
shifts the symptom to the next one. The dispatch is provably fine. The hang is a property of the **full
|
||||
180 MB asyncify+O2 module's runtime** (the real KiCad vtables/table in the live process) that no
|
||||
isolation reproduces and that the disassembly couldn't byte-verify. The one stubborn fact that resists
|
||||
*every* named mechanism: `SetWidth` parks with asyncify state **Normal** and `currData` **null** —
|
||||
identical to the repros that pass.
|
||||
|
||||
---
|
||||
|
||||
# Part 5 — The decision: (A) devirtualize-through vs (B) defer collab
|
||||
|
||||
Everything else in native-EH pcbnew is green (core e2e, 3D, and every other app —
|
||||
[`10-pthreads-native-eh.md`](10-pthreads-native-eh.md)). Collab apply is the lone holdout.
|
||||
|
||||
## Option A — devirtualize-through (treat the confound, get collab green)
|
||||
|
||||
Replace every `vii` virtual call in the apply path with a class-qualified (direct) call, dispatching
|
||||
on `Type()` where the override matters.
|
||||
|
||||
**Where the remaining work is** (the snapshot getters are already done via `itemPosition`/`itemClass`/
|
||||
the text-getter edits):
|
||||
|
||||
| File | Site | Kind |
|
||||
|------|------|------|
|
||||
| `wasm/bindings/pcbnew_embind.cpp` | `applyChanged` else-branch `aItem->SetPosition(...)` | 1 setter (type-dispatched) |
|
||||
| `wasm/bindings/pcbnew_embind.cpp` | `makeItem` added-item setters (`SetWidth`, `SetPosition`, `SetText`, …; `SetStart/SetEnd` already non-virtual, `SetLayer` already done) | ~3–5 setters |
|
||||
| `wasm/bindings/eeschema_embind.cpp` | `itemToJson` snapshot getters (`GetPosition`/`GetClass`/text — mirror of pcbnew's helpers) | ~3–5 getters (1 helper pair) |
|
||||
| `wasm/bindings/eeschema_embind.cpp` | `doApply`/`applyChanged` + `makeItem` setters (`Move` already devirtualized) | ~3–5 setters |
|
||||
|
||||
**Effort estimate (concrete):**
|
||||
- **Code edits:** ~10–15 sites, each a 1-line class-qualify or a small `switch(Type())` helper.
|
||||
Mechanically small — call it **2–4 hours of editing**, including writing the eeschema helper pair to
|
||||
match pcbnew's.
|
||||
- **The real cost is the build/test loop, not the edits.** Each un-devirtualized `vii` surfaces *one
|
||||
at a time* (the apply hangs at the first one; you fix it, rebuild ~10–35 min, it hangs at the next).
|
||||
Doing it reactively ⇒ **~10–15 build cycles**. You can cut that by proactively grepping every
|
||||
by-value getter/void setter in the two apply paths and devirtualizing them in one pass, but you
|
||||
still need a few full builds + the **3-browser** e2e (Firefox/Chrome/WebKit) per the project rule.
|
||||
Realistically **~1–2 days wall-clock**, dominated by builds + cross-browser verification.
|
||||
- **Fragility (the ongoing cost):** this fixes nothing structural. Any *new* by-value getter or void
|
||||
setter added to the apply/snapshot path later — a new field synced, a KiCad upstream change — will
|
||||
**silently re-hang** the apply under native-EH. Mitigation: a prominent comment + ideally a tiny
|
||||
lint/grep in CI flagging un-class-qualified virtual calls in the embind apply paths. Without that,
|
||||
it's a latent foot-gun.
|
||||
|
||||
**Net:** A is a known, finite, *certain* path to green, but it's symptom-treatment with a maintenance
|
||||
tail.
|
||||
|
||||
## Option B — defer collab (recommended)
|
||||
|
||||
Ship native-EH as the default for everything that's green (core, 3D, gerbview, pl_editor,
|
||||
symbol_editor, eeschema, footprint_editor). Leave the **collab apply** path as a documented native-EH
|
||||
limitation; it continues to work on the legacy-EH build. Revisit if/when the large-module root ever
|
||||
surfaces (e.g. a future Binaryen/emscripten bump changes the picture, or someone reproduces it
|
||||
minimally).
|
||||
|
||||
**Why recommended:** the root resisted 2 deep binary investigations + 5 controlled repros + the FIX-D
|
||||
test; the dispatch is provably correct; A is fragile work treating a confound. The cost/benefit of
|
||||
chasing a non-reproducible large-module runtime bug — or maintaining a hand-devirtualized apply path
|
||||
forever — is poor relative to shipping the 95% that's done.
|
||||
|
||||
---
|
||||
|
||||
## Appendix — artifacts and key locations
|
||||
|
||||
**Validated fixes (keep):**
|
||||
- `wasm/shims/futex_yield.c` — main-thread futex Asyncify-yield (fix #2).
|
||||
- `wasm/bindings/pcbnew_embind.cpp` — `kicadCollabApply` COROUTINE (fix #1) + rebaseline-on-main-stack
|
||||
(fix #3) + the `itemPosition`/`itemClass`/`itemLayer` devirtualized snapshot helpers.
|
||||
- `scripts/kicad/build-kicad-target.sh` — `futex_yield.o` wired into the link; `-fno-pch-timestamp`.
|
||||
|
||||
**Isolation repros (temporary — remove before final staging):**
|
||||
- `tests/apps/standalone/coroutine-pthread/vcall_fiber_repro.cpp` (signature isolation, +suspend)
|
||||
- `tests/apps/standalone/coroutine-pthread/vcall_mainloop_repro.cpp` (rAF/`dynCall_v` context)
|
||||
- `tests/apps/standalone/coroutine-pthread/vcall_ehloop_repro.cpp` (try/catch_all + RAII + loop)
|
||||
- `tests/apps/standalone/pool-callafter/` (the futex deadlock red→green repro for fix #2)
|
||||
- `tests/e2e/coroutine-vcall.spec.ts`, `tests/e2e/coroutine-poolwait.spec.ts`
|
||||
- The `[collab-diag]`/`[push-diag]` `EM_ASM` markers in `pcbnew_embind.cpp` + `board_commit.cpp` and
|
||||
the `#include <emscripten.h>` in `board_commit.cpp` are temporary diagnostics to revert.
|
||||
|
||||
**Key reading:**
|
||||
- `binaryen/src/passes/Asyncify.cpp` — the suspend/rewind instrumentation (type-agnostic).
|
||||
- `scripts/common/inject-dyncall-shims.sh`, `scripts/common/shims/{handlesleep.js,dyncall-binding.js.tmpl}`
|
||||
— the JS-side asyncify/dynCall plumbing.
|
||||
- [`docs/features/async/`](../async/) — the `currData` contention dossier.
|
||||
|
|
@ -1,5 +1,9 @@
|
|||
# `-fexceptions` vs `-fwasm-exceptions` in KiCad-WASM — research dossier
|
||||
|
||||
> **✅ FINALIZED:** native wasm-EH is now the **only** build mode — there is no `-fexceptions` /
|
||||
> `WX_LEGACY_EH` path, and the 3D viewer builds by default. The migration plan, audit, and spike
|
||||
> notes below are retained as the historical research/decision record.
|
||||
|
||||
> **Status:** research / decision record. A parallel session attempted the migration
|
||||
> end-to-end and **parked it** on an emscripten-4.0.2 LLVM codegen bug — see
|
||||
> [`docs/wasm-exceptions-experiment.md`](../../wasm-exceptions-experiment.md) (full
|
||||
|
|
@ -11,6 +15,15 @@
|
|||
> `currData` contention dossier) — this dossier covers the *exception-handling* axis of
|
||||
> the same machine.
|
||||
|
||||
> **UPDATE 2026-06-22 (see [`06-spike-plan.md`](06-spike-plan.md)).** A 5-agent spike refreshed
|
||||
> this dossier and corrected three things below: (1) **the encoding is resolved to LEGACY**
|
||||
> (`WASM_LEGACY_EXCEPTIONS=1`) — Asyncify can't consume exnref in any released Binaryen, so the
|
||||
> "exnref → TryTable variant" fork is closed; the experiment's `=0` was a dead end. (2) **Binaryen
|
||||
> is not a blocker** — CI/publish already pin `BINARYEN_VERSION=130` (the "v121 locally" note below
|
||||
> is only the finalize/in-link copy). (3) The long pole is the **emsdk/LLVM compiler bump** for
|
||||
> parseable legacy wasm-EH + the OCC `br_table` fix, *not* a newer wasm-opt. The phased red-green
|
||||
> plan lives in 06.
|
||||
|
||||
## Why this exists
|
||||
|
||||
The whole build is on **`-fexceptions`** (Emscripten's JavaScript-based exception
|
||||
|
|
@ -60,10 +73,12 @@ KiCad at all.
|
|||
| [`01-background-two-eh-models.md`](01-background-two-eh-models.md) | How JS-EH (`invoke_*`) and wasm-EH actually work, and the three concrete couplings into our Asyncify machine. |
|
||||
| [`02-measurements.md`](02-measurements.md) | Our controlled size experiment on pcbnew (methodology + numbers) and the published third-party benchmarks. |
|
||||
| [`03-toolchain-status.md`](03-toolchain-status.md) | Compatibility matrix: emcc checks, binaryen history (what merged in v125, what didn't), JSPI/fibers, setjmp/longjmp, mixing modes. |
|
||||
| [`04-kicad-audit.md`](04-kicad-audit.md) | The brace-matching catch-block audit: 85 direct / 93 review / 458 trivial of 636; libpng/libjpeg setjmp story; refactor effort if done by hand. |
|
||||
| [`05-asyncify-fork-design.md`](05-asyncify-fork-design.md) | Asyncify.cpp internals, why catch arms are structurally hard, and the catch-arm-hoisting fork design with limits and effort. |
|
||||
| [`catch_audit.py`](catch_audit.py) | The audit tool (re-runnable; suitable as a CI gate on the kicad submodule). |
|
||||
| [`audit-results.txt`](audit-results.txt) | Full audit output incl. all 85 direct-suspend sites. |
|
||||
| [`06-spike-plan.md`](06-spike-plan.md) | **(2026-06-22)** Refreshed findings + the phased red-green spike plan; supersedes the encoding/Binaryen-version framing above. |
|
||||
| [`07-spike-results-and-opinion.md`](07-spike-results-and-opinion.md) | **(2026-06-22)** Toy-spike results: asyncify + legacy-wasm-EH works; the `HoistCppCatches` Binaryen pass flips suspend-in-catch green on all 3 engines; go/no-go opinion. |
|
||||
| [`08-wx-app-render-rootcause.md`](08-wx-app-render-rootcause.md) | Why a native-EH wx app rendered blank: the `set_main_loop` `"unwind"` throw caught by native-EH `catch_all` cleanup pads tore down the main frame. |
|
||||
| [`09-event-loop-deparking-plan.md`](09-event-loop-deparking-plan.md) | The EH-agnostic main-loop rework (de-park → per-frame-yield `while`-loop) fixing the blank render + the coroutine/menu regressions. |
|
||||
| [`10-pthreads-native-eh.md`](10-pthreads-native-eh.md) | **(2026-06-24)** Native-EH × pthreads: the main-thread thread-spawn regression (`invalid state: 1` / re-entrant `main()`); the pool pattern survives; the KiCad raw→pool refactor plan + the test gap. |
|
||||
|
||||
## Relationship to docs/features/async/
|
||||
|
||||
|
|
|
|||
|
|
@ -1,135 +0,0 @@
|
|||
{
|
||||
"total": 636,
|
||||
"trivial": 458,
|
||||
"needs_review": 93,
|
||||
"direct_suspend": 85
|
||||
}
|
||||
|
||||
== per top-level dir (direct/review/trivial/infobar):
|
||||
eeschema direct= 37 review= 17 trivial=127 infobar= 0
|
||||
pcbnew direct= 32 review= 29 trivial=128 infobar= 0
|
||||
common direct= 9 review= 35 trivial=126 infobar= 0
|
||||
cvpcb direct= 3 review= 1 trivial= 0 infobar= 0
|
||||
pcb_calculator direct= 2 review= 0 trivial= 2 infobar= 0
|
||||
kicad direct= 1 review= 1 trivial= 21 infobar= 0
|
||||
pagelayout_editor direct= 1 review= 1 trivial= 3 infobar= 0
|
||||
scripting direct= 0 review= 0 trivial= 1 infobar= 0
|
||||
plugins direct= 0 review= 1 trivial= 4 infobar= 0
|
||||
3d-viewer direct= 0 review= 2 trivial= 6 infobar= 0
|
||||
utils direct= 0 review= 4 trivial= 27 infobar= 0
|
||||
libs direct= 0 review= 2 trivial= 7 infobar= 0
|
||||
gerbview direct= 0 review= 0 trivial= 6 infobar= 0
|
||||
|
||||
== top 25 unknown callees inside needs_review catches (freq):
|
||||
4 clearOutlines
|
||||
4 message
|
||||
4 wxASSERT_MSG
|
||||
4 handleException
|
||||
4 tl::unexpected
|
||||
3 wxLogFatalError
|
||||
3 GetFullFilename
|
||||
3 std::string
|
||||
3 ReportMsg
|
||||
3 line_at
|
||||
3 positions
|
||||
3 LIBRARY_PARSE_ERROR
|
||||
3 move_push
|
||||
3 wxRemoveFile
|
||||
3 IsTooRecent
|
||||
3 Contents
|
||||
3 GetFieldValue
|
||||
3 ToOrigString
|
||||
3 SetValue
|
||||
2 GetFilename
|
||||
2 ClearShapes
|
||||
2 GetFPIDAsString
|
||||
2 wxString::FromUTF8
|
||||
2 GetFormatName
|
||||
2 nan
|
||||
|
||||
== direct-suspend sites (85):
|
||||
cvpcb/display_footprints_frame.cpp:310
|
||||
cvpcb/cvpcb_mainframe.cpp:977
|
||||
cvpcb/readwrite_dlgs.cpp:163
|
||||
pcb_calculator/datafile_read_write.cpp:69
|
||||
pcb_calculator/calculator_panels/panel_r_calculator.cpp:130
|
||||
kicad/kicad_manager_frame.cpp:822
|
||||
common/draw_panel_gal.cpp:325
|
||||
common/draw_panel_gal.cpp:561
|
||||
common/drawing_sheet/ds_data_model_io.cpp:100
|
||||
common/drawing_sheet/ds_data_model_io.cpp:131
|
||||
common/widgets/design_block_pane.cpp:121
|
||||
common/widgets/design_block_pane.cpp:212
|
||||
common/widgets/design_block_pane.cpp:315
|
||||
common/widgets/design_block_pane.cpp:356
|
||||
common/widgets/design_block_pane.cpp:418
|
||||
pcbnew/load_select_footprint.cpp:378
|
||||
pcbnew/pcb_base_frame.cpp:1111
|
||||
pcbnew/pcb_base_frame.cpp:1239
|
||||
pcbnew/pcb_edit_frame.cpp:2209
|
||||
pcbnew/files.cpp:670
|
||||
pcbnew/files.cpp:678
|
||||
pcbnew/files.cpp:689
|
||||
pcbnew/files.cpp:1024
|
||||
pcbnew/files.cpp:1108
|
||||
pcbnew/footprint_libraries_utils.cpp:198
|
||||
pcbnew/footprint_libraries_utils.cpp:288
|
||||
pcbnew/footprint_libraries_utils.cpp:401
|
||||
pcbnew/footprint_libraries_utils.cpp:545
|
||||
pcbnew/footprint_libraries_utils.cpp:613
|
||||
pcbnew/footprint_libraries_utils.cpp:673
|
||||
pcbnew/footprint_libraries_utils.cpp:817
|
||||
pcbnew/pcb_design_block_utils.cpp:107
|
||||
pcbnew/pcb_design_block_utils.cpp:168
|
||||
pcbnew/pcb_design_block_utils.cpp:198
|
||||
pcbnew/pcb_design_block_utils.cpp:226
|
||||
pcbnew/pcb_design_block_utils.cpp:332
|
||||
pcbnew/pcb_design_block_utils.cpp:429
|
||||
pcbnew/pcb_draw_panel_gal.cpp:781
|
||||
pcbnew/tools/board_editor_control.cpp:751
|
||||
pcbnew/tools/footprint_editor_control.cpp:529
|
||||
pcbnew/tools/pcb_control.cpp:1990
|
||||
pcbnew/netlist_reader/netlist.cpp:74
|
||||
pcbnew/exporters/export_idf.cpp:662
|
||||
pcbnew/exporters/export_idf.cpp:670
|
||||
pcbnew/dialogs/dialog_board_setup.cpp:385
|
||||
pcbnew/specctra_import_export/specctra_import.cpp:71
|
||||
pcbnew/widgets/pcb_design_block_preview_widget.cpp:191
|
||||
eeschema/project_rescue.cpp:671
|
||||
eeschema/project_rescue.cpp:819
|
||||
eeschema/sch_draw_panel.cpp:200
|
||||
eeschema/project_sch.cpp:108
|
||||
eeschema/project_sch.cpp:122
|
||||
eeschema/sch_design_block_utils.cpp:139
|
||||
eeschema/sch_design_block_utils.cpp:179
|
||||
eeschema/sch_design_block_utils.cpp:222
|
||||
eeschema/sch_design_block_utils.cpp:364
|
||||
eeschema/sch_design_block_utils.cpp:446
|
||||
eeschema/sch_design_block_utils.cpp:526
|
||||
eeschema/sch_edit_frame.cpp:1528
|
||||
eeschema/sch_base_frame.cpp:96
|
||||
eeschema/sheet.cpp:231
|
||||
eeschema/files-io.cpp:356
|
||||
eeschema/files-io.cpp:365
|
||||
eeschema/files-io.cpp:374
|
||||
eeschema/files-io.cpp:911
|
||||
eeschema/files-io.cpp:1410
|
||||
eeschema/files-io.cpp:1423
|
||||
eeschema/symbol_library_manager.cpp:376
|
||||
eeschema/symbol_library_manager.cpp:553
|
||||
eeschema/tools/sch_editor_control.cpp:569
|
||||
eeschema/tools/sch_editor_control.cpp:1703
|
||||
eeschema/dialogs/dialog_edit_symbols_libid.cpp:742
|
||||
eeschema/dialogs/dialog_sheet_properties.cpp:597
|
||||
eeschema/dialogs/dialog_sim_model.cpp:1526
|
||||
eeschema/dialogs/dialog_bom.cpp:361
|
||||
eeschema/sim/spice_simulator.cpp:44
|
||||
eeschema/sim/spice_value.cpp:415
|
||||
eeschema/sim/simulator_frame_ui.cpp:1672
|
||||
eeschema/symbol_editor/symbol_editor.cpp:170
|
||||
eeschema/symbol_editor/symbol_editor.cpp:236
|
||||
eeschema/symbol_editor/symbol_editor.cpp:1147
|
||||
eeschema/symbol_editor/symbol_editor.cpp:1187
|
||||
eeschema/symbol_editor/symbol_editor_import_export.cpp:104
|
||||
eeschema/symbol_editor/symbol_editor_import_export.cpp:110
|
||||
pagelayout_editor/tools/pl_edit_tool.cpp:531
|
||||
|
|
@ -1,109 +0,0 @@
|
|||
#!/usr/bin/env python3
|
||||
"""Audit KiCad catch blocks for wasm-EH safety (suspension inside catch handlers)."""
|
||||
import os, re, sys, json
|
||||
from collections import defaultdict
|
||||
|
||||
ROOT = "/Users/V/IdeaProjects/kicad-wasm/kicad"
|
||||
SKIP_DIRS = {"thirdparty", ".git", "qa", "build"}
|
||||
|
||||
# Calls that suspend (Asyncify) directly or are dialog wrappers in KiCad/wx
|
||||
DIRECT_SUSPEND = [
|
||||
"DisplayErrorMessage", "DisplayError", "DisplayInfoMessage", "DisplayHtmlInfoMessage",
|
||||
"wxMessageBox", "ShowModal", "ShowQuasiModal", "KIDIALOG", "OKOrCancelDialog",
|
||||
"wxMessageDialog", "IsOK(", "DisplayLoadError", "ShowAboutDialog",
|
||||
"wxGetSingleChoice", "wxTextEntryDialog", "wxFileDialog", "wxDirDialog",
|
||||
"GetDataFromClipboard", "SaveToClipboard", "wxClipboard", "EnumerateFacenames",
|
||||
]
|
||||
INFOBAR = ["ShowInfoBarError", "ShowInfoBarMsg", "ShowInfoBarWarning"]
|
||||
# Benign callees: logging (wxLog* is deferred to idle-time flush -> not inside catch),
|
||||
# string formatting, rethrow, reporters writing text
|
||||
BENIGN = {
|
||||
"wxLogError", "wxLogWarning", "wxLogMessage", "wxLogTrace", "wxLogDebug", "wxLogVerbose",
|
||||
"Format", "Printf", "printf", "fprintf", "snprintf", "What", "Problem", "Where",
|
||||
"GetErrorMessage", "wxString", "FROM_UTF8", "TO_UTF8", "UTF8", "c_str", "mb_str",
|
||||
"GetChars", "IsEmpty", "empty", "clear", "size", "length", "Report", "ReportTail",
|
||||
"ReportHead", "Add", "push_back", "emplace_back", "insert", "append", "Append",
|
||||
"SetError", "assert", "wxASSERT", "wxFAIL", "wxCHECK", "abort", "exit",
|
||||
"GetMessages", "GetErrors", "reset", "get", "release", "find", "count", "at",
|
||||
"begin", "end", "str", "Mid", "Left", "Right", "Trim", "Lower", "Upper",
|
||||
"StartsWith", "EndsWith", "Contains", "Replace", "Remove", "make_unique",
|
||||
"make_shared", "static_cast", "dynamic_cast", "const_cast", "reinterpret_cast",
|
||||
"Clear", "Close", "swap", "resize", "erase", "Set", "SetBitmap", "Destroy",
|
||||
"wxT", "_", "_HKI", "traceSchPlugin", "TRACE", "what", "THROW_IO_ERROR", "wxS", "wxFAIL_MSG", "IDF_ERROR", "LIBRARY_ERROR", "FUTURE_FORMAT_ERROR", "PARSE_ERROR", "KI_PARAM_ERROR", "fmt::format", "format", "GetFullPath", "HandleException", "Pgm", "current_exception", "rethrow_exception", "Nickname", "GetName", "GetLibNickname", "GetLibItemName", "wx_str", "GetMessageString", "UnescapeString", "exceptions", "CLOSE_STREAM", "AddError", "GetRequiredVersion", "Disconnect", "From_UTF8", "typeid", "LogException", "Instance", "GetFullName", "GetUniStringLibId", "ShowText", "SetStatusText", "GetItemDescription", "GetClass", "GetFriendlyName", "IsValid", "GetPath", "GetFileName", "GetExtension", "Length", "GetData", "data", "front", "back", "pop_back", "emplace", "GetSettingsManager",
|
||||
}
|
||||
CALL_RE = re.compile(r"\b([A-Za-z_][A-Za-z0-9_:]*)\s*\(")
|
||||
CATCH_RE = re.compile(r"\bcatch\s*\(")
|
||||
|
||||
def find_block(text, start):
|
||||
"""Return (block_text, end_idx) for brace-block starting at first '{' at/after start."""
|
||||
i = text.find("{", start)
|
||||
if i < 0: return None, start
|
||||
depth, j, n = 0, i, len(text)
|
||||
in_str = in_chr = in_lc = in_bc = False
|
||||
while j < n:
|
||||
c = text[j]; p = text[j-1] if j else ""
|
||||
if in_lc:
|
||||
if c == "\n": in_lc = False
|
||||
elif in_bc:
|
||||
if p == "*" and c == "/": in_bc = False
|
||||
elif in_str:
|
||||
if c == '"' and p != "\\": in_str = False
|
||||
elif in_chr:
|
||||
if c == "'" and p != "\\": in_chr = False
|
||||
elif c == "/" and j+1 < n and text[j+1] == "/": in_lc = True
|
||||
elif c == "/" and j+1 < n and text[j+1] == "*": in_bc = True
|
||||
elif c == '"': in_str = True
|
||||
elif c == "'": in_chr = True
|
||||
elif c == "{": depth += 1
|
||||
elif c == "}":
|
||||
depth -= 1
|
||||
if depth == 0: return text[i:j+1], j+1
|
||||
j += 1
|
||||
return None, start
|
||||
|
||||
stats = defaultdict(int)
|
||||
direct_sites, review_sites = [], []
|
||||
review_callees = defaultdict(int)
|
||||
per_dir = defaultdict(lambda: defaultdict(int))
|
||||
|
||||
for dirpath, dirnames, filenames in os.walk(ROOT):
|
||||
dirnames[:] = [d for d in dirnames if d not in SKIP_DIRS]
|
||||
for fn in filenames:
|
||||
if not fn.endswith(".cpp"): continue
|
||||
path = os.path.join(dirpath, fn)
|
||||
rel = os.path.relpath(path, ROOT)
|
||||
top = rel.split(os.sep)[0]
|
||||
try: text = open(path, encoding="utf-8", errors="replace").read()
|
||||
except OSError: continue
|
||||
for m in CATCH_RE.finditer(text):
|
||||
block, _ = find_block(text, m.end())
|
||||
if block is None: continue
|
||||
stats["total"] += 1
|
||||
line = text[:m.start()].count("\n") + 1
|
||||
loc = f"{rel}:{line}"
|
||||
if any(s in block for s in DIRECT_SUSPEND):
|
||||
stats["direct_suspend"] += 1; per_dir[top]["direct"] += 1
|
||||
direct_sites.append(loc)
|
||||
continue
|
||||
if any(s in block for s in INFOBAR):
|
||||
stats["infobar"] += 1; per_dir[top]["infobar"] += 1
|
||||
continue
|
||||
calls = set(CALL_RE.findall(block)) - {"catch", "if", "for", "while", "switch", "return", "sizeof", "throw"}
|
||||
unknown = {c for c in calls if c.split("::")[-1] not in BENIGN and c not in BENIGN}
|
||||
if not unknown:
|
||||
stats["trivial"] += 1; per_dir[top]["trivial"] += 1
|
||||
else:
|
||||
stats["needs_review"] += 1; per_dir[top]["review"] += 1
|
||||
review_sites.append((loc, sorted(unknown)[:6]))
|
||||
for c in unknown: review_callees[c] += 1
|
||||
|
||||
print(json.dumps(stats, indent=1))
|
||||
print("\n== per top-level dir (direct/review/trivial/infobar):")
|
||||
for d in sorted(per_dir, key=lambda d: -per_dir[d]["direct"]):
|
||||
p = per_dir[d]
|
||||
print(f" {d:24s} direct={p['direct']:3d} review={p['review']:3d} trivial={p['trivial']:3d} infobar={p['infobar']:2d}")
|
||||
print("\n== top 25 unknown callees inside needs_review catches (freq):")
|
||||
for c, n in sorted(review_callees.items(), key=lambda kv: -kv[1])[:25]:
|
||||
print(f" {n:3d} {c}")
|
||||
print(f"\n== direct-suspend sites ({len(direct_sites)}):")
|
||||
for s in direct_sites: print(" " + s)
|
||||
Loading…
Reference in a new issue