Options: a group materialized while the freeze is running ends up frozen, and a cloned registry carries the freeze

[lahma]

Closes #3444. Closes #3445.

Two fixes, not one

They share a file and an invariant — everything reachable from a frozen Options is frozen — and nothing
else. #3444 is a publication-ordering defect between two threads that is reachable today; #3445 is a
single-threaded asymmetry in cloning that is currently unreachable because both callers compensate for it.
They travel together only because a second PR in Jint/Options.ReadOnly.cs would have to be rebased onto
this one anyway. The reasoning for each is below, separately.

#3444: the interleaving, as proved

Every group accessor was public IntlOptions Intl => Materialize(ref _intl, _readOnly);, so the owner's
frozen state was read at the call site, before Materialize allocated and published. MakeReadOnly
sets the flag and then cascades over the twelve backing fields. Both halves can therefore miss the same
group:

1. Thread A reads _readOnly == false and enters Materialize.
2. Thread B sets _readOnly = true, then reads _intl — still null, so the cascade covers nothing.
3. Thread A publishes with the Interlocked.CompareExchange, unfrozen, onto a frozen Options.

It stays writable for the life of the process, and the engine reads Intl and Temporal lazily long after
construction, so a write to one of them still lands on a live engine. Options is documented as safe to
share between engines being constructed concurrently, so this is an ordinary embedding — one host thread
finishing with an instance while another is still touching a group — not a contrivance.

Jint.Tests.PublicInterface/HostOptionsShapeTests.AGroupMaterializedWhileTheOptionsAreBeingFrozenIsStillFrozen
runs two threads released from one Barrier, one materializing every group in the order the cascade walks
them and one calling MakeReadOnly(), and then asserts that every group refuses a write. Against
unmodified main, first run:

Failed AGroupMaterializedWhileTheOptionsAreBeingFrozenIsStillFrozen [271 ms]
  Expected collection to be empty because a group materialized while MakeReadOnly runs must end up frozen,
  and 109 of 1000 attempts published a writable one, but found at least one item
  {"attempt 0: Options.Constraints accepted a write on a frozen Options"}.

It is a bounded sweep rather than a deterministic gate, and that is a real limitation of the seam. The
window is the few nanoseconds between the accessor's state read and its publication, and nothing
host-supplied runs inside it — no callback, no host object's constructor — so there is nothing outside the
assembly a ManualResetEventSlim could be wedged into. What makes the sweep sharp rather than a guess is
alignment, not load: two threads and one barrier, the materializing one walking the groups in the cascade's
own order so that the freeze falls inside the first group's window as often as the scheduler allows, and
the clock resolved before the round starts so the freezing thread arrives at the freeze with nothing else to
do first. Measured against the unfixed engine, four runs of a thousand attempts each: 109, 62, 105 and 77
escapes, the earliest on attempt 0. It collects every escape and reports the count rather than stopping at
the first. After the fix, 20 000 attempts produce none.

The fix

• Materialize takes the flag by reference and reads it again after the interlocked publication,
  freezing the winner if it is set. Interlocked.CompareExchange is a full fence, so a freeze that has
  already set the flag is visible to that second read whether or not its cascade saw the field.
• SetReadOnly publishes the flag with Volatile.Write plus Thread.MemoryBarrier() before it reads
  the fields. It already set the flag first; what it lacked was the fence. A release write orders the
  stores before it and not the loads after it, so without a StoreLoad barrier the two halves can still
  miss simultaneously — the cascade reading a field the accessor has not published yet, while the accessor
  reads a flag still sitting in the freezing thread's store buffer. That is Dekker's, and it is real on x86
  as well as on ARM.

Between them one of the two always catches the group: the cascade finds it published, or the accessor finds
the flag. Freezing an already-frozen group is a no-op, so it does not matter which. Options._readOnly and
WebApiOptions._readOnly are the two flags that gate a publication and so the two that get the barrier;
the other eighteen group flags gate value setters only, where a stale read costs one write that should have
been refused — which is what happens anyway when a setter passes ThrowIfReadOnly a moment before the
freeze begins, and is not something a barrier can prevent. That distinction is written down on the field.

What it costs

Nothing that is not cold, and no lock.

path	before	after
accessor hit (group already exists)	load _readOnly, pass by value	pass ref _readOnly; the flag is not read at all
accessor miss (once per group per Options)	one branch	one extra acquire read + one predictable branch, and on a frozen owner one redundant SetReadOnly cascade of at most eight null checks
MakeReadOnly()	—	one Thread.MemoryBarrier(), i.e. one lock or [rsp], 0 on x64 with the line already hot
ThrowIfReadOnly / IsReadOnly / AddConfiguration	plain load	acquire load (a plain mov on x64, ldar on ARM64) — host-side setters only, never an execution path

The engine constructor calls MakeReadOnly() twice (Options and sourceOptions, usually the same
object), so an engine build gains two barriers, plus one more if it materializes WebApi. I have not run
BenchmarkDotNet for it and would rather say so than quote a number I have not measured — say the word if you
want the construction row measured rather than argued. The one thing worth noting in the other direction:
the flag read moved off the accessor's hit path, and Options.Interop is read on hot interop paths.

#3445: a cloned registry carries the freeze

OptionsList<T>.Clone() was new(_name, new List<T>(_items)), and a fresh registry is born with
_readOnly == false. Every group's own Clone() is MemberwiseClone() and therefore does carry the
source's frozen state, so a group cloned from a frozen source came back frozen around writable registries —
IsReadOnly answering true on Options.Interop and false on Interop.ObjectConverters beside it.

Nothing shipped was broken by it because both callers compensated: CloneWithPrivateWebApiOptions re-froze
the subtree it had just copied, and CreateEngineOptions thaws the whole clone immediately. Clone() now
carries the flag, which makes the first of those unnecessary rather than load-bearing — the explicit
re-freeze and the comment naming the asymmetry are both gone, since a comment describing something that is
no longer true is worse than none.

Two tests, because the defect is reachable from two distances:

• Jint.Tests/Runtime/OptionsCloneReadOnlyTests goes at InteropOptions.Clone and ConstraintOptions.Clone
  directly — the two the issue names, because neither of their callers compensates. This is the clean red,
  needing no other change:

  Failed AGroupClonedFromFrozenOptionsIsFrozenAroundItsRegistriesToo
    Expected boolean to be True because a clone of a frozen group must not be frozen around a writable
    registry, but found False.
  

  Its sibling TheUntrustedProfilesPrivateCopyIsThawedThroughout is the other half: the one caller that
  wants a writable copy asks for it, and still gets it all the way down to the registries.

• HostOptionsReadOnlyTests.TheEnginesPrivateWebApiCopyIsFrozenAroundFrozenRegistries is the embedder's
  view, through the only public door onto a cloned group — Engine.WebApi.Enable's callback, with Fetch
  materialized before the freeze so the copy comes from Clone rather than from the accessor. It is red
  once the compensation is removed, which is precisely the claim that the compensation was load-bearing:

  Failed TheEnginesPrivateWebApiCopyIsFrozenAroundFrozenRegistries
    Expected webApi.Fetch.AllowedSchemes.IsReadOnly to be True because the copy is frozen, and a registry
    inside a frozen group is frozen too, but found False.
  

  Beside it, the existing TheLiveWebApiDoorSuspendsTheGuardForTheGroupItIsConfiguringAndNothingElse stayed
  green in that same run — worth stating, because it asserts the same refusal for a sub-group nobody had
  materialized before the freeze, which reaches the copy through Materialize rather than through Clone.
  Two paths, and only the clone one was broken.

The sibling audit

Materialize is not the only lazy publish on Options, so every field written on read and every place a
_readOnly value is captured before the work it guards:

site	verdict
the twelve Options group accessors + the eight on WebApiOptions	the defect — fixed
Options.TimeSystem's _timeSystem ??=	the same shape, left alone deliberately: #3447 was already replacing it with an Interlocked.CompareExchange and resolving it at freeze time, and has since merged. Fixing it here would have been a second edit to the same lines
Options.ResultLimits, ProfilingOptions.MaxEvents	plain getters over a backing field; nothing is written on read
Options._nodeBuiltinModules	written by UseNodeBuiltinModules, never on a read (and #3447 has since put a guard on that door)
AddConfiguration — if (_readOnly) throw; _configurations.Add(...)	check-then-act, but it publishes no object: the worst case is one appended configuration, the accepted "the freeze is a state change, not a lock" property. Now an acquire read for consistency within the class
every ThrowIfReadOnly()-then-write setter (about ninety-five of them)	same, and the same verdict — the issue says so itself
CloneWithPrivateWebApiOptions's SetReadOnly(webApi, _readOnly)	captured the flag after cloning; removed, because the clone now carries it
the other eighteen group _readOnly flags	plain, deliberately — see the paragraph above

Composition with #3447

#3447 merged while this was being written, so this branch is rebased onto it rather than composed with
it. The two touched one hunk in common and it was textual rather than semantic: #3447 added
if (value) { _ = TimeSystem; } to the top of SetReadOnly, this PR replaced the _readOnly = value below
it with the volatile write and the barrier. Resolved by keeping both in that order — resolve TimeSystem
first, then publish the flag with the fence, then cascade — and everything else (Options.cs,
Options.WebApi.cs, AGENTS.md, both test files) auto-merged. The full suite and test262 were re-run on the
rebased tree; the numbers below are from that run.

Not a public API change

Materialize is private, OptionsList<T>.Clone and SetReadOnly are internal, and both _readOnly fields
stayed private. The five baselines are unchanged (the net10.0 leg that verifies them is green), so there is
no docs/v5-migration.md row and no baseline regeneration here.

Verification

On the rebased tree:

• dotnet build -c Release (solution) and dotnet test -c Release (solution) green, exit 0.
• Jint.Tests 10 783 passed on net8.0 and net10.0, 7 407 on net472; Jint.Tests.PublicInterface
  3 132 / 3 122 / 2 501 on net10.0 / net8.0 / net472; Jint.Tests.CommonScripts 28;
  Jint.Tests.SourceGenerators 71.
• Jint.Tests.PublicInterface green again on all three target frameworks with
  JINT_HOST_CONTRACT_VERIFICATION=1 (3 145 / 3 135 / 2 514).
• test262: 102 495 passed, 0 failed, 189 skipped — unchanged.
• The race test at 20 000 attempts: no escapes. At its shipped 1 000 attempts it costs ~200 ms.

🤖 Generated with Claude Code

https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S