Small fixes for the SDK - #6833
Conversation
There was a problem hiding this comment.
Pull request overview
This PR applies several targeted fixes across WASIX, the JS backend, and the C-API imports crate to improve SDK behavior on main, primarily by addressing platform-specific build/runtime issues and re-entrancy/thread-safety edge cases.
Changes:
- Avoid nested
RefCellborrow panics in WASIX signal registration and thread-local handle access by switching some borrows to non-panickingtry_*variants. - Improve JS/wasm32 compatibility by adjusting time imports (
web_time) and selecting WASIX dependency features per target (sysvsjs). - Extend the JS backend module representation to retain
ModuleInfowhenwasm-types-polyfillis enabled, and improve theinfo()failure message otherwise.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/wasix/src/syscalls/wasix/callback_signal.rs | Switches signal callback registration to a non-panicking inner-mut access path to tolerate re-entrancy. |
| lib/wasix/src/state/handles/thread_local.rs | Replaces a panicking mutable borrow with try_borrow_mut and makes destroy avoid panicking on borrow conflicts. |
| lib/virtual-fs/src/mount_fs.rs | Uses web_time on js builds while retaining std::time elsewhere. |
| lib/c-api-imports/src/lib.rs | Adds an unsafe impl Send for WasmCapiEnv on wasm32 to satisfy FunctionEnv constraints. |
| lib/c-api-imports/Cargo.toml | Selects wasmer-wasix feature set by target arch (sys on native, js on wasm32). |
| lib/api/src/backend/js/entities/module.rs | Stores ModuleInfo behind wasm-types-polyfill and implements info() accordingly. |
| let Some(mut env_inner) = ctx.data_mut().try_inner_mut() else { | ||
| return Ok(()); | ||
| }; | ||
| let inner = env_inner.main_module_instance_handles_mut(); | ||
| inner.signal = funct; |
| if let Ok(mut map) = map.try_borrow_mut() { | ||
| map.remove(&id); | ||
| } |
| // segment runs. wasm-bindgen correctly marks raw JS handles as !Send, but the | ||
| // worker scheduler provides the stronger single-owner invariant required by | ||
| // `FunctionEnv`. | ||
| #[cfg(target_arch = "wasm32")] | ||
| unsafe impl Send for WasmCapiEnv {} |
Compact-unwind entries encode unsigned 32-bit offsets from the base returned by the dynamic-unwind callback. Using Wasmer's dylib base is invalid when JIT code, LSDA data, or the personality GOT slot is mapped below the dylib. This caused subtraction panics in debug builds and invalid libunwind pointers, resulting in SIGBUS during forced guest termination when Wasmer was loaded as a dynamic library. Derive the base from the actual JIT image addresses and validate that the complete address range and personality offset fit in u32.
Compact-unwind entries encode unsigned 32-bit offsets from the base returned by the dynamic-unwind callback. Using Wasmer's dylib base is invalid when JIT code, LSDA data, or the personality GOT slot is mapped below the dylib. This caused subtraction panics in debug builds and invalid libunwind pointers, resulting in SIGBUS during forced guest termination when Wasmer was loaded as a dynamic library. Derive the base from the actual JIT image addresses and validate that the complete address range and personality offset fit in u32.
marxin
left a comment
There was a problem hiding this comment.
We've got some JS CI job - do we know why the particular compilation errors haven't been caught?
| wasmer_vm::libcalls::wasmer_eh_personality as *const _, | ||
| &mut info as *mut _, | ||
| ); | ||
| let personality = self.maybe_eh_personality_addr_in_got.ok_or_else(|| { |
Because we are testing only a subset, and not the full amount of things needed to run. If we integrate the wasmer-js SDK codebase here, we would have seen it earlier: github.com/wasmerio/wasmer-js |
Compact-unwind entries encode unsigned 32-bit offsets from the base returned by the dynamic-unwind callback. Using Wasmer's dylib base is invalid when JIT code, LSDA data, or the personality GOT slot is mapped below the dylib. This caused subtraction panics in debug builds and invalid libunwind pointers, resulting in SIGBUS during forced guest termination when Wasmer was loaded as a dynamic library. Derive the base from the actual JIT image addresses and validate that the complete address range and personality offset fit in u32.
|
Based on the discussion with @syrusakbary, it's gonna be split into multiple PRs. |
… to simplify usage
Add an optional task-manager hook for terminating the executor that owns a specific WASM thread. This allows SIGKILL to stop browser workers directly, while falling back to waking atomic waiters for runtimes without targeted cancellation. Keep child-thread exits thread-local: mark killed threads as interrupted, prevent child threads from performing process-wide cleanup, publish the main thread result before broadcasting cleanup signals, and avoid tainting the runtime for non-zero child-thread exits.
This PR does many small fixes on Wasmer that allows running properly the SDK on main