Skip to content

fix: avoid crash on OOM by safe-guarding uninitialized VM pointer#40

Closed
jtakakura wants to merge 1 commit intoclojurewasm:mainfrom
jtakakura:fix/module-not-fully-loaded-error
Closed

fix: avoid crash on OOM by safe-guarding uninitialized VM pointer#40
jtakakura wants to merge 1 commit intoclojurewasm:mainfrom
jtakakura:fix/module-not-fully-loaded-error

Conversation

@jtakakura
Copy link
Copy Markdown
Contributor

This PR fixes a bug where, if allocator.create(Vm) fails due to OOM in WasmModule.loadLinked,
the returned module had an uninitialized vm field. This could cause undefined behavior
(e.g., crash in deinit) if the module was later cleaned up.
Now, vm is always set to null on OOM, and all accesses are guarded.
Additionally, invoke and related methods return error.ModuleNotFullyLoaded if vm is null.
All code and tests are updated to ensure safety in OOM scenarios.

Closes #39.

@jtakakura jtakakura force-pushed the fix/module-not-fully-loaded-error branch 2 times, most recently from ab2e216 to de0261d Compare April 23, 2026 14:16
@jtakakura jtakakura force-pushed the fix/module-not-fully-loaded-error branch from de0261d to 2380e42 Compare April 24, 2026 08:12
@jtakakura jtakakura force-pushed the fix/module-not-fully-loaded-error branch from 2380e42 to 6f0761f Compare April 24, 2026 08:46
chaploud added a commit that referenced this pull request Apr 24, 2026
Merge PR #40: OOM-safe VM pointer in loadLinked (jtakakura) + refinements
@chaploud
Copy link
Copy Markdown
Contributor

Hi @jtakakura — this was too useful to leave in draft with #41 (Zig 0.16.0 migration) coming up, so I picked your commit up and shipped it as v1.9.1 via #44. Your commit stays intact at the base of the feature branch so contributor credit is preserved.

What changed vs your version

  • Rebased onto v1.9.0 — combines your const vm = self.vm.?; extraction in loadCore with the config.cancellable override line that landed via Merge PR #28: Execution cancellation API (jtakakura) + refinements #43.
  • orelse idiominvoke() / invokeInterpreterOnly() now open with const vm = self.vm orelse return error.ModuleNotFullyLoaded;, flattening the bodies instead of nesting them inside an if (self.vm) |vm| { ... } block.
  • cancel() null guard — your nullable refactor exposed a latent segfault: calling WasmModule.cancel() on a partially-loaded module (OOM after Phase 1) would deref undefined. Added if (self.vm) |vm| vm.cancel();, matching the C API contract that already documents cancel as a no-op on idle modules.
  • CHANGELOG entry relocated to v1.9.1 with PR/issue credit; docs/api-boundary.md note on loadLinked kept as-is.

Verified

  • CI green (Ubuntu / macOS / Windows / size-matrix / benchmark).
  • Ubuntu x86_64 Merge Gate locally — unit 408/411, spec 62263/62263, e2e 796/796, realworld 50/50, FFI 80/80, minimal 268/283.
  • ClojureWasm downstream verification — test/run_all.sh --quick 4/4 pass against v1.9.1.

v1.9.1 is tagged and pushed. ClojureWasm bump PR: clojurewasm/ClojureWasm#4. Thanks for the careful fix — the FailingAllocator fail-index sweep to deterministically trigger the OOM path was a nice touch.

@chaploud
Copy link
Copy Markdown
Contributor

Shipped as v1.9.1 via #44 — see comment above for details. Your commit preserved at the base of that branch, contributor credit maintained.

@chaploud chaploud closed this Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WasmModule.loadLinked may return partially initialized module on OOM

2 participants