ReviewOS

zig-utils/zig-js

Threads: close residual Object.elements side doors

#17
Closed chris opened this 9 days ago ·
9 days ago

Parent: #12, #1

Goal

Audit and migrate remaining direct Object.elements storage reads/writes that can become reachable from shared-realm no-GIL JavaScript, so element-backed arrays, Maps/Sets, iterator helper state, and engine tuples consistently go through Object.elements_lock or a narrower documented synchronization path.

Why this deserves its own tracker

The memory-model docs already say residual direct element side doors are tracked, but the current codebase still has many direct elements.items / elements.append / clear/shrink sites across builtins.zig, interpreter.zig, structured_clone.zig, vm.zig, C API helpers, and tests. Some are private construction or single-owner paths; others may be reachable from shared-realm parallel JS. That classification is too broad for drive-by edits and should be made explicit.

Evidence from the current audit:

rg -n "\.elements\.items|\.elements\.append|\.elements\.clear|\.elements\.deinit|\.elements\.orderedRemove|\.elements\.swapRemove|\.elements\.shrink|\.elements\.insert|\.elements\.replaceRange" src -g'*.zig'

The audit includes direct users in:

  • src/builtins.zig
  • src/interpreter.zig
  • src/structured_clone.zig
  • src/vm.zig
  • src/c_api.zig
  • src/context.zig test/witness code

Acceptance criteria

  • Classify each direct Object.elements access as one of:
    • private construction before publication,
    • single-owner host/C API path,
    • GC/finalization trace/finalize path with its own stop-the-world or lock rule,
    • shared-realm reachable and requiring Object.elements_lock,
    • test-only witness that should be rewritten or documented.
  • Migrate shared-realm reachable reads/writes to existing helpers (elementAt, setElementAt, appendElement, dense helpers, snapshots) or add a narrower helper with documented lock scope.
  • Never hold Object.elements_lock across JS callbacks.
  • Add focused no-GIL/TSan witnesses for any migrated public path.
  • Update docs/threads/bindings.md after the residual side-door list shrinks, so #12 stays honest.

Notes

This is not claiming all direct uses are bugs. The first deliverable is classification; code changes should be small and evidence-backed.

9 days ago

Small cleanup landed in f010122: the two parallel-GC test witnesses that directly iterated shared.elements.items now read via Object.elementAt(), so the assertions use the same elements_lock-protected path as shared-realm code. This classifies those two direct accesses as test-only witnesses and removes them from the residual side-door set.

Validation:

  • zig build test -Dtest-filter="contended parallel appends" -Dtsan=true --summary none
  • zig build test -Dtest-filter="quiescent collection after a parallel build" -Dtsan=true --summary none
  • git diff --check
9 days ago

Another small #17 slice landed in 66027de: C-API array construction now uses Object.appendElement(), and indexed property reads use Object.elementAt() instead of direct elements storage. Added a focused C-API regression for JSObjectMakeArray + JSObjectGetPropertyAtIndex.

Validation:

  • zig build test -Dtest-filter="C-API: array construction" --summary none
  • git diff --check
9 days ago

Structured-clone deserialization cleanup landed in c2b5e61. Arrays and Map-entry pair arrays now append via Object.appendElement(), and Map/Set internal backing storage uses a new Object.appendInternalElement() helper that keeps lock + GC-barrier discipline without toggling indexed-property bookkeeping.

This removes the deserializer-side raw elements.append paths while deliberately leaving serializer elements.items reads for a separate snapshot design, because recursive serialization must not hold elements_lock across calls/getters.

Validation:

  • zig build threadfuzz-bin --summary none && ./zig-out/bin/threadfuzz midgcpromise 1 1
  • zig build test -Dtest-filter="structuredClone" --summary none
  • git diff --check
9 days ago

Another #17 slice landed in 8a32df2 (fix(set): lock set operation element scans).\n\nWhat changed:\n- Set operation methods no longer read this/result Set backing storage through raw Object.elements.items loops.\n- Added collectLiveSetEntries() for locked snapshots before copying native Set entries into result Sets.\n- Added liveSetEntryAt() for locked per-index reads where the spec needs live-by-index behavior because set-like has() callbacks can mutate this.\n- Native Set getSetRecord snapshots now share the same locked live-entry helper.\n- Added a regression covering isSubsetOf/intersection when the set-like has() callback deletes a trailing element from this.\n\nValidation:\n- zig build test -Dtest-filter='Set operations keep live-index semantics' --summary none\n- git diff --check\n- plus the prior CI timeout root cause was fixed in zig-gc 49c7d8f; focused repro ./zig-out/bin/threadfuzz amplify 1 1 now passes locally.

9 days ago

Structured-clone serializer slice landed in e8e434f (fix(clone): snapshot element storage while serializing).\n\nWhat changed:\n- Map serialization snapshots live key/value pairs under element locks before recursive serialization. Deleted Map backing slots are skipped instead of surfacing as malformed entries.\n- Set serialization snapshots live entries under lock and skips tombstones.\n- Array serialization snapshots dense elements plus hole bits under lock, then serializes from the snapshot without holding elements_lock across recursion.\n- Snapshot object values are pushed into the interpreter temp-root list for the duration of recursive serialization, so concurrent mutation plus GC cannot strand a copied Value.\n- Added structuredClone coverage for deleted Map/Set slots.\n\nValidation:\n- zig build test -Dtest-filter='structuredClone' --summary none\n- git diff --check

9 days ago

Another #17 slice landed in 5fe9656 (fix(array): lock async-from dense appends).\n\nWhat changed:\n- Added Object.appendDataIndexIfDense(), which performs the CreateDataPropertyOrThrow dense-array append fast path as one elements_lock critical section.\n- Array.of / Array.from / Array.fromAsync createDataIndexOrThrow callers now use that helper instead of reading elements.items.len and appending directly.\n- Array.fromAsync's scheduled sync-iterator continuation tuple now uses appendInternalElement(), elementAt(), and setElementAt() instead of raw elements storage.\n- Updated thread docs to reflect the concrete element-storage side doors already closed and classify the residual direct elements.items buckets.\n- Extended the Array.fromAsync test to cover the scheduled sync-iterator rest callback with promised values.\n\nValidation:\n- zig build test -Dtest-filter='Array.fromAsync awaits thenable elements' --summary none\n- git diff --check

9 days ago

Another #17 slice landed in 18c2606 (fix(object): lock descriptor element reads).\n\nWhat changed:\n- Object.keys/Object.values/Object.entries live enumerable rechecks now use denseElementPresent() for array-index element presence instead of raw elements.items/isHole reads.\n- Object.assign uses the same locked dense-element presence path for source array indices.\n- Object.freeze/Object.seal/isFrozen/isSealed dense-index scans use elementsLen() + denseElementPresent().\n- Object.getOwnPropertyDescriptor reads array element descriptors via denseElement()/arrayLength() instead of raw storage.\n\nValidation:\n- zig build test -Dtest-filter='Object.keys/values/entries enumerate array indices' --summary none\n- zig build test -Dtest-filter='property descriptors: defineProperty attrs + getOwnPropertyDescriptor' --summary none\n- zig build test -Dtest-filter='array index property attributes' --summary none\n- zig build test -Dtest-filter='Object.freeze / seal / preventExtensions' --summary none\n- zig build test -Dtest-filter='enable_gc: collectGarbage reclaims unreachable objects' --summary none\n- zig build test -Dtest-filter='Array.fromAsync awaits thenable elements' --summary none\n- git diff --check

9 days ago

Landed another #17 slice in zig-js@42be6ea (fix(object): lock defineProperty element writes).

What changed:

  • Moved array length and array-index Object.defineProperty checks off raw elements.items.len / array_len reads and onto locked helpers (arrayLength, elementsLen, denseElementPresent, denseElement).
  • Replaced direct dense-element growth/value writes in the array-index data descriptor path with setOrGrowDenseElement, so gap materialization, hole clearing, element writes, and GC barriers happen under elements_lock.
  • Preserved partial descriptor semantics: omitted value keeps the current dense element for existing indices, snapshots mapped arguments bindings when needed, and materializes undefined for new present indices.
  • Replaced remaining generic array length extension writes in this path with extendArrayLengthFloor.

Focused validation:

  • zig fmt src/builtins.zig
  • zig build test -Dtest-filter='array index property attributes' --summary none
  • zig build test -Dtest-filter='array length defineProperty invariants' --summary none
  • zig build test -Dtest-filter='property descriptors: defineProperty attrs + getOwnPropertyDescriptor' --summary none
  • zig build test -Dtest-filter='defineProperty rejects incompatible redefinition' --summary none
  • git diff --check

The previous pushed run for 18c2606e is still in progress, but at the time of this comment it had no failed/cancelled/timed-out jobs (7 successes, 16 running). New CI will start from 42be6ea.

9 days ago

Landed another #17 slice in zig-js@6dcfe13 (fix(object): snapshot dense own-key indices).

What changed:

  • Added Object.denseElementIndices() to snapshot present dense element indices under elements_lock.
  • Switched objectOwnKeysList dense-index enumeration for String exotic objects, Arrays, and generic objects from raw elements.items + isHole() loops to the locked snapshot helper.
  • Kept key formatting, sorting, and duplicate merging outside of any JS callbacks; the helper snapshots indices only, not values.

Focused validation:

  • zig fmt src/value.zig src/interpreter.zig
  • zig build test -Dtest-filter='Object.keys/values/entries enumerate array indices' --summary none
  • zig build test -Dtest-filter='Object defineProperties uses ToObject and proxy ownKeys' --summary none
  • zig build test -Dtest-filter='Reflect.ownKeys' --summary none
  • zig build test -Dtest-filter='array index property attributes' --summary none
  • git diff --check

The immediately previous run for 42be6ea had no failed/cancelled/timed-out jobs at the pre-push snapshot. New CI will start from 6dcfe13.

9 days ago

Landed a tiny #17 follow-up in zig-js@5333a01 (fix(object): lock hasProperty dense checks).

What changed:

  • Replaced the remaining raw idx < elements.items.len && !isHole(idx) dense-array check in hasProperty() with denseElementPresent(idx), so [[HasProperty]] / in / prototype-chain indexed checks no longer peek at elements.items.len directly.

Focused validation:

  • zig fmt src/interpreter.zig
  • zig build test -Dtest-filter='interpreter compound assignments, nullish, and in' --summary none
  • zig build test -Dtest-filter='array instances inherit from Array.prototype (incl. holes)' --summary none
  • zig build test -Dtest-filter='Object.prototype: hasOwnProperty / isPrototypeOf' --summary none
  • git diff --check

The immediately previous run for 6dcfe13 had no failed/cancelled/timed-out jobs at the pre-push snapshot. New CI will start from 5333a01.

9 days ago

Landed another #17 slice in zig-js@c38002f (fix(object): lock array object-spread reads).

What changed:

  • Replaced the array branch in spreadDataProps / object spread ({ ...array }) that directly walked elements.items with objectOwnKeysList + descriptor filtering + [[Get]].
  • This avoids raw element-storage reads on a public shared-realm path and keeps Object.elements_lock out of JS callbacks.
  • Tightened semantics at the same time: array holes and non-enumerable indices are not copied, Array length is filtered out by descriptor enumerability, and accessor indices are read through [[Get]].

Focused validation:

  • zig fmt src/interpreter.zig src/context.zig
  • zig build test -Dtest-filter='object spread' --summary none
  • zig build test -Dtest-filter='array index property attributes' --summary none
  • zig build test -Dtest-filter='Object.keys/values/entries enumerate array indices' --summary none
  • zig build test -Dtest-filter='interpreter assignment and parameter destructuring' --summary none
  • git diff --check

The immediately previous run for 5333a01 had no failed/cancelled/timed-out jobs at the pre-push snapshot (8 successes, 15 running). New CI will start from c38002f.

9 days ago

Docs follow-up for #17 landed in zig-js@ad5a8b5 (docs(threads): update element side-door audit).

What changed:

  • Updated docs/threads/bindings.md to name the newly locked descriptor reflection/definition, own-key enumeration, hasProperty, and array object-spread paths in the Object.elements_lock ruling.
  • Updated docs/threads/P7-gil-removal.md blocker row 5 with the same newly closed paths.
  • Kept the remaining direct elements.items work classified as helper internals, GC trace/finalize paths, private construction/tuples, single-owner host internals, and a smaller set of public fast-path candidates needing locked snapshots or a documented single-owner proof.

Validation:

  • git diff --check

The immediately previous run for c38002f had no failed/cancelled/timed-out jobs at the pre-push snapshot. New CI will start from ad5a8b5.

9 days ago

Landed another #17 slice in zig-js@a7323d5 (fix(array): snapshot packed spread inputs).

What changed:

  • Added Object.packedDenseElementsSnapshot(), which copies packed dense array values under elements_lock.
  • spreadInto now uses that locked snapshot only for truly packed arrays; arrays with holes, sparse logical tails, or index accessors fall back to the normal iterator protocol, so no elements_lock is held across JS callbacks.
  • This removes the raw elements.items read from array/call spread fast paths and fixes spread semantics for inherited holes, sparse-tail call arity, and accessor-backed indices.
  • Updated docs/threads/bindings.md and docs/threads/P7-gil-removal.md to move packed array/call spread into the closed/locked bucket.

Focused validation:

  • zig fmt src/value.zig src/interpreter.zig src/context.zig
  • zig build test -Dtest-filter='spread of iterables (generator, string, user iterator)' --summary none
  • zig build test -Dtest-filter='interpreter spread in arrays and calls' --summary none
  • zig build test -Dtest-filter='array literal elision (holes)' --summary none
  • zig build test -Dtest-filter='array instances inherit from Array.prototype (incl. holes)' --summary none
  • git diff --check

The immediately previous run for ad5a8b5 had no failed/cancelled/timed-out jobs at the pre-push snapshot (3 successes, 20 running). New CI will start from a7323d5.

9 days ago

Landed another #17 slice in zig-js@2dac7c1 (fix(function): snapshot array argument lists).

What changed:

  • Function.prototype.apply argument-list construction now uses the packed dense snapshot helper instead of returning a raw elements.items slice.
  • The Reflect CreateListFromArrayLike helper for Reflect.apply / Reflect.construct now reads length, uses a locked packed snapshot only when safe, and falls back to per-index [[Get]] for holes, sparse logical tails, and accessors.
  • This removes another public raw Object.elements read path and fixes sparse-tail arity / inherited-hole / accessor-index behavior for apply/Reflect argument lists.
  • Updated docs/threads/bindings.md and docs/threads/P7-gil-removal.md to move CreateListFromArrayLike into the closed/locked bucket and shrink the residual public-fast-path list.

Focused validation:

  • zig fmt src/interpreter.zig src/context.zig
  • zig build test -Dtest-filter='Function.prototype: call / apply / bind' --summary none
  • zig build test -Dtest-filter='Reflect: prototype, toStringTag, array-like argumentsList' --summary none
  • zig build test -Dtest-filter='spread of iterables (generator, string, user iterator)' --summary none
  • zig build test -Dtest-filter='interpreter spread in arrays and calls' --summary none
  • git diff --check

The immediately previous run for a7323d5 had no failed/cancelled/timed-out jobs at the pre-push snapshot (5 successes, 18 running). New CI will start from 2dac7c1.

9 days ago

Landed another #17 slice in zig-js@5d257ad (fix(array): lock iterator length checks).

What changed:

  • forInKeyList no longer uses a raw elements.items.len gate for dense-index enumeration; it snapshots elementsLen() under elements_lock and uses that bound while rechecking present dense elements.
  • Array iterator next() now compares against arrayLength() instead of raw @max(elements.items.len, array_len), so %ArrayIteratorPrototype%.next avoids direct storage-length reads while still reading values through [[Get]].
  • Updated docs/threads/bindings.md and docs/threads/P7-gil-removal.md to record for-in dense enumeration and array iterator length checks as locked paths.

Focused validation:

  • zig fmt src/interpreter.zig src/context.zig
  • zig build test -Dtest-filter='for-of / for-in with destructuring + member targets' --summary none
  • zig build test -Dtest-filter='Array.prototype Symbol.iterator aliases values and rejects nullish this' --summary none
  • zig build test -Dtest-filter='spread of iterables (generator, string, user iterator)' --summary none
  • git diff --check

The immediately previous run for 2dac7c1 had no failed/cancelled/timed-out jobs at the pre-push snapshot (4 successes, 19 running). New CI will start from 5d257ad.

9 days ago

Closed another direct element-storage shortcut in the issue #17 path.

Commit: 718d772 (fix(object): snapshot groupBy array inputs)

What changed:

  • Object.groupBy / Map.groupBy no longer return raw elements.items for arrays.
  • Packed ordinary arrays now use the locked packed-array snapshot helper.
  • Sparse/customized arrays fall back to iterator protocol, so holes, prototype values, accessors, and custom iterators are observed before the groupBy callback runs.
  • Updated the thread audit docs to move groupBy collection into the locked/snapshot bucket.

Validation:

  • zig build test -Dtest-filter="interpreter JSON, Object, Number builtins" --summary none
  • git diff --check
9 days ago

Closed another #17 array-element shortcut.

Commit: bcae220 (fix(destructuring): honor sparse array rest)

What changed:

  • Array destructuring now uses logical array length for the optimized rest path instead of dense storage length.
  • Array destructuring element reads now go through the same [[Get]] helper used by array iteration, so sparse tails, holes, inherited index properties, and accessors stay observable.
  • Added regressions for new Array(3) rest collection and inherited sparse-tail values.
  • Updated the thread audit docs to include array destructuring/index-iteration length checks in the locked-helper bucket.

Validation:

  • zig build test -Dtest-filter="destructuring" --summary none
  • git diff --check
9 days ago

Closed another #17 direct element fast-path race.

Commit: 86bd6d4 (fix(array): lock result append fast path)

What changed:

  • Added Object.appendElementIfLen, which checks the current dense element length and appends under the same elements_lock hold.
  • arrayResultPush now uses that helper instead of reading elements.items.len directly before a separate append.
  • If the locked fast append cannot prove the expected index, it falls back to CreateDataPropertyOrThrow.
  • Updated the thread audit docs to count Array-method result appends in the locked-helper bucket.

Validation:

  • zig build test -Dtest-filter="interpreter Array.prototype methods" --summary none
  • git diff --check
9 days ago

Closed another #17 element side-door in Promise combinators.

Commit: c226f8f (fix(promise): lock combinator result arrays)

What changed:

  • Promise.all / allSettled / any result-array placeholders now append through appendElement instead of raw elements.append.
  • Per-element combinator reactions now write results through setElementAt instead of values.elements.items[index].
  • Keyed combinator settlement reads result values through elementAt instead of indexing the raw element slice.
  • Updated the thread audit docs to count Promise combinator result arrays in the locked-helper bucket.

Follow-up:

  • Opened #19 for the broader Promise aggregation state (remaining, already, final-settlement first-wins). This commit closes the result-array side door but deliberately does not pretend the whole aggregation record is synchronized.

Validation:

  • zig build test -Dtest-filter="Promise" --summary none
  • git diff --check
9 days ago

Iterator-helper internal-array runtime access got cleaned up as part of #18.

Commit: b15eea2 (fix(iterator): snapshot helper internals)

Relevant to #17:

  • Iterator.concat no longer borrows raw source/method elements.items slots while stepping sources; it uses locked element helpers.
  • Iterator.zip / zipKeyed snapshots engine-owned source/key arrays before iterator callouts and uses locked helpers for flags, padding, and result reads/writes.
  • return() close paths snapshot the iterator list before invoking underlying return() methods.
  • Construction-time raw fills remain private-before-publication; runtime/shared access is the part this retired.

Leaving #17 open for the broader residual Object.elements side-door sweep.

9 days ago

Closed another #17 residual direct element-storage class.

Commit: 8b65fa1 (fix(intl): lock locale list element access)

What changed:

  • Added small helpers for canonical Intl locale-list membership, first-locale, and first-supported-locale reads.
  • canonicalizeLocaleList now appends through Object.appendElement() and checks duplicates through locked element reads.
  • Date/Number/BigInt/String/Collator/Temporal/DurationFormat/Intl service locale selection now reads canonical locale-list elements through elementAt() / helper paths instead of direct elements.items access.
  • Intl.*.supportedLocalesOf appends output through appendElement().
  • Updated the thread audit docs to move Intl canonical locale-list reads/appends into the locked-helper bucket.

Validation:

  • zig build test -Dtest-filter='Date basics' --summary none
  • zig build test -Dtest-filter='Date setters + string conversions' --summary none
  • zig build test -Dtest-filter='String generics + .constructor + match/search' --summary none
  • zig build test -Dtest-filter='Number/Boolean/String prototypes are Exotic Objects with their primitive' --summary none
  • zig build test -Dtest-filter='TypedArray toLocaleString uses empty strings after shrink' --summary none
  • zig build test -Dtest-filter='Intl' --summary none
  • git diff --check
9 days ago

Closed another #17 side-door in the VM spread-call path.

Commit: d2653cc (fix(vm): snapshot spread call arguments)

What changed:

  • VM call_spread, call_method_spread, and new_spread now snapshot the spread argument array through Object.internalElementsSnapshot() before invoking the callee/constructor.
  • This avoids passing a borrowed elements.items slice across user callouts.
  • Updated docs/threads/bindings.md and docs/threads/P7-gil-removal.md to include VM spread-call argument snapshots in the locked/snapshot bucket.

Validation:

  • zig build test -Dtest-filter='interpreter spread in arrays and calls' --summary none
  • zig build test -Dtest-filter='spread of iterables (generator, string, user iterator)' --summary none
  • zig build test -Dtest-filter='Function.prototype: call / apply / bind' --summary none
  • zig build test -Dtest-filter='Reflect: prototype, toStringTag, array-like argumentsList' --summary none
  • git diff --check
9 days ago

Tiny #17 follow-up landed.

Commit: 6cf3d96 (fix(class): snapshot default super arguments)

What changed:

  • Synthesized default derived constructors now snapshot their rest-argument array with Object.internalElementsSnapshot() before forwarding it into super(...) / construction.
  • This removes another borrowed elements.items slice that crossed a user constructor callout.
  • Updated the thread audit docs alongside the VM spread-call argument snapshot note.

Validation:

  • zig build test -Dtest-filter='class methods/accessors/constructor are non-enumerable' --summary none
  • zig build test -Dtest-filter='empty statements + class-declaration sequencing' --summary none
  • zig build test -Dtest-filter='spread of iterables (generator, string, user iterator)' --summary none
  • zig build test -Dtest-filter='DataView constructor observes NewTarget prototype side effects' --summary none
  • git diff --check
9 days ago

Closed another small #17 public element-storage read.

Commit: 22312ea (fix(json): lock array length snapshot)

What changed:

  • JSON.stringify array serialization now snapshots ordinary array length through Object.arrayLength() instead of reading shape.elements.items.len directly.
  • The element lock is not held across element serialization / getter callouts; only the length snapshot is taken under the helper.
  • Updated docs/threads/bindings.md and docs/threads/P7-gil-removal.md to include JSON array serialization length snapshots in the locked-helper bucket.

Validation:

  • zig build test -Dtest-filter='interpreter JSON, Object, Number builtins' --summary none
  • zig build test -Dtest-filter='JSON' --summary none
  • git diff --check
9 days ago

Closed another #17 internal element-list side door.

Commit: 4ba7d5d (fix(dispose): snapshot resource entries)

What changed:

  • DisposableStack / AsyncDisposableStack resource tuple fills now use appendInternalElement().
  • Resource-list appends now use appendInternalElement().
  • Disposal snapshots \x00ds_res before invoking user disposal callbacks / awaiting async disposal, so it no longer walks a borrowed elements.items slice across callouts.
  • Resource tuple fields are read through elementAt().
  • Added a dedicated docs row for DisposableStack resource element storage and updated the main Object.elements_lock audit bucket.

Important split:

  • This is the element-storage cleanup only. Same-stack disposed-state / push / move / dispose synchronization is broader than #17, so I opened #21 to track that explicitly.

Validation:

  • zig build test -Dtest-filter='Disposable' --summary none
  • zig build test -Dtest-filter='using' --summary none
  • zig build test -Dtest-filter='SuppressedError' --summary none
  • git diff --check
9 days ago

Follow-up to the DisposableStack element-storage cleanup: the broader same-stack race is now handled too.

Commit: 63f29ff (fix(dispose): lock stack resource state)

Relevant to #17:

  • The resource tuple/list element cleanup from 4ba7d5d is now paired with a stack-state mutex, so resource-list snapshot/clear and push/transfer are not just individually lock-helper based, but coherently serialized around disposed publication.
  • The stack lock is released before user disposal callbacks / async awaits, preserving the #17 rule not to hold element locks across callouts.
  • #21 has the full state-synchronization details and no-GIL witness.
9 days ago

Landed another small #17 cleanup in 444aacd: Object.keys/values/entries result arrays, Object.getOwnPropertyNames/Symbols result arrays, Array constructor element insertion, and JSON.parse array construction now append through Object.appendElement instead of direct elements.append calls.

This is mostly fresh/private result construction, so it was not one of the scarier shared-object races, but it removes another public builtin-side direct append class and keeps indexed bookkeeping/barriers centralized behind the element-storage funnel.

Docs updated:

  • docs/threads/bindings.md
  • docs/threads/P7-gil-removal.md

Validation run for this chunk:

  • zig fmt src/builtins.zig
  • zig build test -Dtest-filter=Object.keys/values/entries enumerate array indices --summary none
  • zig build test -Dtest-filter=Array / Object constructors --summary none
  • zig build test -Dtest-filter=interpreter JSON, Object, Number builtins --summary none
  • zig build test -Dtest-filter=JSON --summary none
  • git diff --check
9 days ago

Landed another narrow #17 cleanup in 94d8893: fresh/public array builders in the VM and interpreter now append through Object.appendElement instead of direct elements.append.

Touched paths:

  • VM array-literal ordinary element append
  • for-in key arrays
  • tagged-template cooked/raw template arrays
  • proxy apply/construct argument-list arrays
  • iterableToArray materialization
  • function arguments objects
  • rest-parameter arrays

I deliberately left array-literal hole elision alone for now; it wants an explicit hole-aware helper rather than treating an absent element as an ordinary append.

Validation run for this chunk:

  • zig fmt src/vm.zig src/interpreter.zig
  • zig build test -Dtest-filter=interpreter array literal, index, length, push/pop --summary none
  • zig build test -Dtest-filter=interpreter template literals --summary none
  • zig build test -Dtest-filter=interpreter arguments object and Array/Object statics --summary none
  • zig build test -Dtest-filter=interpreter default and rest parameters --summary none
  • zig build test -Dtest-filter=Reflect: prototype, toStringTag, array-like argumentsList --summary none
  • zig build test -Dtest-filter=interpreter for-of and for-in --summary none
  • zig build test -Dtest-filter=Array.from with iterables + map fn --summary none
  • zig build test -Dtest-filter=spread of iterables (generator, string, user iterator) --summary none
  • git diff --check
9 days ago

Landed 326f346 for another #17 cleanup: RegExp builtin exec result arrays and global match result arrays now use Object.appendElement, and the global-match empty-result check reads through elementsLen instead of directly inspecting elements.items.len.

This keeps the RegExp/String match array construction on the same element-storage funnel while still avoiding locks across user callbacks or generic exec calls.

Validation run for this chunk:

  • zig fmt src/interpreter.zig
  • zig build test -Dtest-filter=RegExp builtin exec exposes UTF-16 positions --summary none
  • zig build test -Dtest-filter=RegExp match reads flags and observes lastIndex recompilation --summary none
  • zig build test -Dtest-filter=interpreter regex literals --summary none
  • zig build test -Dtest-filter=RegExp generic exec dispatch validates protocol results --summary none
  • zig build test -Dtest-filter=RegExp search uses generic exec and restores lastIndex --summary none
  • zig build test -Dtest-filter=RegExp replace uses generic exec and UTF-16 positions --summary none
  • zig build test -Dtest-filter=String.prototype.split: limit + regex separators --summary none
  • git diff --check
9 days ago

Landed another #17 cleanup in 1d2e673: array-literal holes and a few fresh result/bucket arrays now go through Object element helpers.

What changed:

  • Added Object.appendArrayHole(), which marks the current dense end as a hole and appends the length-counting undef placeholder under one elements_lock critical section.
  • VM and tree-walker array-literal elisions now use appendArrayHole() instead of markHole(length) + raw elements.append.
  • Tree-walker ordinary array-literal elements, destructuring rest arrays, Object.groupBy/Map.groupBy bucket arrays, and Reflect.ownKeys result arrays now append through Object.appendElement().
  • Updated docs/threads/bindings.md and docs/threads/P7-gil-removal.md to record the newly funneled paths.

Validation run for this chunk:

  • zig fmt src/value.zig src/vm.zig src/interpreter.zig
  • zig build test -Dtest-filter=array literal elision (holes) --summary none
  • zig build test -Dtest-filter=interpreter array literal, index, length, push/pop --summary none
  • zig build test -Dtest-filter=interpreter assignment and parameter destructuring --summary none
  • zig build test -Dtest-filter=interpreter JSON, Object, Number builtins --summary none
  • zig build test -Dtest-filter=Reflect.ownKeys --summary none
  • zig build test -Dtest-filter=array destructuring over the iterator protocol --summary none
  • git diff --check
9 days ago

Landed another small #17 cleanup in 4f89a54: fresh pair/result arrays now use Object.appendElement instead of raw elements.append.

Touched paths:

  • RegExp hasIndices [start, end] pairs and the outer .indices result array.
  • Array/arguments/proxy iterator .entries() pair arrays.
  • TypedArray iterator .entries() pair arrays.

Docs updated:

  • docs/threads/bindings.md
  • docs/threads/P7-gil-removal.md

Validation run for this chunk:

  • zig fmt src/interpreter.zig
  • zig build test -Dtest-filter=RegExp builtin exec exposes UTF-16 positions --summary none
  • zig build test -Dtest-filter=Array.prototype Symbol.iterator aliases values and rejects nullish this --summary none
  • zig build test -Dtest-filter=interpreter TypedArray iterators --summary none
  • zig build test -Dtest-filter=Map/Set expose [Symbol.iterator]; Set keys === values --summary none
  • zig build test -Dtest-filter=Set/Map are iterable: for-of, spread, Array.from, destructuring --summary none
  • git diff --check
9 days ago

Landed another #17 cleanup in e2ecab8: public iterator/Intl result arrays now append through Object.appendElement instead of raw elements.append.

Touched paths:

  • Iterator.prototype.toArray and AsyncIterator.prototype.toArray result arrays.
  • Intl.supportedValuesOf result arrays.
  • Intl.Locale list/weekend/timezone arrays.
  • Intl.DateTimeFormat formatToParts and formatRangeToParts result arrays.

Docs updated:

  • docs/threads/bindings.md
  • docs/threads/P7-gil-removal.md

Validation run for this chunk:

  • zig fmt src/interpreter.zig
  • zig build test -Dtest-filter=Intl.supportedValuesOf timeZone values are code-unit sorted --summary none
  • zig build test -Dtest-filter=Intl.DateTimeFormat German numeric date pattern --summary none
  • zig build test -Dtest-filter=parallel_js: Iterator helper rejects concurrent next while running --summary none
  • zig build test -Dtest-filter=Intl --summary none
  • zig build test -Dtest-filter=parallel_js: AsyncIterator helper rejects concurrent entry while running --summary none
  • git diff --check
9 days ago

Landed the second Intl result-array #17 cleanup in 73be4d5: remaining Intl formatter result arrays now append through Object.appendElement instead of raw elements.append.

Touched paths:

  • Intl.NumberFormat formatToParts / formatRangeToParts arrays.
  • Intl.DurationFormat formatToParts arrays.
  • Intl.ListFormat formatToParts arrays.
  • Intl.RelativeTimeFormat formatToParts arrays.
  • Intl.PluralRules resolvedOptions().pluralCategories.

After this slice, the remaining direct elements.append sites in src/interpreter.zig are the deliberately classified Map/Set storage paths and iterator-helper private record arrays, not general public result-array builders.

Validation run for this chunk:

  • zig fmt src/interpreter.zig
  • zig build test -Dtest-filter=Intl --summary none
  • zig build test -Dtest-filter=Intl.PluralRules validates localeMatcher option --summary none
  • git diff --check
9 days ago

Landed another #17 cleanup in 994696f: Iterator helper private-record arrays now use Object.appendInternalElement() instead of raw elements.append.

Touched paths:

  • Iterator.concat captured source/method records.
  • Iterator.zip source iterator records, padding records, and done-flag records.
  • Iterator.zipKeyed key/source/padding records.

This is intentionally classified as engine-private storage rather than public indexed array output; appendInternalElement keeps lock + GC-barrier discipline without marking the objects as having observable indexed own properties. The remaining raw elements.append sites in src/interpreter.zig are now the lock-scoped Map/Set storage paths.

Added coverage:

  • Iterator concat and zip helpers produce expected records

Validation run for this chunk:

  • zig fmt src/context.zig src/interpreter.zig
  • zig build test -Dtest-filter=parallel_js: Iterator helper rejects concurrent next while running --summary none
  • zig build test -Dtest-filter=parallel_js: AsyncIterator helper rejects concurrent entry while running --summary none
  • zig build test -Dtest-filter=Iterator concat and zip helpers produce expected records --summary none
  • git diff --check
9 days ago

Final #17 audit/cleanup landed in zig-js@dcca4347 (fix(clone): snapshot transfer list validation).

What changed in this closing slice:

  • structuredClone(value, { transfer }) now snapshots the transfer-list array with Object.internalElementsSnapshot() before validating entries, so it no longer borrows public JS input through elements.items.
  • docs/threads/bindings.md and docs/threads/P7-gil-removal.md now classify the remaining direct raw element-storage sites concretely instead of saying generic public fast-path candidates remain.

Final audit result:

  • The only remaining raw elements.append sites outside the value.zig helper funnel are the Map/Set backing-storage appends in interpreter.zig; both run under Object.elements_lock and publish barriers before appending.
  • Remaining direct elements.items users are classified as value.zig lock-helper internals, GC stop-the-world/explicit-lock tracing, structured-clone lock-and-copy snapshots, lock-scoped Map/Set storage/cursor helpers, or iterator-helper private setup/abrupt-close snapshots.
  • No unclassified shared-realm public fast path from the original #17 sweep is left borrowing Object.elements storage directly.

Validation for this closing slice:

  • zig fmt src/interpreter.zig
  • zig build test -Dtest-filter=structuredClone --summary none
  • git diff --check
  • residual grep audit for .elements.append / .elements.items categories

Closing this tracker as done; future new direct Object.elements side doors should get their own issue with the specific path and synchronization rule.

Sign in to comment on this issue.