wasm: Add revamped tracked JavaScript values API

[bettio]

The existing JavaScript bridge (cast, call and the promise NIFs) only
passes integers and strings, so values such as DOM nodes or callback
functions could not be handled from Erlang at all. This API lets
Erlang hold opaque handles to arbitrary JavaScript values and ties
the JavaScript value lifetime to the Erlang handle lifetime.

emscripten:run_script_tracked/1 evaluates a script on the browser's
main thread; each element of the array it evaluates to is stored in a
JavaScript-side map under a fresh integer key, and the caller gets
one opaque handle (a NIF resource wrapping the key) per element. When
the VM garbage collects a handle, the resource destructor dispatches
a call to the main thread that drops the value from the map.
emscripten:get_tracked/2 maps handles back to their integer keys, or
fetches the current values as UTF-8 binaries (values must be
JavaScript strings; anything else is typically serialized to JSON
first).

The JavaScript side is a set of hooks on the emscripten module object
that embedders may override to customize evaluation, fetching and
cleanup without patching AtomVM; the defaults are the reference
implementation of the hook contract. On the VM side the trap answers
are built in a private heap and copied into a signal, so no foreign
thread ever touches a process heap.

The API was designed and first implemented by Jakub Gonet (Software
Mansion) for Popcorn, where it powers Elixir-to-JavaScript interop in
production. This PR revamps that work for upstream: rebased onto
release-0.7, review findings fixed, hardened against throwing hooks
and allocation failures, and completed with typespecs, edoc, prose
documentation and a browser test suite.

Supersedes: #2013
Closes: #2013

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

[bettio]

This PR supersedes #2013.
Here is what happened between that PR and this one, so you know what you are
reviewing.

Provenance and history shape. The five original commits are Jakub's
series as it exists in FissionVM (jgonet/js-api-popcorn), in
dependency-correct order so every commit builds.

I rebased them onto release-0.7 and
re-expressed the CMake changes on top of the current JIT_LINK_FLAGS
structure; the PR-era AVM_USE_WASM_MJS option is gone (ES6 output is
already unconditional on release-0.7). On top of that sit seven preparation
commits of mine (Erlang wrappers with specs and edoc, a browser test suite,
prose documentation and its pipeline registration, marker comments) and one
consolidated commit with all the review fixes.

Review comments from #2013. All of them are addressed:

• ref_keys leaks (@pguyot): fixed; the value path passes it as the
  dispatch satellite, the key path frees it on every exit.
• enif_make_resource deprecation (@pguyot): gone; handles are created
  with term_from_resource on a pre-sized private heap, so the
  abort-on-OOM path no longer exists.
• "post-increment is not atomic in C11" (@pguyot): it is; C11 6.5.2.4
  defines ++ on an atomic as an atomic read-modify-write, equivalent to
  atomic_fetch_add with seq_cst, so the counter stays as it was.
• The pre.js validation bug (@pguyot): confirmed, and worse than reported:
  no standard build defines NDEBUG, so the default hook threw on every
  successful array evaluation and the caller hung forever. The default hook
  is rewritten, and the EM_JS boundary now survives throwing hooks in
  general.
• The OUT_OF_MEMORY question (@pguyot) and the "how to raise?" TODO: out of
  memory while building an answer now raises error:out_of_memory in the
  caller through TrapExceptionSignal, the mechanism process_info
  answers already use, uniform with the get_tracked(_, key) path.
• Status enum, early returns instead of } else { // sender died },
  process_id naming: done.
• Erlang module functions, typespecs and documentation: done;
  emscripten:run_script_tracked/1 and emscripten:get_tracked/2 have
  specs and edoc, and the internals guide documents the flow, the lifetime
  model and the hook contract.
• -O3 hardcode: pre-existing, not introduced by this work; left alone,
  happy to revisit in a separate PR.
• cwrap/stringToNewUTF8 (@pguyot): dropped, nothing consumes them
  through the Module object. EMULATE_FUNCTION_POINTER_CASTS (@pguyot):
  removed; it rewrites indirect calls and table entries into
  uniform-signature thunks at link time, which traps at runtime with the
  wasm JIT, whose generated functions register with their real signatures.
  The JIT CI legs are green without it.
• ES6 as an option (@pguyot): moot, unconditional upstream since.
• Main-thread-only execution (@pguyot): kept; the values this API exists
  for (DOM nodes, callbacks) are only reachable from the main thread, and
  popcorn runs the VM in an iframe for exactly that reason. Relaxing it
  later would be an additive change.

New findings fixed on top of the review. A full pass over the whole
diff, with the crash paths checked against libAtomVM internals, found a few
things nobody had reported:

• Both dispatched calls built the trap answer on the trapped caller's heap
  from the browser main thread. The process-table read lock prevents
  destruction but not scheduling: any signal (a linked process dying,
  erlang:garbage_collect/1 from another process) lets a scheduler thread
  mutate the same heap concurrently. Answers are now built in a private
  heap and copied by mailbox_send_term_signal, following the
  process_info request pattern, so no foreign thread ever touches a
  process heap.
• A one-byte heap overflow in js_get_tracked_objects: stringToUTF8
  always writes a trailing NUL, and for the last string it landed one byte
  past the malloc'ed buffer, corrupting the next allocator chunk header on
  exact-fit sizes.
• Unchecked _malloc results in the EM_JS helpers (on failure they wrote
  through address zero, which is silent corruption on wasm) and no
  try/catch around the overridable hooks (a throwing override hung the
  caller forever). Both handled; hook throws map to the documented error
  returns.
• The default onRunTrackedJs turned sparse array holes into key 0,
  aliasing whatever real entry owns key 0 (wrong data on fetch, and the
  bogus handle's GC deleted the real entry).
• Key type unification (uint32_t end to end; keys above the small
  integer range produced wrong terms through the deprecated
  term_from_int32), and tracked_object() is typed reference()
  (handles are resource references, not binaries).

Compatibility with popcorn. Popcorn consumes this API in production, so
its exact usage (hooks it overrides, atoms it matches, exports it calls)
was mapped and preserved: NIF names and return shapes, hook names and
signatures, the post-init override mechanism, FS and the C exports (plus
a new additive _free). Two behavior changes are deliberate and safe for
popcorn: out-of-memory answers raise instead of returning a bare atom
(popcorn never matched those), and the two unused runtime method exports
are gone. Warts frozen by compatibility are marked with FIXMEs and
documented as unstable in the edoc: {error, badarg} conflates JavaScript
evaluation errors with argument errors, and a hook contract violation
yields a bare badvalue atom.

Note for Software Mansion: FissionVM swm ships two of the bugs fixed
here in production popcorn: the strings buffer overflow, and the hook throw
hang (popcorn's onGetTrackedObjects is JSON.stringify without a catch,
which throws on circular values). You probably want to pick at least those
two fixes.

Testing. The cypress suite (test_run_script_tracked) pins the whole
NIF contract in a real browser: valid and invalid inputs, the badarg
battery, badkey/badvalue paths, multibyte UTF-8 and refc-sized values,
garbage collection of dropped handles, and the misbehaving-hook liveness
cases.

Thanks @jgonet for the original design and implementation, and @pguyot for
the review of the original PR.

[pguyot]

I don't think FS is needed within this PR.

[bettio]

Fixed.

[pguyot]

I don't think FS is needed within this PR.

[bettio]

Fixed.

[pguyot]

This allocation looks wrong and I wonder if we should use TERM_BINARY_DATA_SIZE_IN_TERMS macro. It is too complex for the CodeQL queries, but the following additional tests crash:

test_many_large_values_round_trip() ->
    Strings = [lists:duplicate(50, C) || C <- lists:seq($a, $h)],
    {ok, Refs} = emscripten:run_script_tracked(tracked_script(Strings)),
    Expected = [{ok, list_to_binary(S)} || S <- Strings],
    Expected = emscripten:get_tracked(Refs, value),
    ok.

test_many_small_values_round_trip() ->
    Strings = [lists:duplicate(10, C) || C <- lists:seq($a, $j)],
    {ok, Refs} = emscripten:run_script_tracked(tracked_script(Strings)),
    Expected = [{ok, list_to_binary(S)} || S <- Strings],
    Expected = emscripten:get_tracked(Refs, value),
    ok.

tracked_script(Strings) ->
    Quoted = [[$', S, $'] || S <- Strings],
    ["[", lists:join($,, Quoted), "]"].

[bettio]

Fixed.

[pguyot]

We need to clean up what js_tracked_eval inserted.

[bettio]

Fixed.

[pguyot]

Not a fan of defining get_tracked_result() here, but if we do, we should export it.

[bettio]

Fixed. I believe it is more readable this way.

[pguyot]

malloc(0) could return NULL. Tests test_empty_array_yields_empty_list/0 and test_null_yields_empty_list/0 don't fail because emscripten's malloc(0) doesn't return NULL.

[bettio]

Fixed.

[petermm]

AMP, usual caveats:

PR review: tracked JavaScript values API

Range reviewed: HEAD~21..HEAD (96aa573287884ddefe075e9dc06f6478d450344c through 54f63f7d0392392594a347d0e0fb83e4a3e8ff0b)
Verdict: Changes requested — the private heap used by get_tracked/2 can be under-allocated in normal use, causing an out-of-bounds write in release builds. The OOM cleanup and delivery paths also contain leaks/crash paths that conflict with the API's documented recoverable out_of_memory behavior.

The final state of the 21 commits was reviewed as one change set; bugs fixed by later commits in the range were not reported. The review covered the JavaScript/C boundary, resource ownership, trap delivery, heap sizing, key identity, public Erlang contracts, and browser tests. Oracle was also asked to independently stress-test the final diff; its findings were verified against the cited implementation before inclusion below.

Findings

1. [Critical] Size each returned binary separately or get_tracked/2 writes past its private heap

do_get_tracked_objects allocates binary storage as though all returned string bytes belonged to one binary:

LIST_SIZE(strings_n, BINARY_HEADER_SIZE)
    + term_binary_data_size_in_terms(all_byte_size)

The loop then calls term_create_uninitialized_binary separately for every successful value. On wasm32, every string of at least 32 bytes is a ref-counted binary and therefore consumes TERM_BOXED_REFC_BINARY_SIZE (6) heap words regardless of its byte length. Combining all bytes into one term_binary_data_size_in_terms call accounts for only one ref-counted-binary body.

For four 80-byte strings, the result needs 44 words: 20 for four cons cells and four 2-tuples, plus 24 for four ref-counted binaries. The current expression allocates only 42 words: 20 + 16 + 6. memory_heap_alloc checks the heap boundary only under DEBUG_HEAP_ALLOC, so a release build writes beyond the heap fragment. More returned large strings increase the overwrite.

The current test at test_run_script_tracked.erl:104 fetches only one large value and cannot expose the bug.

Suggested fix: sum term_binary_heap_size(sizes[i]) for each TRACKED_OBJECT_OK entry and reject arithmetic overflow before allocating the heap.

diff --git a/src/platforms/emscripten/src/lib/platform_nifs.c b/src/platforms/emscripten/src/lib/platform_nifs.c
--- a/src/platforms/emscripten/src/lib/platform_nifs.c
+++ b/src/platforms/emscripten/src/lib/platform_nifs.c
@@ -409,8 +409,21 @@ static void do_get_tracked_objects(uint32_t *ref_keys, size_t keys_n, int32_t pr
     }
 
     Heap heap;
-    size_t heap_size = LIST_SIZE(objects_n, TUPLE_SIZE(2)) + LIST_SIZE(strings_n, BINARY_HEADER_SIZE) + term_binary_data_size_in_terms(all_byte_size);
-    if (UNLIKELY(memory_init_heap(&heap, heap_size) != MEMORY_GC_OK)) {
+    const size_t result_terms = TUPLE_SIZE(2) + CONS_SIZE;
+    bool heap_size_overflow = objects_n > SIZE_MAX / result_terms;
+    size_t heap_size = heap_size_overflow ? 0 : objects_n * result_terms;
+    for (size_t i = 0; !heap_size_overflow && i < objects_n; ++i) {
+        if (statuses[i] == TRACKED_OBJECT_OK) {
+            size_t binary_terms = term_binary_heap_size(sizes[i]);
+            if (binary_terms > SIZE_MAX - heap_size) {
+                heap_size_overflow = true;
+            } else {
+                heap_size += binary_terms;
+            }
+        }
+    }
+    if (UNLIKELY(heap_size_overflow || memory_init_heap(&heap, heap_size) != MEMORY_GC_OK)) {
         free(sizes);
         free(statuses);
         free(strings);

Add a browser regression that fetches at least four 80-byte strings in one call.

---

2. [High] OOM while creating handles permanently retains unowned JavaScript values

The default onRunTrackedJs inserts every evaluated value into trackedObjectsMap before the native side creates any resource handle. Several subsequent allocation failures do not delete entries that never acquired a resource:

1. If the key-array _malloc fails at platform_nifs.c:219, all values have already been inserted, but JavaScript returns 0 without deleting them. This path is also misreported as {error, badarg} rather than out_of_memory.
2. If private-heap allocation fails at platform_nifs.c:241, no resources exist, so freeing keys leaves every JavaScript value retained.
3. If resource allocation fails partway through the reverse loop at platform_nifs.c:257, destroying the heap deletes only keys whose resources were already created (i + 1 .. keys_n - 1). Keys 0 .. i remain in JavaScript forever.

This is self-amplifying under memory pressure: a failed call can retain a large graph of DOM objects, making later allocations still more likely to fail.

Suggested fix: once onRunTrackedJs returns keys, make ownership explicit on every exit. On JS _malloc failure, invoke Module.onTrackedObjectDelete for all returned keys and report an OOM sentinel distinct from evaluation failure. On private-heap failure, delete all keys. On partial resource failure at index i, delete keys 0 .. i; destroying the private heap will release the remainder. Deletion-hook exceptions should be caught and logged as they are in the resource destructor.

This spans the JS result protocol and partial C construction, so a small isolated diff would be misleading. Add fault-injection coverage that forces _malloc or resource allocation to fail after tracking and asserts that trackedObjectsMap.size returns to its baseline.

---

3. [High] A mailbox-copy allocation failure dereferences NULL

send_tracked_trap_answer calls mailbox_send_term_signal for every successful asynchronous result. mailbox_message_create_from_term can return NULL when its allocation fails, but mailbox_send_term_signal passes that pointer directly to mailbox_post_message, whose enqueue path dereferences it.

This PR makes that pre-existing unsafe helper reachable with a newly constructed result proportional to the total fetched strings: get_tracked/2 can successfully allocate its private result heap and then exhaust memory while allocating the mailbox copy. Instead of the documented out_of_memory exception, AtomVM crashes on the browser main-thread callback.

Smallest PR-local fix: create the signal explicitly and use the existing immediate OOM trap on failure. (A follow-up can make the generic mailbox helper return a status and migrate all callers.)

diff --git a/src/platforms/emscripten/src/lib/platform_nifs.c b/src/platforms/emscripten/src/lib/platform_nifs.c
--- a/src/platforms/emscripten/src/lib/platform_nifs.c
+++ b/src/platforms/emscripten/src/lib/platform_nifs.c
@@ -184,8 +184,13 @@ static void send_tracked_trap_answer(GlobalContext *global, int32_t process_id,
 {
     Context *target_ctx = globalcontext_get_process_lock(global, process_id);
     if (target_ctx) {
-        mailbox_send_term_signal(target_ctx, TrapAnswerSignal, answer);
+        MailboxMessage *signal
+            = mailbox_message_create_from_term(TrapAnswerSignal, answer);
+        if (LIKELY(signal)) {
+            mailbox_post_message(target_ctx, signal);
+        } else {
+            mailbox_send_immediate_signal(target_ctx, TrapExceptionSignal, OUT_OF_MEMORY_ATOM);
+        }
         globalcontext_get_process_unlock(global, target_ctx);
     } // else: sender died, nobody is waiting
 }

The immediate-signal allocation can itself fail under total allocator exhaustion, but this removes the deterministic null dereference and matches the failure strategy already used by send_tracked_trap_oom.

---

4. [Medium] Validate custom-hook keys before narrowing them to uint32_t

The public contract says an overridden onRunTrackedJs returns “an array of integer keys” (emscripten.erl:339, atomvm-internals.md:500), but the native resource and ABI store only uint32_t. HEAPU32.set(keys, ...) silently converts negative, fractional, NaN, and greater-than-2^32 - 1 numbers. Some other values (for example a Symbol) throw from HEAPU32.set outside the hook try/catch, leaving the trapped Erlang caller waiting forever.

For example, a custom hook that stores and returns key 4294967296 creates a resource for key 0. Fetching then addresses the wrong entry, and destruction deletes key 0 while the original entry remains retained.

Suggested fix: define keys as unsigned 32-bit integers and reject malformed hook output before allocation/copying.

diff --git a/src/platforms/emscripten/src/lib/platform_nifs.c b/src/platforms/emscripten/src/lib/platform_nifs.c
--- a/src/platforms/emscripten/src/lib/platform_nifs.c
+++ b/src/platforms/emscripten/src/lib/platform_nifs.c
@@ -210,8 +210,13 @@ EM_JS(uint32_t *, js_tracked_eval, (const char *code, uint32_t *size, bool debug
         keys = null;
     }
-    // the hook must return an array of keys or null
-    if (keys === null || !Array.isArray(keys)) {
+    // the hook must return an array of uint32 keys or null
+    if (keys === null || !Array.isArray(keys)
+        || !keys.every((key) => Number.isInteger(key)
+            && key >= 0
+            && key <= 0xffffffff)) {
         HEAPU32[size / HEAPU32.BYTES_PER_ELEMENT] = 0;
         return 0;
     }

Update both API documents from “integer” to “unsigned 32-bit integer” and add custom-hook tests for a fractional key, a negative key, and 2 ** 32.

---

5. [Medium] The “fresh” default key aliases a live handle after counter wrap

The default key source is a wasm32 atomic_size_t, incremented without exhaustion handling in sys_get_next_tracked_object_key. nextTrackedObjectKey then explicitly narrows the signed ccall result back to 32 bits. After 2^32 tracked values, the counter returns 0 again even if the original key-0 handle is still alive.

Map.set(0, newValue) then overwrites the old value, and garbage collection of either handle invokes delete(0), invalidating the other. At 1,000 tracked values per second, wrap occurs in roughly 50 days, which is reachable for a persistent browser VM.

Suggested fix: reserve a key as an exhaustion sentinel and use a saturating atomic compare/exchange; propagate exhaustion as an error instead of reusing an identity. A 64-bit key ABI is a larger alternative. Whichever approach is chosen, document the actual key range as part of the hook contract.

Regression coverage requested

• Fetch four or more 80-byte strings in a single get_tracked/2 call (finding 1).
• Fault-inject allocation failure after the default hook has inserted values and assert no map growth (finding 2).
• Override onRunTrackedJs with malformed/out-of-range keys and assert deterministic {error, badarg} rather than truncation or a hang (finding 4).
• Exercise key exhaustion through a test-only counter seam rather than iterating 2^32 times (finding 5).

Review notes

• git diff --check HEAD~21..HEAD passes.
• The private-heap strategy correctly avoids mutating the trapped caller's heap from the main thread.
• Successful mailbox copying correctly retains resource references before the private heap is destroyed.
• UTF-8 concatenation/reconstruction and result ordering are correct for representable buffer sizes.
• No browser runtime suite was executed as part of this static review; the critical heap-size finding follows directly from the allocator formulas and release-only boundary-check behavior.

[bettio]

AMP, usual caveats:

PR review: tracked JavaScript values API

Fixed all the reported issues.

[petermm]

by now I'm just the messsenger;-)

Follow-up review: tracked JavaScript values fixes

Follow-up range: 54f63f7d0392392594a347d0e0fb83e4a3e8ff0b..1078b72cd18ede37e46f2ae6b451c44d36958391 (7 commits)
Verdict: Changes requested — the critical heap overwrite and mailbox null dereference are fixed, but malformed hook output and key exhaustion can still retain unowned JavaScript values, and a throwing getter in a get-hook result can still leave an Erlang caller trapped forever.

The follow-up diff and the resulting final PR state were reviewed. Oracle independently reviewed the same range and the five findings in PR_REVIEW.md; its claims were checked against the current source before inclusion here.

Disposition of the original findings

Original finding	Status	Verification
1. Per-binary private-heap under-allocation	Resolved	do_get_tracked_objects now adds term_binary_heap_size(sizes[i]) for each successful value. The new many-large-values browser test enters the wasm32 ref-counted-binary branch and would expose the original overwrite.
2. Unowned values leak on handle-construction OOM	Partially resolved	JS key-array OOM, private-heap OOM, and partial resource construction now roll back the keys they own. Invalid returned keys and the exhaustion sentinel are still omitted from rollback; see finding 1. Native allocation failure paths also remain untested.
3. Mailbox-copy OOM dereferences NULL	Resolved	send_tracked_trap_answer now checks mailbox_message_create_from_term and falls back to an immediate out_of_memory trap exception.
4. Hook keys are narrowed without validation	Resolved for numeric range	Keys are validated as integers in 0..4294967294 before HEAPU32.set; negative, fractional, overflow, and reserved values have browser tests. Ownership/uniqueness remains underspecified; see finding 3.
5. Key counter wraps into live identities	Partially resolved	The native counter now saturates atomically at UINT32_MAX, so it cannot alias an ordinary key. The default hook stores under the exhaustion sentinel before validation rejects it, and rollback skips that sentinel; see finding 1.

Remaining findings

1. [High] Roll back invalid keys too, or exhaustion and malformed hooks still retain values

js_tracked_eval detects malformed keys but calls:

dropKeys(keys.filter(isKey));

That deletes only valid siblings. Any value stored under the invalid key remains owned by neither JavaScript nor an Erlang resource.

This also breaks the new exhaustion path. The default trackValue unconditionally executes trackedObjectsMap.set(key, value). Once the saturated counter returns 4294967295, the default hook stores the evaluated value under that sentinel; native validation rejects it, then filter(isKey) removes it from cleanup. The call returns {error, badarg} while retaining the value indefinitely. Repeated exhausted calls overwrite the same entry, so the leak is one map entry but that entry can retain an arbitrarily large object graph.

The new invalid-key test does not catch this: install_invalid_key_hook stores only under the valid sibling key and records that valid key in window.invalidHookKey; it never stores under the invalid key being tested.

Every key returned by the hook is an ownership claim for this attempted call. If validation fails before resources exist, cleanup should invoke the deletion hook for every returned key, including malformed values. Exceptions are already caught by dropKeys.

Smallest fix:

diff --git a/src/platforms/emscripten/src/lib/platform_nifs.c b/src/platforms/emscripten/src/lib/platform_nifs.c
--- a/src/platforms/emscripten/src/lib/platform_nifs.c
+++ b/src/platforms/emscripten/src/lib/platform_nifs.c
@@ -263,7 +263,7 @@ EM_JS(uint32_t *, js_tracked_eval, (const char *code, uint32_t *size, uint8_t *s
     const isKey = (key) => Number.isInteger(key) && key >= 0 && key <= 0xfffffffe;
     if (!keys.every(isKey)) {
         console.error("onRunTrackedJs returned invalid keys", keys);
-        dropKeys(keys.filter(isKey));
+        dropKeys(keys);
         setOutcome(ERROR, 0);
         return 0;
     }

Update the test hook to store values under both the valid sibling and the actual invalid key, then assert both are absent. Add an exhaustion regression by temporarily overriding only Module.nextTrackedObjectKey to return 0xffffffff, leaving the default onRunTrackedJs installed, and asserting the sentinel is absent after {error, badarg}.

---

2. [High] Copy get-hook output inside the try or a getter can trap the caller forever

js_get_tracked_objects catches exceptions from the direct Module.onGetTrackedObjects(keys) call, but it reads objects.length and each objects[i] after leaving the try block. A hook can return an actual array containing an accessor that throws:

const result = new Array(keys.length);
Object.defineProperty(result, 0, {
  get() { throw new Error("getter boom"); },
});
return result;

The hook itself returns normally. Reading objects[0] then throws out of the main-thread callback, no trap answer is sent, and emscripten:get_tracked(Refs, value) waits indefinitely. A revoked proxy can similarly throw while Array.isArray examines the result.

js_tracked_eval already addresses the equivalent problem by using Array.from inside its hook boundary. Apply the same pattern here:

diff --git a/src/platforms/emscripten/src/lib/platform_nifs.c b/src/platforms/emscripten/src/lib/platform_nifs.c
--- a/src/platforms/emscripten/src/lib/platform_nifs.c
+++ b/src/platforms/emscripten/src/lib/platform_nifs.c
@@ -396,16 +396,17 @@ EM_JS(char *, js_get_tracked_objects, (uint32_t *keys_ptr, uint32_t keys_n, uint
 
     let objects;
     try {
-        objects = Module['onGetTrackedObjects'](keys);
+        const result = Module['onGetTrackedObjects'](keys);
+        objects = Array.isArray(result) ? Array.from(result) : null;
     } catch (e) {
         // the hook contract forbids throwing; swallowing the throw here keeps
         // the trapped caller from waiting forever
         console.error("onGetTrackedObjects threw", e);
         objects = null;
     }
     // the hook must return exactly one entry per requested key
-    if (!Array.isArray(objects) || objects.length !== keys_n) {
+    if (objects === null || objects.length !== keys_n) {
         return 0;
     }

Add a browser test using a getter-throwing array. It should promptly return the existing whole-call badvalue, after which restoring the hook and fetching the original handle should still succeed.

---

3. [Medium] Require unique, exclusively owned keys from custom run hooks

Current validation checks only the numeric range. A hook that stores one value and returns [key, key] therefore receives two independently destructible Erlang resources for the same map entry. Garbage collecting either resource invokes onTrackedObjectDelete(key), invalidating the still-live second handle. Returning a key already owned by a handle from an earlier call has the same problem; rollback after an allocation failure can even delete the older handle's value.

The public contract currently permits this: emscripten.erl and the internals hook table require only an array of keys in range. The implementation's destructor protocol actually requires each returned key to be unique and exclusively transferred to this call until onTrackedObjectDelete runs.

Suggested fix: reject duplicates within one returned array, and document cross-call reuse as a hook-contract violation. A native live-key registry would add substantial synchronization and rollback bookkeeping merely to defend a trusted customization hook.

The validation should construct a Set inside an exception boundary, then require set.size === keys.length as well as the existing range check. On failure, use the all-key rollback from finding 1. Add a test returning [key, key] and assert {error, badarg} plus removal of the map entry.

Document the ownership transfer explicitly:

diff --git a/libs/avm_emscripten/src/emscripten.erl b/libs/avm_emscripten/src/emscripten.erl
--- a/libs/avm_emscripten/src/emscripten.erl
+++ b/libs/avm_emscripten/src/emscripten.erl
@@ -342,8 +342,12 @@
 %% factory configuration are overwritten by the defaults). An overridden
 %% `Module.onRunTrackedJs' must return an array of unsigned 32 bit integer
 %% keys (0 to 4294967294, the last value of the range being reserved), or
-%% `null' on error, and must not throw; a throw, or a key outside that
-%% range, is treated as an evaluation error and yields `{error, badarg}'.
+%% `null' on error. Returned keys must be unique, newly acquired for this
+%% call, and not associated with an existing live handle. Returning a key
+%% transfers exclusive ownership to the VM until
+%% `Module.onTrackedObjectDelete' is called. The hook must not throw; a
+%% throw, duplicate key, or key outside the range is treated as an
+%% evaluation error and yields `{error, badarg}'.

Mirror this requirement in atomvm-internals.md, including that failure rollback may call onTrackedObjectDelete before run_script_tracked/1 returns.

---

4. [Medium] Distinguish get-result allocation failure from a malformed get hook

The three JavaScript allocations in js_get_tracked_objects—sizes, statuses, and concatenated strings—return 0 on OOM. do_get_tracked_objects cannot distinguish those failures from a throwing or malformed hook and sends the bare atom badvalue.

This contradicts the exported API contract, which says get_tracked/2 raises out_of_memory when the VM cannot build the result. A conforming hook can therefore receive badvalue solely because _malloc failed.

Suggested fix: add a TrackedGetStatus out parameter analogous to TrackedEvalStatus: OK for a complete result, ERROR for malformed/throwing hook output, and OOM for any allocation failure. Route OOM to send_tracked_trap_oom and retain the current whole-call badvalue for ERROR.

This protocol touches each JS allocation and the C dispatch result, so a tiny partial diff would be misleading. Add fault-injection tests that temporarily wrap Module._malloc, fail each allocation point, assert an out_of_memory exception, and restore the allocator. The same seam should verify key-array OOM cleanup in js_tracked_eval; a native allocation seam is still needed to exercise partial resource rollback.

Verification performed

• git diff --check 54f63f7d0392392594a347d0e0fb83e4a3e8ff0b..HEAD passes.
• A clean release web build with Emscripten 5.0.7 completed successfully with AVM_DISABLE_JIT=ON, including the final AtomVM.mjs link.
• The emscripten_erlang_test_modules target compiled all Emscripten test modules, including the revised test_run_script_tracked.erl.
• The browser/Cypress suite was not run, so the behavioral findings above are source-verified rather than reproduced in a browser.
• Oracle suggested treating removal of the transient FS runtime export as a compatibility regression. That was not retained as a finding: compared with the pre-PR base (2903b59b), final HEAD restores the original runtime-method export list rather than removing an established API.

Merge recommendation

Do not merge yet. The two original memory-safety/crash blockers are fixed, but findings 1 and 2 still violate core promises of the tracked API: failed calls can retain values, and a custom hook can leave the caller trapped forever. Findings 3 and 4 should be addressed while the public hook and error contracts are still new.

[petermm]

Second follow-up review: tracked JavaScript values fixes

Remote range reviewed: 1078b72cd18ede37e46f2ae6b451c44d36958391..449ff9301c7c69ded84e00e2c8e4d29ed9eba321 (4 commits)
Remote ref: github-desktop-bettio/updated-emscripten-api
Verdict: Changes requested — all four findings from FOLLOWUP.md have substantive fixes, but duplicate-key validation introduces one new trapped-caller/leak path, and duplicate rollback invokes the deletion hook more than once for a single owned identity.

The contributor's remote branch was fetched and reviewed without changing the local pr/2359 branch. Oracle independently reviewed the same range; its findings below were verified against remote tip 449ff9301.

Disposition of FOLLOWUP.md

Finding	Status	Verification
1. Invalid/exhausted keys omitted from rollback	Resolved	Validation now passes every returned key to dropKeys. Tests store under both valid and malformed keys, and the exhaustion test uses the default run hook and verifies that the sentinel entry is absent.
2. Throwing getter in get-hook output traps the caller	Resolved	The hook result is copied with Array.from inside the exception guard. The new getter test expects the whole-call badvalue, restores the hook, and successfully fetches the original handle afterward.
3. Duplicate/reused key ownership is not enforced or documented	Partially resolved	Duplicate keys within one result are rejected, and cross-call exclusive ownership is documented. The validation itself can still throw outside the guard, and rollback calls deletion once per occurrence rather than once per identity; see findings 1 and 2.
4. Get-side allocation OOM is reported as badvalue	Resolved in implementation; untested	Every explicit js_get_tracked_objects failure initializes the new call status. Allocation failures route to out_of_memory; malformed hooks retain the existing badvalue path. No allocator fault-injection test exercises the distinction.

Findings

1. [High] Guard duplicate validation or evaluated JavaScript can trap the caller and leak its values

After onRunTrackedJs returns, duplicate validation executes outside the surrounding try/catch:

const hasDuplicates =
    keys.length > 1 && new Set(keys).size !== keys.length;

Set is a mutable JavaScript global and construction consumes the array iterator. Either operation can throw. This is reachable through the default hook, not only a deliberately broken override: the evaluated script runs before this line and can replace globalThis.Set, then return ordinary values:

globalThis.Set = class {
  constructor() {
    throw new Error("boom");
  }
};
["owned"];

The default hook stores "owned" and returns its key. The replacement Set constructor then throws outside the guard, so no trap answer is sent and no rollback runs. The Erlang caller waits indefinitely while the JavaScript value remains retained without a resource handle. A catchable allocation failure during Set construction has the same control flow. keys.every(isKey) is also outside the guard and resolves a mutable prototype method.

Suggested fix: perform validation inside an exception boundary, and make rollback independent of array iterators. The following combines that with the distinct-key cleanup required by finding 2:

diff --git a/src/platforms/emscripten/src/lib/platform_nifs.c b/src/platforms/emscripten/src/lib/platform_nifs.c
--- a/src/platforms/emscripten/src/lib/platform_nifs.c
+++ b/src/platforms/emscripten/src/lib/platform_nifs.c
@@ -230,14 +230,29 @@ EM_JS(uint32_t *, js_tracked_eval, (const char *code, uint32_t *size, uint8_t *s
     // the hook stored these values but no resource owns them yet: drop them
     // the way the resource destructor does, or they stay tracked forever
     const dropKeys = (keys) => {
-        for (const key of keys) {
+        for (let i = 0; i < keys.length; ++i) {
+            const key = keys[i];
+            let seen = false;
+            for (let j = 0; j < i; ++j) {
+                const previous = keys[j];
+                // SameValueZero, matching JavaScript Map key identity.
+                if (previous === key
+                        || (previous !== previous && key !== key)) {
+                    seen = true;
+                    break;
+                }
+            }
+            if (seen) {
+                continue;
+            }
             try {
                 Module['onTrackedObjectDelete'](key);
             } catch (e) {
-                console.error("onTrackedObjectDelete threw", e);
+                try {
+                    console.error("onTrackedObjectDelete threw", e);
+                } catch (_) {
+                    // Cleanup must not escape the dispatched callback.
+                }
             }
         }
     };
@@ -262,11 +277,21 @@ EM_JS(uint32_t *, js_tracked_eval, (const char *code, uint32_t *size, uint8_t *s
     // would coerce anything else silently (-1 to 4294967295, 2 ** 32 to 0,
     // a fraction to its truncation) and alias an unrelated entry
     const isKey = (key) => Number.isInteger(key) && key >= 0 && key <= 0xfffffffe;
-    // a repeated key would get one handle each, and the first of them
-    // collected deletes the value the others still address; only an array of
-    // two or more keys can repeat one, so a single value result builds no set
-    const hasDuplicates = keys.length > 1 && new Set(keys).size !== keys.length;
-    if (!keys.every(isKey) || hasDuplicates) {
+    let hasInvalidKeys;
+    let hasDuplicates;
+    try {
+        hasInvalidKeys = !keys.every(isKey);
+        hasDuplicates
+            = keys.length > 1 && new Set(keys).size !== keys.length;
+    } catch (e) {
+        dropKeys(keys);
+        setOutcome(ERROR, 0);
+        return 0;
+    }
+    if (hasInvalidKeys || hasDuplicates) {
         console.error("onRunTrackedJs returned invalid keys", keys);
         // every returned key is an ownership claim, malformed ones included:
         // the hook may have stored a value under them too

This is the smallest control-flow correction. A stronger implementation can avoid mutable Set/Array.prototype.every entirely by validating and checking duplicates with indexed loops; that trades the temporary set allocation for quadratic duplicate checking.

Add a browser regression that replaces Set during the evaluated script, verifies that run_script_tracked/1 returns promptly, and verifies that the newly tracked value was deleted. Restore Set before making the assertion call.

---

2. [Medium] Roll back each distinct key once, not once per duplicate occurrence

When a hook returns [Key, Key], duplicate validation correctly rejects the result, but dropKeys(keys) invokes Module.onTrackedObjectDelete(Key) twice. The documented model transfers ownership of a key identity, not ownership of every occurrence in a malformed array. Since no resources were created, rollback should release that identity exactly once.

The default Map.delete hook is idempotent, so the current duplicate test passes. A valid custom deletion hook need not be idempotent: it may decrement a reference count, release an external object, or perform application-side cleanup. Calling it twice can therefore double-release state even if a second thrown exception is caught.

The indexed, SameValueZero-aware dropKeys diff in finding 1 fixes this without relying on Set, a mutable iterator, or native-key coercion. Update the internals wording from “every returned key is deleted” to “each distinct returned key is deleted once.”

Add a browser test that installs a counting onTrackedObjectDelete, returns [Key, Key], and asserts exactly one deletion callback before restoring the hook.

Verification performed

• git diff --check 1078b72cd18ede37e46f2ae6b451c44d36958391..449ff9301c7c69ded84e00e2c8e4d29ed9eba321 passes.
• A clean release web build of exact remote tip 449ff9301 completed with Emscripten 5.0.7 and AVM_DISABLE_JIT=ON.
• AtomVM.mjs linked successfully.
• The emscripten_erlang_test_modules target compiled all Emscripten test modules, including the revised test_run_script_tracked.erl.
• The explicit js_get_tracked_objects status/cleanup paths were audited: malformed results set CALL_ERROR; all three allocation failures and the unrepresentable aggregate size set CALL_OOM; success sets CALL_OK after publishing outputs.
• Browser/Cypress tests and allocation fault injection were not run.

Merge recommendation

Do not merge remote tip 449ff9301 yet. Guard duplicate validation and make rollback operate once per distinct key. Both changes are localized; focused browser coverage should be added for the escaped-validation exception and deletion callback count.

[petermm]

btw I believe FS is used by popcorn - so maybe keep it around, seems unrelated to PR either way.

[bettio]

btw I believe FS is used by popcorn - so maybe keep it around, seems unrelated to PR either way.

I will make a PR just for it