Repository navigation
debug: inspect parked Wasm goroutine source stacks - #2742
Conversation
|
@fennoai please review the opt-in Wasm logical goroutine registry, caller-frame instrumentation and standalone decoder, especially GC/lifetime safety, paused-worker consistency and default-build overhead. Full compiler/browser/debugger/schema tests and the actual Chrome Memory32/64 1/2/4-worker matrix pass locally, including parked recursive stacks after GC and record removal after exit. The frontend still requires pausing other worker threads before shared-memory inspection; it does not claim automatic worker control or parked-frame locals. |
There was a problem hiding this comment.
Review: WASM debug runtime + goroutines panel
Thorough, well-engineered PR. The lock-free registry snapshot protocol (epoch at the registry level, per-node odd/even sequence guards with begin/end double-reads, re-verified in the JS reader after the walk) is clean and well-tested (Memory32/64, 1/2/4 workers, corruption cases). Build-tag partitioning across the stub / wasm-debug / other variants fully covers with no overlap. Docs (dev/browser-debugging.md, doc/debugging.md, the READMEs) and schema_v1.json accurately match the runtime behavior and consumer fields.
Security: no actionable issues. Goroutine strings reach DevTools as structured protocol objects ({type:'string', ...}), not HTML, so no DOM-injection sink is introduced; keep them text-only downstream. Memory reads are bounded (LIMIT = 4096, cycle detection, numberAddress range checks, Chrome's bounds-checked getWasmLinearMemory).
Findings below are minor/perf — nothing blocking.
Focused review: GC/lifetime, paused-worker consistency, default-build overheadI verified each of your three highlighted concerns directly against the code (locked range ✅ Default-build overhead — zero
✅ GC / lifetime safety
✅ Paused-worker consistencyThe seqlock protocol is correct and defensively enforced in the standalone decoder (
Net effect: if a worker is not actually paused, the decoder fails loudly with a "pause all workers" error rather than returning torn/corrupt data — which matches your stated contract that the frontend requires pausing other workers and makes no automatic-worker-control claim. Notes (non-blocking, carried over from the diff review)
No blocking issues on any of the three areas you flagged. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Current doc_verify failure is the unchanged README LLDB homepage (https://lldb.llvm.org/): lychee reports a connection failure, with 95 other links passing and no excluded links. Direct access currently returns HTTP 200 and the official documentation remains valid. I have not retried CI, added link exclusions/tolerance, or changed a valid URL to bypass the failure. The full local Chrome debugger matrix and all 23 extension tests pass; the fresh hosted runtime/debugger checks are still running. |
|
@fennoai please check the small P0 correction in eed7d78. Runtime P IDs are zero-based; debugger records now initialize last-P to -1 and the decoder reads the signed field so P0 remains valid. A new Memory32/64 unit test covers zero and the unstarted sentinel, and the real Chrome matrix now asserts the main G is P0 and both parked Gs have valid last-P values. All 24 extension tests and the complete actual 1/2/4-worker, embedded/external matrix pass. The earlier external-link failure was left intact; this head changes runtime decoding and tests, with no URL or CI-rule changes. |
P0 correction verified ✅ — zero-based processor IDsChecked The P0 fix (
|
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 |
ede6941 to
49441f3
Compare
49441f3 to
e2b9d68
Compare
visualfc
left a comment
There was a problem hiding this comment.
Approve. The lock-free registry snapshot, opt-in logical frames, and $goroutines decoder look correct. One non-blocking follow-up on empty Go strings.
Browser debug builds retain logical Go caller frames and a versioned runtime registry, so DevTools
$goroutinescan show live IDs, parent IDs, state, last processor and source stacks even when a fiber is parked.llgo debugenables the opt-in tag; ordinary builds retain their existing instrumentation policy. The extension decoder uses batched memory reads, sequence guards and lazy snapshot objects that are released with their children.Validation: compiler/debugger/schema Go tests and extension tests; ten real Chrome for Testing 152 sessions covering Memory32/64, 1/2/4 workers, embedded/external DWARF, parked recursive stacks after GC, P0 display and record removal after exit. The same browser acceptance runs in PR CI.
Rebased on main
1bbfc1f75, preserving upstream's caller-frame slot reuse and cached source-location handling. Restore the GC metadata fixture’s segmented-heap mode required by the current runtime. The macOS deadline regression that failed the previous CI head passes three consecutive runs with the upstream fix. Update the pinned QEMU image to 10.2.3 after an ARM64 sysroot tool query crashed under QEMU 7; all 52 CI workflow contract tests pass.Users pause all worker threads in DevTools before inspecting shared state. Logical frames expose function/file/line, not parked-frame locals or automatic worker control. Advances browser runtime views in #2164; WASI guest-debug transport and physical-board qualification remain separate.