fix(wasm): stabilize WAMR thread GC and EH boundaries - #2695
Conversation
There was a problem hiding this comment.
Review: WASI thread stability
Reviewed the full diff (base ef3fce4 → head 0ee628d) across four dimensions (code quality, performance, security, documentation). The threading/GC design is careful and well-documented: a coherent stop-the-world protocol, per-thread root chains, a segmented heap, and the C/Go Context layout coupling guarded by _Static_assert. Build-tag partitioning (default/stub vs. llgo.wasi_threads && llgo.wasm.gc.linear) is consistent with no gaps or overlaps. Test coverage of the new wasm-wasi-threaded-gc fixture is thorough (cross-thread pointer handoff, parked receivers, blocked-in-C collection skipping, arena growth). Documentation changes were verified accurate, and the new PEM fixtures are self-consistent, repository-unique test keys (not reused/compromised production keys).
No blocking correctness defects. The notes below are worth considering; one inline nit and a few design-level observations.
Performance (steady-state cost of the new WASI-threaded-GC build)
- Per-safepoint mutex on the hottest path.
wasiGCSafepoint→llgo_wasi_gc_pending(runtime/internal/runtime/_wrap/wasi_gc_world.c) takes and releases the process-wideworld_mutexunconditionally on every function entry / loop back-edge. The pending check only readsworld_epoch(auint32) plus the owner; a lock-free atomic epoch read on the fast path (taking the lock only when a stop appears pending) would remove a large fixed tax and cross-thread cache-line contention on all Go execution in this build. - O(segments) linear scans inside O(heap) GC loops.
segmentForBlock/segmentForAddress/nextSegmentBlock(tinygogc/segments.go) linear-scan up to 128 segments, and they are called per scanned word instartMark, per block in theAllocfree scan, and per block insweep. With 32 MiB arena growth this makes mark/sweep/alloc effectively O(heap·segments) for larger heaps. Since segments are contiguous in block space, deriving the segment during the walk (or a sorted array + binary search) would restore near-O(1). Relatedly,gcStateOfresolves the segment twice per call (gcStateByteOfthengcStateFromByte) — passing the resolved segment/state through would halve the scans.
Correctness / behavior
- Parked initial thread stays registered in the GC world.
parkInitialWasiThread(runtime/internal/runtime/goexit_initial_wasi_threads.go) sets the initial G dead and, when not the last goroutine, spins infor { c.Usleep(1000) }without unregistering from the GC world. Parked in uninstrumented C, it never reaches a safepoint, so subsequentllgo_wasi_gc_stopcalls hit the 500ms timeout and skip sweeping. The testedwasm-wasi-main-goexit*fixtures all converge to the deadlock exit so this path isn't exercised, but a program with a long-lived allocating worker aftermainGoexits would see repeated 500ms GC stalls and arena-only growth toward OOM. The surrounding comment documents the intent (keep the environment alive to avoid resuming a dead main), so this is a known tradeoff — worth either unregistering from the GC world before the idle loop or documenting the OOM/stall consequence explicitly.
Minor
- Inline nit on the redundant boolean clause in
segments.go(see inline comment).
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
047fcd0 to
9a2cd3c
Compare
|
Addressed the review and rebased onto current main (including merged #2539). The branch remains ready for review.
Added root-selection coverage for threaded GC versus nogc/single-worker profiles. Fresh CI is queued; no coverage threshold was relaxed. |
|
Follow-up CI fixes are now pushed in fb2a438 (still ready for review):
The prior head's full Linux Go tests and patch-coverage gate passed. Fresh CI for this script/C-only follow-up is queued. The allocator/yield experiments made while investigating timing did not produce reliable gains and are not included. |
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
fb2a438 to
4f29c5c
Compare
4291ef3 to
5b44983
Compare
5b44983 to
f46c1a8
Compare
Caught Wasm exceptions in WAMR's classic interpreter can briefly broadcast a cluster-wide termination signal while unwinding to a Wasm caller. A sibling goroutine can exit during that window, causing deferred Goexit to hang or return without running all expected code.
Apply a WAMR 2.4.5 interpreter patch that unwinds directly to the Wasm caller, plus the POSIX signal-handler backport from wasm-micro-runtime/wasm-micro-runtime#5119. Cache identity includes both patches. Uncaught exceptions escaping the native invocation still terminate execution.
Runtime pthread waits also blocked collection while reacquiring locks held by stopped Go threads. Publish suspended callers' roots around the known mutex/condition waits (including timers), and keep those callers out of Go until GC resumes. Initial Goexit unregisters the thread that will never return to Go. Use the same GC-safe pthread mutex for the allocator instead of a host-polling spin lock; let symbol-table initialization waiters acknowledge GC. Smaller initial arenas and a Release WAMR interpreter reduce collection overhead.
Add repeated GC/nogc regression coverage for cross-function panic/recover, concurrent C setjmp/longjmp, deferred worker Goexit, main/init Goexit, unrecovered panic, and a raw escaping Wasm exception. Add finalizer/reflect GC acceptance and a pthread regression for mutex reacquisition during GC. The reflection type-name lookup avoids temporary string allocation. Record the EH comparison in
dev/wasm-eh-comparison.mdand failure modes/check commands indev/wasm-wasi-validation.md.Rebased onto main at 653957f, including merged #2669, #2539, and #2700. This PR includes the remaining EH validation rather than splitting it into another PR. Fixes #2676.
Validation on macOS arm64:
bash dev/build_iwasm.shbuilds and installs the patched runner.python3 dev/test_wasm_wasi_threads.pypasses, including threaded GC, filesystem, selected standard-library packages, and GOROOT sentinel.python3 dev/compare_wasm_eh.py --browserwith LLGo Binaryen llgo-v132.3 passes all C++ encoding variants in Node/Chrome, Go panic/recover, and O0/O2 Go/C++ wrappers.test/gobinary passes all 242 top-level tests. Concurrent function-info lookup took 9.47 seconds in that run; the previous allocator exceeded the full suite's 12-minute deadline in that test.test/goin shard 9/16 passes, includingcrypto/elliptic; the subsequent completetest/gorun verifies the remaining failure is fixed. This is not a claim that the entire standard library has been re-audited on every host. CI runs Linux acceptance and the cross-platform WAMR builds.Review follow-up: safepoints use an atomic epoch fast path; known blocked conditions/timers no longer poll every 20 ms. Block-state walks carry their segment, address/block lookup uses binary search, and metadata layout is shared by initial allocation and contiguous growth without clearing live metadata. Regression coverage includes 128 out-of-order arenas and indefinite pthread wait/reacquisition. The GC fixture compiles once; finalizers/callback/symbols, the 20 pointer-argument startup races, and the 20 zero-argument startup races execute in three fresh WAMR invocations, each bounded by 300 seconds. Linux amd64 validation of the unchanged runtime completed those groups in 15.94, 36.81 and 11.51 seconds. Every test and repetition remains enabled. Linux arm64 build and execution of the focused regression both pass locally; CI remains the cross-platform gate.
The HTML host self-build and per-worker callback bridge fixes now come from main via #2700; duplicate contribution commits have been removed. Linux host Go tests and patch coverage passed before this rebase; fresh CI validates the rebased head.