Merge PR #40: OOM-safe VM pointer in loadLinked (jtakakura) + refinements#44
Merged
Merge PR #40: OOM-safe VM pointer in loadLinked (jtakakura) + refinements#44
Conversation
Zig idiomatic pass on the null guards introduced for the `vm: ?*Vm` nullable
pointer. Replaces the two `if (self.vm) |vm| { ... } else return
error.ModuleNotFullyLoaded` blocks in `invoke`/`invokeInterpreterOnly` with
a single `const vm = self.vm orelse return error.ModuleNotFullyLoaded;` line,
flattening the body and matching how the rest of the codebase handles this
pattern.
Also adds a missed guard on `cancel()` — after the nullable refactor, calling
`self.vm.cancel()` on a partially-loaded module (OOM after Phase 1) would
segfault. The new `if (self.vm) |vm| vm.cancel();` matches the C API contract
that already documents cancel as a no-op on idle modules.
No behaviour change on the happy path; tests still 399/399.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #39. Based on @jtakakura's #40 (DRAFT) — his commit is preserved as the base of this branch for contributor credit.
Summary
WasmModule.vma nullable pointer (?*Vm). Ifallocator.create(Vm)fails inloadLinkedafter Phase 1 has already written into the shared store, the module stays alive (to keep the store consistent) withvm = nullandapply_error = error.OutOfMemory. Subsequentinvoke()/invokeInterpreterOnly()return the newerror.ModuleNotFullyLoaded.const vm = self.vm orelse return error.ModuleNotFullyLoaded;and adds the missed null guard toWasmModule.cancel()(segfault fix for partially-loaded modules, matches the already-documented C API contract).loadCore(combines @jtakakura'sconst vm = self.vm.?;extraction with thecancellableoverride line added in v1.9.0). CHANGELOG entry relocated under new[Unreleased]with PR/issue credit.Test plan
zig build test— 399/399 pass (one more than v1.9.0: the new OOM-path test)python3 test/spec/run_spec.py --build --summary— 62263/62263, 0 fail, 0 skipbash test/e2e/run_e2e.sh --convert --summary— 796/796, 0 failbash test/realworld/run_compat.sh— PASS=50, FAIL=0, CRASH=0bash test/c_api/run_ffi_test.sh --build— 80/80 pass