Skip to content

fix(napi): bound guest copies and retain queued message ownership - #95

Closed
theduke wants to merge 1 commit into
split/napi-02-managed-frameworkfrom
split/napi-03-guest-accounting
Closed

theduke wants to merge 1 commit into
split/napi-02-managed-frameworkfrom
split/napi-03-guest-accounting

Conversation

@theduke

@theduke theduke commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Guest-supplied lengths, strings, property descriptors and serialized values could allocate retained host memory beyond the tenant's WASM memory budget. Serialized worker messages could outlive their producer's store while retaining pointers into its shared mapping.

Use the existing JavaScript memory-handle conversion for host-JS guest-size checks, preserving main’s fast memory path; this also corrects a wasm32 type error found by CI during the split.

Reserve scratch memory before host copies and compiler/property/module work, validate lengths and output addresses before allocation, handle embedded NUL names without reading beyond a fallback string, and return reservations after the actual native operation unwinds. Keep transferred external-buffer ownership correct on callback errors.

Bound structured clone work and keep each pending message's native payload, budget charge, guest heap and shared-memory mapping alive until delivery/drop. Check producing-memory identity and remove a message atomically, keep charges through native take and callback unwinding, and close the registry on permanent stop. The nonlegacy registry has one owner per payload; repeatable legacy reads and their extra shared-lease machinery are excluded.

The released EdgeJS 0.2.5 atom was inspected directly: it imports 67 napi_extension_wasmer_v0 symbols and 110 standard napi symbols, all already registered by main, with zero old napi.unofficial_* imports. The approximately 1,266-line 0.0.1 adapter, fixtures and legacy-only bridge hooks are deliberately omitted. The two native tests for the private short-call legacy dynamic-import adapter are omitted with that adapter; current API module graph, callback and bound checks remain. The actual released 0.2.5 atom also boots this provider and prints its runtime version successfully without legacy adapters. A compile-only list still mentions 0.0.1; that does not require executing its old imports. This stack therefore does not add compatibility for that older frozen ABI.

Independent review checked all skipped compatibility commits for incidental nonlegacy fixes. Output validation, native module teardown, overflow-safe payload bounds, bad-allocation cleanup and publication rollback were retained.

Stack and provenance

#93 → #94 → #95 → #96 → #97 → #98 → #99 → #100 → #101

Part 3 of the audited split of #84, reconstructed on current main eac0a13fa65a4d6103864d875ed76ab7efb1816e. Review against split/napi-02-managed-framework; predecessor branches are required. Original PR #84 remains unchanged. The stack preserves newer main host-JS behavior and deliberately excludes the unused frozen 0.0.1 ABI adapter.

Validation

guest_boundary_accounting, guest_error_callbacks, core_callbacks, review_memory, external_transfer, message_memory, and host-control handle/string/descriptor regressions.

Completed on the intended Edge SDK eaed67d: 6 guest-boundary, 1 guest-error, 3 message-memory, 6 memory-safety and 1 external-transfer tests passed. The unchanged finite-budget CLI message regression also passed against that SDK. The standalone public SDK pin has an older shared-memory detachment API and deliberately refuses finite-budget shared detachment to preserve quota ownership; its corresponding CLI case fails for that compatibility limitation. No forwarding workaround was introduced. Further local tests were stopped at the user’s request; remaining execution belongs to CI.

Latest-main rebase

Rebased the dependency stack onto eac0a13fa65a4d6103864d875ed76ab7efb1816e after main merged PR #92. This preserves unsigned guest IDs, pointers and lengths, bounded result/argv writes, host-JS buffer failure cleanup and the high-pointer/high-ID regressions. Conflicts received independent native and host-JS source review. All historical local results above predate this rebase; no local tests or builds were run for it, as requested. Updated-head validation belongs to CI and may be pending. This PR keeps its prerequisite branch as its base to preserve the focused diff.

@theduke
theduke force-pushed the split/napi-03-guest-accounting branch from 4730716 to 1add813 Compare October 22, 2026 08:20
@theduke
theduke added this pull request to stack #102 October 22, 2026 08:24
@theduke
theduke marked this pull request as ready for review October 22, 2026 08:26
@theduke theduke closed this Oct 23, 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.

1 participant