Recover extensions after WebAssembly traps - #59
Conversation
Discard poisoned stores after traps and initialize a fresh component for the next queued call. Preserve the original error without replaying operations that may already have performed side effects. Cover recovery, ordinary errors, initialization failures, and caller cancellation with real WebAssembly regression tests. Fixes #58.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Essentials Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. 📝 SummaryRecover Wasm extensions after runtime traps Discard the poisoned store and component instance after a trap, then initialise fresh instances before processing queued calls. Preserve the original trap, avoid replaying failed operations, and surface recovery failures. Keep ordinary extension errors separate from runtime traps. Ensure queued calls complete when callers cancel their requests. Add regression tests for recovery, fresh initialisation, ordinary errors, recovery failures, and cancellation. WalkthroughThe extension host now propagates ChangesWasm extension recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The change restores extension operation after WebAssembly traps while preserving ordinary error behavior, with no substantiated merge-blocking issue remaining. Sequence Diagram(s)sequenceDiagram
participant Caller
participant WasmExtension
participant ExtensionWorker
participant WasmInstance
Caller->>WasmExtension: Queue Result-returning callback
WasmExtension->>ExtensionWorker: Send callback
ExtensionWorker->>WasmInstance: Execute callback
WasmInstance-->>ExtensionWorker: Return result or trap
ExtensionWorker->>WasmInstance: Instantiate replacement after trap
ExtensionWorker-->>Caller: Return execution or recovery result
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Comment |
Objective
Fixes #58.
A WebAssembly trap currently leaves an extension unusable until reload or editor restart. Subsequent calls keep entering the poisoned store and fail with
cannot enter component instance, as reported with the Ruby extension zed-extensions/ruby#323.Solution
Discard both the store and component instance after a Wasmtime trap. Initialize a fresh pair before serving the next queued call, retaining the compiled component and existing call queue. Return the original trap to the failed caller without replaying an operation that may already have performed side effects.
Ordinary extension errors retain the instance. Recovery failures reach the caller and leave the worker available to attempt initialization on the next request. Caller cancellation does not interrupt an in-flight invocation.
Testing
cargo test -p extension_host wasm_host::tests:: -- --nocapture: all 5 tests passed on Linux, including 4 new regressions using a real WebAssembly component.dprint check crates/extension_host/src/wasm_host.rs crates/extension_host/Cargo.tomlandgit diff --checkpassed.Self-Review Checklist
Suggested .rules additions
Keep
#[gpui::test]functions returning(). The macro discards a returnedResult, so explicitly assert or unwrap an async helper's result to ensure setup and execution errors fail the test.Release Notes: