Skip to content

Key the host-boundary constraint gate on execution depth and close remaining lanes#2715

Merged
lahma merged 1 commit into
sebastienros:mainfrom
lahma:amortized-host-boundary-depth-gate
Jul 21, 2026
Merged

Key the host-boundary constraint gate on execution depth and close remaining lanes#2715
lahma merged 1 commit into
sebastienros:mainfrom
lahma:amortized-host-boundary-depth-gate

Conversation

@lahma

@lahma lahma commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Final hardening pass over the host-boundary constraint mechanism from #2713/#2714 (an adversarial review round completed against #2714's diff after it merged). The headline fix: #2714's gate keyed on the ambient evaluation context, which async resume paths do not install — so all boundary checks were silently disabled during host-driven event-loop drains, and a cancellation could go entirely unobserved there.

Fixes

  • Gate on execution-context stack depth, not the ambient evaluation context. Resolving a promise from the host (Advanced.RegisterPromise().Resolve(...), UnwrapIfPromise waits) drains async continuations that create evaluation contexts via _activeEvaluationContext ?? new ... without installing the ambient field, so Gate host-boundary constraint checks on active evaluation and harden coverage #2714's gate saw null mid-drain and skipped every check — while (cond) { slowWork(); await ...; } could ignore cancellation indefinitely. Every execution shape (script evaluation, calls, generator/async resumes, module evaluation) pushes an execution context and idle host-side access does not, so depth > 1 is the reliable "evaluation in progress" signal. A regression test drives the drain from a host-resolved promise.
  • Debug exemption narrowed from all debug-mode execution to debugger expression evaluation only (DebugHandler.Evaluate, tracked via an engine flag). Gate host-boundary constraint checks on active evaluation and harden coverage #2714 disabled the checks for the whole lifetime of any debugger-enabled engine; now normal full-speed debug-mode execution keeps full enforcement while watch/breakpoint-condition evaluation stays exempt. Tests cover both directions.
  • Throwing host calls now re-check inside the TargetInvocationException conversion handlers — previously a loop of throwing host calls never reached the return-side checks and fell back to the 64-statement countdown.
  • Awaitable-result ordering completed in the sibling lanes: MethodInfoFunction and the ReflectionAccessor read lane (check relocated to ReflectionDescriptor.DoGet) now convert results before the check, matching Gate host-boundary constraint checks on active evaluation and harden coverage #2714's DelegateWrapper fix, so an in-flight Task always gets its promise continuation attached.
  • Remaining interop lanes covered: dictionary in/delete (ContainsDictionaryKey/TryRemoveDictionaryValue), the ICollection count fast path, array-like integer-index reads (ArrayLikeWrapper.GetAt), wrapped-dictionary key enumeration (for-in/Object.keys, per key pulled from the user enumerator), and CLR operator overloads — checked in the operator call sites outside their catch-alls, so constraint exceptions are no longer laundered into TargetInvocationException (this also restores per-call detection that Gate host-boundary constraint checks on active evaluation and harden coverage #2714 had removed entirely for operators).
  • Iterator disposal: a boundary check throwing during MoveNext (before for-of has a successful first step to close from) now disposes the user's enumerator instead of leaking it.
  • Flaky-test removal: the idle-timeout host-side test no longer executes an interop read under a live short timeout (it could race the CTS timer on a loaded runner), and the new TimeConstraint boundary test is deterministic — the host call polls the public Constraints.Check() until the budget has observably expired, immune to CTS timer starvation.

Still intentionally uninstrumented: built-in ClrFunction dispatch (per-call cost on hot built-ins; built-ins self-check via Engine.ConstraintCheckInterval) and user-supplied converter/factory callbacks. Entry checks remain deliberately absent: they would block host cleanup calls in JS finally blocks; the documented residual is that a cancelled script can start at most one more host call per boundary.

Verification

10 new/updated tests in ExecutionConstraintTests (61 total in the class), all deterministic; the host-driven async drain, debug-mode enforcement, and debugger-exemption tests fail against #2714 as merged. Full Jint.Tests, Jint.Tests.PublicInterface, Jint.Tests.CommonScripts and Test262 (99,431) pass on net10.0 and net472. The discussion #2707 repro still throws at ~290 ms.

No checks were added to the benchmarked interop read lanes relative to #2714 (the ClrObject/JsonNode check moved after result conversion, the dictionary hit-path check is unchanged and its miss-path double-check was removed); allocations are byte-identical across all benchmark runs.

🤖 Generated with Claude Code

Second review pass over the boundary-check mechanism:

- Gate on execution-context stack depth instead of the ambient
  evaluation context: async resume paths create evaluation contexts
  without installing the ambient field, so the previous gate silently
  disabled all boundary checks during host-driven event-loop drains
  (UnwrapIfPromise, promise resolution from C#) — a cancellation could
  go entirely unobserved there. Every execution shape pushes an
  execution context, and idle host-side access does not, so depth is
  the reliable signal.
- Exempt only debugger expression evaluation (DebugHandler.Evaluate,
  tracked via a flag) instead of all debug-mode execution; debug-mode
  engines keep full enforcement while running.
- Re-check inside the TargetInvocationException conversion handlers so
  a loop of throwing host calls cannot stretch the detection window.
- Convert results before the check in MethodInfoFunction and the
  ReflectionAccessor read lane (relocated to ReflectionDescriptor) so
  awaitable results always get their promise continuation attached,
  matching the DelegateWrapper ordering.
- Cover the remaining interop lanes: dictionary contains/remove,
  ICollection count fast path, array-like integer-index reads, wrapped
  key enumeration (for-in/Object.keys), and CLR operator overloads
  (checked outside the operator catch-all so constraint exceptions are
  not laundered into TargetInvocationException).
- Dispose the user's enumerator when a boundary check throws during
  iteration before for-of starts closing the iterator record.
- Tests: deterministic TimeConstraint boundary test (polls the public
  constraint check, immune to CTS timer starvation), host-driven async
  drain test, debug-mode enforcement + debugger-evaluation exemption
  tests, non-string-keyed dictionary and MemberAccessor lane tests;
  the idle-timeout host-side test no longer races the timer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lahma
lahma merged commit dcede5f into sebastienros:main Jul 21, 2026
4 checks passed
@lahma
lahma deleted the amortized-host-boundary-depth-gate branch July 21, 2026 06:42
lahma added a commit that referenced this pull request Jul 22, 2026
…#2736)

The 4.14 host-boundary constraint gate (#2715) keys on execution-context
stack depth, which made push/pop balance a load-bearing invariant under
every exception type. Several resume lanes only handled
JavaScriptException and leaked their frame when a raw constraint
exception (TimeoutException, ExecutionCanceledException) unwound them:
generator resume, async function resume, async generator resume, the
top-level-await module arm, and ShadowRealm.importValue on module
resolution failure. A leaked frame satisfied the depth gate forever, so
every later host-side read of a wrapped object on an idle engine
re-observed the re-armed/cancelled constraints — exactly the failure
mode the gate was built to prevent.

Also closes two adjacent gaps found in the same review:

- ExecuteWithConstraints now skips ResetConstraints for nested entries
  (a host callback calling back into the engine), keyed on the same
  depth signal: "while (true) hostCallbackThatEvaluates()" previously
  re-armed the outer script's timeout/statement budget on every
  iteration and ran forever.
- The debugger-evaluation exemption now also covers the per-statement
  amortized countdown, so a longer watch expression evaluated while
  paused past the timeout doesn't trip it.

The generator/async tests fail without the balance fixes.


Claude-Session: https://claude.ai/code/session_0115tQFNyyQqc1HQGPLUgZND

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant