ReviewOS

zig-utils/zig-js

GC: relocate JSON reviver traversal state across moving callbacks

#833
Closed chris opened this 11 days ago ·
11 days ago

Problem

JSON.parse keeps the reviver, holder/value graph, and JsonParseRecord values in raw native locals while InternalizeJSONProperty invokes user code. A reviver can trigger a moving collection; the next callback then uses stale cells.

The forced-moving witness currently aborts in bindRootedParams through internalizeJson / jsonParse because the write barrier receives a stale managed environment.

Required fix

  • Give the recursive reviver traversal an explicit GC rooting/relocation contract for holder, val, reviver, per-child results, and parse-record Value fields.
  • Preserve the ECMA-262 InternalizeJSONProperty ordering and context.source identity rules.
  • Cover normal and abrupt reviver callbacks under forced moving collection in tree-walker and required-bytecode modes.
  • Run the focused witness under TSan and the full unit/conformance gates.

Discovered while auditing synchronous defining-realm wrappers for #830. This is a separate traversal-root bug, not a caller-realm restore bug.

11 days ago

Landed directly on main as 6b964afc — fix(json): relocate reviver traversal state (#833).

The reviver path now roots the coerced source string, reviver, holder, recursive values, callback results, context objects, and every relocatable object/managed-string parse-record value. Scalar records do not consume root slots. The walk reloads these operands after each observable step, preserving InternalizeJSONProperty order and exact context.source SameValue rules through moving collection and abrupt completion.

Validation on the committed snapshot:

  • forced moving GC: 4 cases pass in tree-walker and required-bytecode modes, covering later object/string records, arrays, abrupt throws, and observable text coercion
  • exhaustive allocation-failure sweep: 1/1 pass with value/env/call roots, depth, and active environment restored at every failure boundary
  • unsuppressed TSan: 2/2 pass, 0 failures/leaks/race reports
  • test262 test/built-ins/JSON/parse: 77/77 pass
  • threadfuzz seed 1: default 400/400, verify 300/300
  • relocation inventory: 9 cell kinds, 27 pointer surfaces, 21 boundary tags
  • full Debug suite: 2,217 passed, 1 platform skip, 0 failed/leaked across 2,218 tests (639.6s, 10 shards)

The push-triggered CI run is the remaining remote signal; local completion evidence is recorded here.

11 days ago

Push-triggered CI 33293313872 completed with the JSON change covered by successful unit, TSan-unit, test262-parallel, docs, ABI, and fuzz profiles. The sole failed leg was the pre-existing deterministic Worker/Condition lifecycle assertion tracked in #792; exact recurrence evidence is recorded there. No JSON.parse or reviver failure occurred.

Sign in to comment on this issue.