From 80468e7c5bdbbade0bf8f2cde62580e431ed3c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20T=C3=B6rcsv=C3=A1ri?= Date: Fri, 7 Aug 2026 20:48:35 +0200 Subject: [PATCH] Phase B: main-thread sleep parks its context; the real D blocker is DOM entries wasm/shims/context_sleep.cpp: a main-thread nanosleep whose frame stands on a scheduler context that OWNS the stack arms a mailbox wake and yield_parks that context instead of suspending the stack in place. It lives in the sleep primitive rather than in tool_manager.cpp on purpose - KiCad and the wx core stay untouched (CLAUDE.md's fork rule) and the whole K7 class moves at once, not just TOOL_MANAGER::RunSynchronousAction's spin loop. MEASURED AT D-ON, and it is NOT what unblocks Phase D. The four canvas-tool specs still fail, but the trace now names a different cause: the fatal swap is old= new= with mouseEventHandlerFunc above it - a DOM mouse handler entering wasm DIRECTLY on the main stack, bypassing the tick. So one coroutine is entered two ways: by the tick through the dispatch context as a STAR TRANSFER, and by DOM handlers as a DIRECT SYMMETRIC SWAP. A capture written by one path cannot be rewound by the other -> index out of bounds in doRewind. That is section 7 rule 5 (partial migration is worse than none) in its purest measured form, and it is why the harness stays green: its coroutines are only ever entered from one place. So the next increment is the DOM event entries (mouse/key/wheel/resize must hand their events to the dispatch context as the tick does), not another park site. It subsumes the one-root work too: with no dispatch on the main stack, resolve_root_identity() always answers "the running context". Landing state: STAR_DISPATCH=0, kicad 139 passed / 1 (pre-existing occ-probe) = baseline, with the sleep shim in and inert. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LBjomQfKyRa3jBdeAKpmTw --- .../features/async/22-absorbing-libcontext.md | 49 ++++++++ scripts/kicad/build-kicad-target.sh | 9 +- wasm/shims/context_sleep.cpp | 109 ++++++++++++++++++ wasm/shims/nanosleep_yield.c | 17 ++- wxwidgets | 2 +- 5 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 wasm/shims/context_sleep.cpp diff --git a/docs/features/async/22-absorbing-libcontext.md b/docs/features/async/22-absorbing-libcontext.md index d5df82d..9f832e1 100644 --- a/docs/features/async/22-absorbing-libcontext.md +++ b/docs/features/async/22-absorbing-libcontext.md @@ -745,6 +745,55 @@ sleeping-in-place. Notes for whoever takes it: never the harness battery — the harness has no `RunSynchronousAction` and will stay green either way. That is this session's most transferable lesson. +### The sleep moved to a context — and the REAL blocker surfaced (2026-08-07) + +**Built and measured.** `wasm/shims/context_sleep.cpp`: a main-thread `nanosleep` +whose frame stands on a scheduler context that OWNS the stack arms a mailbox wake and +`yield_park`s that context instead of suspending the stack in place; anything else +falls back to the Asyncify yield. It lives in the sleep primitive rather than in +`tool_manager.cpp` deliberately — KiCad and the wx core stay untouched (CLAUDE.md's +fork rule), and the whole K7 class moves at once instead of the one measured caller. + +**One correction paid for on the way, worth keeping in mind for every future park +site: a context may have only ONE wake owner.** The first cut parked whatever context +was current, including the MAIN-LOOP context — whose wake already belongs to the rAF +pump. The frame wake then resumed a capture the sleep wake had consumed: +`doRewind` trap arriving through `wxWasmArmFrameWake`, and the eeschema simulator spec +went red at D-off. `wxWasmContextWakeIsPumpOwned()` now excludes the main-loop and +dispatch contexts (their parks are the pumps' contract); tool coroutines, which have +no other wake source, are exactly the ones that park. **D-off re-verified at 139/1 +with the shim in — it is correct-or-inert exactly as required.** + +**At D-on the four canvas-tool specs still fail, and the trace names a DIFFERENT +cause — the one that actually blocks Phase D.** The fatal swap is +`fcs old= new= rf=dynCall_iiii`, and the JS stack +above it is `mouseEventHandlerFunc` → `registerMouseEventCallback`: **a DOM mouse +handler entering wasm DIRECTLY on the main stack**, not through the tick. + +So the same tool coroutine is entered by two different mechanisms: + +| entry | path | how the coroutine is resumed | +|---|---|---| +| the tick (`wxWasmTopLevelTick`) | dispatch context → `fiber_transfer` | STAR: parked by the scheduler, capture owned by the scheduler swap | +| a DOM mouse/key handler | main stack, `current() == 0` → direct-swap fallback | SYMMETRIC: entered by `emscripten_fiber_swap` from the root | + +A coroutine suspended by a star transfer and later resumed by a direct symmetric swap +rewinds through an entry path its capture was not written for — `index out of bounds` +in `doRewind`. This is doc 22 §7 rule 5 (partial migration is worse than none) in its +purest measured form, and it explains why the harness stays green: its coroutines are +only ever entered from one place. + +**Therefore the next Phase B/D increment is not another park site — it is the DOM +event entries.** Every `registerMouseEventCallback` / key / wheel / resize handler that +today runs wx dispatch inline on the main stack must instead hand its event to the +dispatch context (enqueue + pump), exactly as the tick already does. Only when EVERY +entry into a tool coroutine goes through the scheduler does the mixed-mode rewind +class disappear. Note this also subsumes the "one root" work: with no dispatch on the +main stack, `resolve_root_identity()` always answers "the running context". + +**Landing state: `wxWASM_STAR_DISPATCH` back to 0**, context-sleep and its +pump-ownership guard kept (inert at D-off, verified 139/1). + 1. **pthreads.** Doc 21 §2 settled that every Asyncify park is main-thread and the lib bridge's worker path is a blocking proxy. Phase A must re-check that libcontext is never driven from a worker before assuming the scheduler is main-thread-only. diff --git a/scripts/kicad/build-kicad-target.sh b/scripts/kicad/build-kicad-target.sh index 066d664..79a7e90 100755 --- a/scripts/kicad/build-kicad-target.sh +++ b/scripts/kicad/build-kicad-target.sh @@ -506,7 +506,14 @@ if [ "${APP_NAME}" = "kicad_tools" ] || [ "${APP_NAME}" = "occ_service" ]; then NANOSLEEP_YIELD_LINK="" else emcc -c -pthread "${PROJECT_ROOT}/wasm/shims/nanosleep_yield.c" -o "${STUBS_BUILD}/nanosleep_yield.o" - NANOSLEEP_YIELD_LINK="${STUBS_BUILD}/nanosleep_yield.o" + # Its scheduler-aware half (docs/features/async/22 Phase B): a main-thread + # sleep on a scheduler context parks THAT CONTEXT instead of suspending the + # stack in place. C++ because the registry is a header-only C++ layer in + # wx's port, hence WX_CXXFLAGS for the include path (the same reason + # thirdparty/libcontext needed it at Phase A). + em++ -c -std=c++17 -pthread ${WX_CXXFLAGS} \ + "${PROJECT_ROOT}/wasm/shims/context_sleep.cpp" -o "${STUBS_BUILD}/context_sleep.o" + NANOSLEEP_YIELD_LINK="${STUBS_BUILD}/nanosleep_yield.o ${STUBS_BUILD}/context_sleep.o" fi # mallinfo() stub for the mimalloc build: -sMALLOC=mimalloc doesn't export the diff --git a/wasm/shims/context_sleep.cpp b/wasm/shims/context_sleep.cpp new file mode 100644 index 0000000..448f50c --- /dev/null +++ b/wasm/shims/context_sleep.cpp @@ -0,0 +1,109 @@ +/* + * Main-thread sleep as a CONTEXT PARK (docs/features/async/22, Phase B). + * + * THE PROBLEM THIS SOLVES. `nanosleep` on the main thread yields via Asyncify + * (see nanosleep_yield.c): it parks THE STACK IT STANDS ON. Doc 21 filed that + * as K7, the "anywhere" class, and its worst caller is a loop inside a tool + * body — TOOL_MANAGER::RunSynchronousAction spins + * + * while( synchronousControl == STS_RUNNING ) { wxYield(); wxMilliSleep(1); } + * + * (kicad/common/tool/tool_manager.cpp:370-371). Every canvas edit/draw/move + * action goes through it. An in-place park there means a tool stack sits + * mid-Asyncify-suspension while a nested wxYield() dispatch runs on top of it, + * and once the scheduler owns dispatch (Phase D) a star transfer aimed at that + * stack rewinds a capture that is still in flight: `index out of bounds` in + * doRewind — the blue screen, measured on four canvas-tool specs 2026-08-07. + * + * THE FIX. When the sleeping frame stands on a scheduler context that OWNS + * that stack, the wait becomes what every other migrated wait already is: arm + * a timed wake, YIELD THE CONTEXT, and let the scheduler resume it. Nothing + * is suspended in place, so there is no in-flight capture for a transfer to + * land on, and the caller's `for(;;)`-shaped poll keeps its exact semantics — + * it just waits by yielding instead of by suspending. + * + * WHY HERE AND NOT IN KiCad. The loop is upstream KiCad code, and CLAUDE.md + * asks the fork to stay close to upstream. Routing this through the sleep + * primitive keeps KiCad and the wx core untouched AND fixes the whole K7 class + * at once (every main-thread sleep_for/wxMilliSleep reached on a context), not + * just the one caller that happened to be measured. + * + * WHY THE POLL DOESN'T NEED A SIGNAL. The waited-for state (an atomic set by a + * later dispatch) has no wake source of its own, so this keeps polling — the + * caller's contract. What changes is only which stack the wait suspends. The + * dispatch that eventually flips the atomic runs on a DIFFERENT context: the + * tick reuses an idle dispatch context or makes one when all are parked + * deeper (the idle-reuse set), which is exactly why that change had to land + * before this one. + */ + +#include + +#include + +// The scheduler mailbox (wx/wasm/private/mailbox.h). A timer message is the +// wake source: it is delivered from a fresh JS task on the main stack, which +// is where a resume is allowed to happen. +extern "C" void wxWasmMailboxEnqueueAfter( void ( *aFn )( void* ), void* aArg, int aMillisecs ); + +// Does some wx pump already own this context's wake (the main loop's rAF, a +// dispatch context's tick)? Parking such a context here would give it TWO +// owners, and the second wake resumes a capture the first already consumed — +// measured 2026-08-07 as a doRewind trap through wxWasmArmFrameWake. Those +// contexts keep the in-place yield; the tool coroutines this exists for have +// no other wake source, which is precisely why their wait must park. +extern "C" int wxWasmContextWakeIsPumpOwned( unsigned aId ); + +namespace +{ + +void wake_sleeper( void* aArg ) +{ + const pcbjam_sched::ContextId id = + static_cast( reinterpret_cast( aArg ) ); + + // mark_ready never resumes inline (doc 13 §1.4); drain_all performs the + // entry from this clean mailbox-tick stack. + if( pcbjam_sched::mark_ready( id, 0 ) ) + pcbjam_sched::drain_all(); +} + +} // namespace + +extern "C" { + +/** + * Park the running context for aMillisecs instead of suspending this stack. + * + * Returns 1 if the wait was taken as a context park, 0 if the caller must fall + * back to the in-place Asyncify yield — which is the right answer whenever no + * context owns this stack: the main loop itself, a bridge entered before the + * scheduler exists, or a libcontext fiber swapped in above a context (yielding + * there would save the WRONG stack — doc 22 §7 rule 4, enforced by + * can_yield_here()). + */ +int pcbjam_context_sleep_ms( double aMillisecs ) +{ + const pcbjam_sched::ContextId self = pcbjam_sched::current(); + + if( !self || !pcbjam_sched::can_yield_here() ) + return 0; + + if( wxWasmContextWakeIsPumpOwned( self ) ) + return 0; + + // Round up: a 0 ms mailbox delay would re-enter this poll in the same + // macrotask chain and spin the CPU exactly as the sleep exists to avoid. + int delay = static_cast( aMillisecs ); + + if( delay < 1 ) + delay = 1; + + wxWasmMailboxEnqueueAfter( &wake_sleeper, + reinterpret_cast( static_cast( self ) ), + delay ); + pcbjam_sched::yield_park( "main-thread-sleep" ); + return 1; +} + +} // extern "C" diff --git a/wasm/shims/nanosleep_yield.c b/wasm/shims/nanosleep_yield.c index bab0d9b..5ded746 100644 --- a/wasm/shims/nanosleep_yield.c +++ b/wasm/shims/nanosleep_yield.c @@ -33,13 +33,28 @@ EM_ASYNC_JS( void, __wasm_main_thread_yield_ms, ( double ms ), { await new Promise( function( resolve ) { setTimeout( resolve, ms ); } ); } ); +/* + * Scheduler-aware sleep (context_sleep.cpp, docs/features/async/22 Phase B): + * when this frame stands on a scheduler context that owns the stack, the wait + * PARKS THAT CONTEXT instead of suspending the stack in place. Returns 0 when + * no context owns the stack, and then the Asyncify yield below is still right. + * + * This is what makes TOOL_MANAGER::RunSynchronousAction's spin loop safe under + * Phase D: an in-place park inside a tool body leaves a capture in flight for a + * star transfer to land on (doRewind -> "index out of bounds"). + */ +extern int pcbjam_context_sleep_ms( double ms ); + int nanosleep( const struct timespec* req, struct timespec* rem ) { if( req ) { double ms = (double) req->tv_sec * 1000.0 + (double) req->tv_nsec / 1.0e6; if( emscripten_is_main_runtime_thread() ) - __wasm_main_thread_yield_ms( ms ); /* yield -> event loop runs -> Worker boots */ + { + if( !pcbjam_context_sleep_ms( ms ) ) + __wasm_main_thread_yield_ms( ms ); /* yield -> event loop runs -> Worker boots */ + } else emscripten_thread_sleep( ms ); /* worker: real blocking sleep */ } diff --git a/wxwidgets b/wxwidgets index 57b781c..b4fb50f 160000 --- a/wxwidgets +++ b/wxwidgets @@ -1 +1 @@ -Subproject commit 57b781c480e6396ea6696b8d63300b0284619dda +Subproject commit b4fb50faa869a827a69bf35832055279b265c3c8