Skip to content

Gate host-boundary constraint checks on active evaluation and harden coverage#2714

Merged
lahma merged 1 commit into
sebastienros:mainfrom
lahma:amortized-host-boundary-hardening
Jul 20, 2026
Merged

Gate host-boundary constraint checks on active evaluation and harden coverage#2714
lahma merged 1 commit into
sebastienros:mainfrom
lahma:amortized-host-boundary-hardening

Conversation

@lahma

@lahma lahma commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follow-up hardening for #2713 (merged before this review pass landed on the branch). An adversarial multi-agent review of the boundary-check mechanism confirmed several edge-case regressions and coverage gaps in the merged version; this PR fixes them.

Regressions fixed

  • Host-initiated interop access observed constraint state outside script execution (A/B-verified against the pre-Re-check amortized constraints at interpreter/host-code boundaries #2713 behavior). The time constraint's CTS is re-armed at the end of every run and keeps counting while the engine is idle, and CancellationConstraint state survives execution — so after Re-check amortized constraints at interpreter/host-code boundaries #2713, reading a wrapped object's members from plain C# after execution could throw TimeoutException/ExecutionCanceledException with no script running. The boundary check is now gated on an active evaluation context.
  • Entry checks blocked host cleanup calls: with cancellation pending, a CLR delegate invoked inside a JS finally block (release-a-lock pattern) threw before the delegate body ran, skipping cleanup that executed pre-Re-check amortized constraints at interpreter/host-code boundaries #2713. Checks now run only after the host call returns — the post-return check alone preserves the ≤ 1-host-call detection bound.
  • Debugger watch evaluation: an engine paused at a breakpoint longer than its TimeoutInterval would deterministically fail every interop read in watch/conditional-breakpoint expressions. Debug-mode engines now keep the pre-existing amortized-countdown-only behavior.
  • Unobserved Task: the check between DynamicInvoke and promise conversion could discard a still-running Task returned by an async host delegate, surfacing its eventual fault as TaskScheduler.UnobservedTaskException. Awaitable results now reach ConvertAwaitableToPromise (attaching the continuation) before the check.
  • Constraint-exception laundering: the checks inside MethodDescriptor.Call sat inside operator-overload resolution's catch-all, which rethrows as TargetInvocationException — embedder sandbox handlers catching ExecutionCanceledException/TimeoutException would miss. The constructor-lane check moved to TypeReference.Construct, where it now also covers the Options.Interop.CreateTypeReferenceObject factory and Activator value-type branches.

Coverage extended

The #2707 deferred-detection gap also existed in lanes #2713 did not instrument; now covered:

  • Dictionary-shaped interop (TryGetDictionaryValue/TrySetDictionaryValue on wrapped IDictionary<,> — e.g. a service-backed dictionary with slow lookups)
  • Wrapped-IEnumerable iteration (MoveNext in for-of)
  • The Options.Interop.MemberAccessor callback

Also removed the redundant GetterFunction/SetterFunction checks (both consumers either re-check inside ReflectionAccessor or run no host code).

Still intentionally uninstrumented: built-in ClrFunction dispatch (per-call cost on hot built-ins; built-ins self-check via Engine.ConstraintCheckInterval), operator-overload resolution, and user-supplied converter/factory callbacks.

Tests

Full Jint.Tests, Jint.Tests.PublicInterface, Jint.Tests.CommonScripts and Test262 suites pass on net10.0 and net472. InteropLambdaBenchmark.ForLoop A/B (including the newly covered Dictionary lane) shows no regression — all rows within variance, allocations byte-identical.

🤖 Generated with Claude Code

Review fixes for the boundary-check mechanism:

- Only check while script evaluation is active and not in debug mode:
  constraint state is meaningless outside an Execute/Invoke window (the
  time constraint's CTS is re-armed after every run and keeps counting,
  and a token cancelled during teardown must not make host-side C# reads
  of wrapped objects throw), and a paused debugger would otherwise trip
  expired timeouts on every watch-evaluation interop read.
- Check only after the host call returns, never on entry: an entry check
  would block host cleanup calls (e.g. inside a JS finally block) once
  cancellation is pending.
- Convert awaitable delegate results to a promise before the check so an
  in-flight Task is never dropped unobserved.
- Move the constructor-lane check from MethodDescriptor.Call (where
  operator-overload resolution's catch-all would launder constraint
  exceptions into TargetInvocationException) to TypeReference.Construct,
  now also covering the user-factory and Activator creation branches.
- Cover the dictionary interop lane (TryGetDictionaryValue /
  TrySetDictionaryValue), the wrapped-IEnumerable iterator lane and the
  Options.Interop.MemberAccessor callback.
- Drop the redundant GetterFunction/SetterFunction checks (both consumers
  either re-check in ReflectionAccessor or run no host code).
- Replace the CTS-timer-dependent timeout test with a deterministic
  lone-call test isolating the post-invoke check, add host-side/debug-mode
  regression tests and dictionary/iterator lane tests, dedupe the test
  scaffolding, and deduplicate the design rationale docs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lahma
lahma enabled auto-merge (squash) July 20, 2026 19:25
@lahma
lahma merged commit 8f31efe into sebastienros:main Jul 20, 2026
4 checks passed
@lahma
lahma deleted the amortized-host-boundary-hardening branch July 20, 2026 20:29
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