From 965259b59decbecb029665b49dd91555d9b8b336 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Sun, 17 May 2026 10:03:49 +0300 Subject: [PATCH 1/4] Fix #2477: spec-compliant async suspension for await using dispose DisposeCapability.DisposeResources collapsed the three Await(...) suspension points defined by the Explicit Resource Management spec (3.d.i, 3.e.ii.1, 4.a) into a single synchronous UnwrapIfPromise. This blocked the calling thread and deadlocked the EvaluateAsync path when dispose ran inside an event-loop continuation: the outer RunAvailableContinuations held _isProcessing, the nested call from UnwrapIfPromise returned early via the re-entry guard, and the dispose-resolve callback sat in the queue until the 10s timeout. Refactor DisposeCapability into a coroutine-style state machine (BeginDispose / ContinueDispose) that exposes each spec-defined Await as a DisposeStepResult.Suspend(promise) the caller is responsible for awaiting. Drive the state machine from two integration patterns: Pattern A (JintBlockStatement) - dispose runs mid-function-body; suspend the async function on each pending dispose promise via the same machinery as a JS `await`, resume by advancing the state machine. Pattern B (function/generator/module exit) - dispose runs after the body has already finished; chain Promise.then handlers via a new DisposeResourcesHelper.DisposeAndThen so the function/generator's return promise is settled only when the full dispose chain has completed. Pattern B applies to: - JintFunctionDefinition.AsyncBlockStart - JintAwaitExpression.AsyncFunctionResume - AsyncGeneratorInstance (both ResumeExecution branches) - SourceTextModule top-level-await execution The legacy DisposeResources(Completion) wrapper still drives the state machine via UnwrapIfPromise for the remaining sync-only callers, where no async-dispose resource can occur (sync syntax). Tests cover: the user's repro (EvaluateAsync + Task), genuinely-async DisposeAsync via Task.Delay, LIFO ordering across multiple await using, nested async functions, SuppressedError chaining, dispose at async function exit, and dispose after unhandled throw. Co-Authored-By: Claude Opus 4.7 (1M context) --- Jint.Tests/Runtime/InteropDisposeTests.cs | 203 ++++++++++++++++ .../AsyncGenerator/AsyncGeneratorInstance.cs | 149 +++++++----- Jint/Native/Disposable/DisposeCapability.cs | 230 ++++++++++++++---- .../Disposable/DisposeResourcesHelper.cs | 60 +++++ .../Environments/DeclarativeEnvironment.cs | 15 +- Jint/Runtime/Environments/Environment.cs | 17 ++ .../Expressions/JintAwaitExpression.cs | 49 ++-- .../Interpreter/JintFunctionDefinition.cs | 60 +++-- .../Statements/JintBlockStatement.cs | 190 +++++++++++---- Jint/Runtime/Modules/SourceTextModule.cs | 56 +++-- Jint/Runtime/SuspendData.cs | 7 +- 11 files changed, 794 insertions(+), 242 deletions(-) create mode 100644 Jint/Native/Disposable/DisposeResourcesHelper.cs diff --git a/Jint.Tests/Runtime/InteropDisposeTests.cs b/Jint.Tests/Runtime/InteropDisposeTests.cs index 76aeb1ddb8..f394e85df5 100644 --- a/Jint.Tests/Runtime/InteropDisposeTests.cs +++ b/Jint.Tests/Runtime/InteropDisposeTests.cs @@ -1,3 +1,5 @@ +using Jint.Runtime; + namespace Jint.Tests.Runtime; public class InteropDisposeTests @@ -50,6 +52,170 @@ public void ShouldAsyncDispose(string program) _asyncDisposable.Disposed.Should().BeTrue(); } + /// + /// Regression test for https://github.com/sebastienros/jint/issues/2477: + /// EvaluateAsync + IAsyncDisposable obtained via an awaited Task must complete, + /// not deadlock. Previously hung for 10s and threw a timeout PromiseRejectedException. + /// + [Fact] + public async Task ShouldAsyncDisposeAfterAwait_EvaluateAsync() + { + var engine = new Engine(o => o.ExperimentalFeatures = ExperimentalFeature.TaskInterop); + var disposable = new AsyncDisposable(); + engine.SetValue("makeDisposable", new Func>(() => + Task.FromResult(disposable))); + + await engine.EvaluateAsync(""" + (async () => { + await using d = await makeDisposable(); + })() + """); + + disposable.Disposed.Should().BeTrue(); + } + + /// + /// Confirms the state machine truly suspends and resumes — not just that + /// synchronously-completed ValueTasks happen to work. Uses Task.Delay so + /// the dispose await crosses a real async boundary. + /// + [Fact] + public async Task ShouldAsyncDisposeWithGenuinelyAsyncTask() + { + var engine = new Engine(o => o.ExperimentalFeatures = ExperimentalFeature.TaskInterop); + var disposable = new DelayedAsyncDisposable(); + engine.SetValue("makeDisposable", new Func>(() => + Task.FromResult(disposable))); + + await engine.EvaluateAsync(""" + (async () => { + await using d = await makeDisposable(); + })() + """); + + disposable.Disposed.Should().BeTrue(); + } + + /// + /// Two await-using declarations in the same block dispose LIFO across two suspensions. + /// + [Fact] + public async Task ShouldAsyncDisposeMultipleResourcesLifoOrder() + { + var engine = new Engine(o => o.ExperimentalFeatures = ExperimentalFeature.TaskInterop); + var order = new List(); + engine.SetValue("makeA", new Func(() => new TrackedAsyncDisposable("A", order))); + engine.SetValue("makeB", new Func(() => new TrackedAsyncDisposable("B", order))); + + await engine.EvaluateAsync(""" + (async () => { + await using a = makeA(); + await using b = makeB(); + })() + """); + + order.Should().Equal("B", "A"); + } + + /// + /// Nested async functions: dispose suspension must work across function boundaries. + /// + [Fact] + public async Task ShouldAsyncDisposeNestedAcrossAsyncFunctions() + { + var engine = new Engine(o => o.ExperimentalFeatures = ExperimentalFeature.TaskInterop); + var disposable = new AsyncDisposable(); + engine.SetValue("makeDisposable", new Func>(() => + Task.FromResult(disposable))); + + await engine.EvaluateAsync(""" + async function inner() { + await using d = await makeDisposable(); + } + async function outer() { + await inner(); + } + outer(); + """); + + disposable.Disposed.Should().BeTrue(); + } + + /// + /// Body throws AND dispose rejects: completion must be a SuppressedError per spec. + /// + [Fact] + public async Task ShouldAsyncDisposeChainsSuppressedError() + { + var engine = new Engine(o => o.ExperimentalFeatures = ExperimentalFeature.TaskInterop); + var disposable = new FaultingAsyncDisposable(); + engine.SetValue("makeDisposable", new Func(() => disposable)); + + var error = await Assert.ThrowsAsync(async () => + { + await engine.EvaluateAsync(""" + (async () => { + await using d = makeDisposable(); + throw new Error("body error"); + })() + """); + }); + + // The rejected value should be a SuppressedError. .error is the (later) dispose + // failure — a .NET AggregateException wrapper since the Task was faulted, so its + // message starts with "One or more errors occurred." and contains "dispose fault". + // .suppressed is the (earlier) body error with the verbatim message. + var rejected = error.RejectedValue.AsObject(); + rejected.Get("name").AsString().Should().Be("SuppressedError"); + rejected.Get("error").AsObject().Get("message").AsString().Should().Contain("dispose fault"); + rejected.Get("suppressed").AsObject().Get("message").AsString().Should().Be("body error"); + } + + /// + /// await using at top level of an async function body — exercises + /// JintFunctionDefinition.AsyncBlockStart's Pattern B dispose path. + /// + [Fact] + public async Task ShouldAsyncDisposeAtAsyncFunctionExit() + { + var engine = new Engine(o => o.ExperimentalFeatures = ExperimentalFeature.TaskInterop); + var disposable = new AsyncDisposable(); + engine.SetValue("getAsync", () => disposable); + + await engine.EvaluateAsync(""" + (async function () { + await using d = getAsync(); + })() + """); + + disposable.Disposed.Should().BeTrue(); + } + + /// + /// Async function body throws — dispose must still run, and the resulting + /// rejection must carry the original throw (no SuppressedError since dispose succeeds). + /// + [Fact] + public async Task ShouldAsyncDisposeAfterUnhandledThrow() + { + var engine = new Engine(o => o.ExperimentalFeatures = ExperimentalFeature.TaskInterop); + var disposable = new AsyncDisposable(); + engine.SetValue("makeDisposable", new Func(() => disposable)); + + var error = await Assert.ThrowsAsync(async () => + { + await engine.EvaluateAsync(""" + (async () => { + await using d = makeDisposable(); + throw new Error("body error"); + })() + """); + }); + + error.RejectedValue.AsObject().Get("message").AsString().Should().Be("body error"); + disposable.Disposed.Should().BeTrue(); + } + private class AsyncDisposable : IAsyncDisposable { public bool Disposed { get; private set; } @@ -60,5 +226,42 @@ public ValueTask DisposeAsync() return default; } } + + private class DelayedAsyncDisposable : IAsyncDisposable + { + public bool Disposed { get; private set; } + + public async ValueTask DisposeAsync() + { + await Task.Delay(20).ConfigureAwait(false); + Disposed = true; + } + } + + private class TrackedAsyncDisposable : IAsyncDisposable + { + private readonly string _name; + private readonly List _order; + + public TrackedAsyncDisposable(string name, List order) + { + _name = name; + _order = order; + } + + public ValueTask DisposeAsync() + { + _order.Add(_name); + return default; + } + } + + private class FaultingAsyncDisposable : IAsyncDisposable + { + public ValueTask DisposeAsync() + { + return new ValueTask(Task.FromException(new InvalidOperationException("dispose fault"))); + } + } #endif } diff --git a/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs b/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs index 3a3952dc1d..a0b623d27a 100644 --- a/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs +++ b/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs @@ -1,3 +1,4 @@ +using Jint.Native.Disposable; using Jint.Native.Iterator; using Jint.Native.Object; using Jint.Native.Promise; @@ -261,13 +262,19 @@ private void ResumeExecution(PromiseCapability promiseCapability) catch (JavaScriptException ex) { var env = _engine.ExecutionContext.LexicalEnvironment; - var disposeResult = env.DisposeResources(new Completion(CompletionType.Throw, ex.Error, null!)); - _engine.LeaveExecutionContext(); - _currentPromiseCapability = null; - _asyncGeneratorState = AsyncGeneratorState.Completed; - _completedAwaits = null; - AsyncGeneratorReject(disposeResult.Value, promiseCapability); - AsyncGeneratorResumeNext(); + DisposeResourcesHelper.DisposeAndThen( + _engine, + env, + new Completion(CompletionType.Throw, ex.Error, null!), + final => + { + _engine.LeaveExecutionContext(); + _currentPromiseCapability = null; + _asyncGeneratorState = AsyncGeneratorState.Completed; + _completedAwaits = null; + AsyncGeneratorReject(final.Value, promiseCapability); + AsyncGeneratorResumeNext(); + }); return; } @@ -291,45 +298,46 @@ private void ResumeExecution(PromiseCapability promiseCapability) return; } - // Generator body completed — dispose resources before leaving context + // Generator body completed — dispose resources before leaving context. var lexEnv = _engine.ExecutionContext.LexicalEnvironment; - result = lexEnv.DisposeResources(result); - - _engine.LeaveExecutionContext(); + DisposeResourcesHelper.DisposeAndThen(_engine, lexEnv, result, final => + { + _engine.LeaveExecutionContext(); - _currentPromiseCapability = null; + _currentPromiseCapability = null; - if (result.Type == CompletionType.Throw) - { - _asyncGeneratorState = AsyncGeneratorState.Completed; - _completedAwaits = null; - AsyncGeneratorReject(result.Value, promiseCapability); - } - else if (result.Type == CompletionType.Return) - { - // Per spec 13.10.1: "return;" (no expression) does NOT await the return value, - // but "return expr;" does call Await(exprValue) before returning. - // When the return statement has no argument, we can resolve directly. - if (result._source is ReturnStatement { Argument: null }) + if (final.Type == CompletionType.Throw) { _asyncGeneratorState = AsyncGeneratorState.Completed; _completedAwaits = null; - AsyncGeneratorResolve(result.Value, true, promiseCapability); + AsyncGeneratorReject(final.Value, promiseCapability); } - else + else if (final.Type == CompletionType.Return) { - _asyncGeneratorState = AsyncGeneratorState.AwaitingReturn; - AsyncGeneratorAwaitReturn(result.Value, promiseCapability); + // Per spec 13.10.1: "return;" (no expression) does NOT await the return value, + // but "return expr;" does call Await(exprValue) before returning. + // When the return statement has no argument, we can resolve directly. + if (final._source is ReturnStatement { Argument: null }) + { + _asyncGeneratorState = AsyncGeneratorState.Completed; + _completedAwaits = null; + AsyncGeneratorResolve(final.Value, true, promiseCapability); + } + else + { + _asyncGeneratorState = AsyncGeneratorState.AwaitingReturn; + AsyncGeneratorAwaitReturn(final.Value, promiseCapability); + } + } + else if (final.Type == CompletionType.Normal) + { + _asyncGeneratorState = AsyncGeneratorState.Completed; + _completedAwaits = null; + AsyncGeneratorResolve(Undefined, true, promiseCapability); } - } - else if (result.Type == CompletionType.Normal) - { - _asyncGeneratorState = AsyncGeneratorState.Completed; - _completedAwaits = null; - AsyncGeneratorResolve(Undefined, true, promiseCapability); - } - AsyncGeneratorResumeNext(); + AsyncGeneratorResumeNext(); + }); } /// @@ -451,48 +459,55 @@ internal void ResumeAfterDelegation(JsValue returnValue, PromiseCapability promi return; } - // Generator completed — dispose resources before leaving context + // Generator completed — dispose resources before leaving context. var lexEnv = _engine.ExecutionContext.LexicalEnvironment; - bodyResult = lexEnv.DisposeResources(bodyResult); - - _engine.LeaveExecutionContext(); + DisposeResourcesHelper.DisposeAndThen(_engine, lexEnv, bodyResult, final => + { + _engine.LeaveExecutionContext(); - _currentPromiseCapability = null; + _currentPromiseCapability = null; - if (bodyResult.Type == CompletionType.Throw) - { - _asyncGeneratorState = AsyncGeneratorState.Completed; - _completedAwaits = null; - if (promiseCapability is not null) + if (final.Type == CompletionType.Throw) { - AsyncGeneratorReject(bodyResult.Value, promiseCapability); + _asyncGeneratorState = AsyncGeneratorState.Completed; + _completedAwaits = null; + if (promiseCapability is not null) + { + AsyncGeneratorReject(final.Value, promiseCapability); + } } - } - else - { - _asyncGeneratorState = AsyncGeneratorState.Completed; - _completedAwaits = null; - if (promiseCapability is not null) + else { - var completionValue = bodyResult.Type == CompletionType.Return ? bodyResult.Value : Undefined; - AsyncGeneratorResolve(completionValue, true, promiseCapability); + _asyncGeneratorState = AsyncGeneratorState.Completed; + _completedAwaits = null; + if (promiseCapability is not null) + { + var completionValue = final.Type == CompletionType.Return ? final.Value : Undefined; + AsyncGeneratorResolve(completionValue, true, promiseCapability); + } } - } - AsyncGeneratorResumeNext(); + AsyncGeneratorResumeNext(); + }); } catch (JavaScriptException ex) { var env = _engine.ExecutionContext.LexicalEnvironment; - var disposeResult = env.DisposeResources(new Completion(CompletionType.Throw, ex.Error, null!)); - _engine.LeaveExecutionContext(); - _currentPromiseCapability = null; - _asyncGeneratorState = AsyncGeneratorState.Completed; - _completedAwaits = null; - if (promiseCapability is not null) - { - AsyncGeneratorReject(disposeResult.Value, promiseCapability); - } - AsyncGeneratorResumeNext(); + DisposeResourcesHelper.DisposeAndThen( + _engine, + env, + new Completion(CompletionType.Throw, ex.Error, null!), + final => + { + _engine.LeaveExecutionContext(); + _currentPromiseCapability = null; + _asyncGeneratorState = AsyncGeneratorState.Completed; + _completedAwaits = null; + if (promiseCapability is not null) + { + AsyncGeneratorReject(final.Value, promiseCapability); + } + AsyncGeneratorResumeNext(); + }); } } diff --git a/Jint/Native/Disposable/DisposeCapability.cs b/Jint/Native/Disposable/DisposeCapability.cs index f630befc38..e62e4e6bfd 100644 --- a/Jint/Native/Disposable/DisposeCapability.cs +++ b/Jint/Native/Disposable/DisposeCapability.cs @@ -1,17 +1,45 @@ +using System.Runtime.InteropServices; using Jint.Runtime; namespace Jint.Native.Disposable; +/// +/// Result of a single step of 's state machine. +/// Either the dispose finished ( with ) +/// or it needs to await a promise () before continuing. +/// +[StructLayout(LayoutKind.Auto)] +internal readonly record struct DisposeStepResult( + bool IsDone, + Completion CompletedResult, + JsValue? PendingPromise) +{ + public static DisposeStepResult Done(Completion c) => new(true, c, null); + public static DisposeStepResult Suspend(JsValue promise) => new(false, default, promise); +} + +/// +/// Tracks where in the spec algorithm we suspended, so the next +/// call applies the right rules. +/// +internal enum DisposeResumePoint +{ + None, + AfterMethodAwait, // resumed after step 3.e.ii.1 + AfterImplicitAwait, // resumed after step 3.d.i or 4.a +} + internal sealed class DisposeCapability { private readonly Engine _engine; private readonly List _disposableResourceStack = []; - /// - /// Set to true after DisposeResources if an async-dispose resource with no method - /// was encountered, indicating the caller should introduce an Await tick per spec. - /// - public bool NeedsAsyncTick { get; private set; } + // State preserved across suspensions (3.e.ii.1, 3.d.i, 4.a). + private int _disposeIndex; + private bool _disposeHasAwaited; + private bool _disposeNeedsAwait; + private Completion _disposeCompletion; + private DisposeResumePoint _disposeResume; public DisposeCapability(Engine engine) { @@ -66,80 +94,184 @@ private DisposableResource CreateDisposableResource(JsValue v, DisposeHint hint, return new DisposableResource(v, hint, method); } + /// + /// Legacy synchronous entry point. Drives the state machine to completion, + /// sync-waiting via + /// on every suspension. Used by callers that can't suspend their own context + /// (sync function bodies, module loading). After all async-context call sites + /// migrated to / , this + /// only runs for contexts where await-using is syntactically illegal, so no + /// async-dispose resource can actually be in the stack and the loop exits in + /// a single Done step without ever calling UnwrapIfPromise. + /// public Completion DisposeResources(Completion c) { - var needsAwait = false; - var hasAwaited = false; + var step = BeginDispose(c); + while (!step.IsDone) + { + var pending = step.PendingPromise!; + JsValue resolved; + bool threw = false; + try + { + resolved = pending.UnwrapIfPromise(_engine.Options.Constraints.PromiseTimeout); + } + catch (PromiseRejectedException e) + { + resolved = e.RejectedValue; + threw = true; + } + catch (JavaScriptException e) + { + resolved = e.Error; + threw = true; + } + step = ContinueDispose(resolved, threw); + } + return step.CompletedResult; + } + + /// + /// Initialize the dispose state machine and run until the first suspension or completion. + /// + public DisposeStepResult BeginDispose(Completion c) + { + _disposeIndex = _disposableResourceStack.Count - 1; + _disposeHasAwaited = false; + _disposeNeedsAwait = false; + _disposeCompletion = c; + _disposeResume = DisposeResumePoint.None; + return Advance(); + } + + /// + /// Continue the dispose state machine after a suspended Await settles. + /// + public DisposeStepResult ContinueDispose(JsValue awaitResult, bool awaitThrew) + { + switch (_disposeResume) + { + case DisposeResumePoint.AfterMethodAwait: + // Spec step 3.e.ii.1 just settled. Promote rejection per 3.e.iii. + if (awaitThrew) + { + HandleDisposeException(awaitResult); + } + _disposeIndex--; + break; + + case DisposeResumePoint.AfterImplicitAwait: + // Spec step 3.d.i or 4.a — Await(undefined) has no error path. + if (_disposeIndex < 0) + { + // We were on step 4.a (final implicit tick). + return Finish(); + } + // Otherwise we were on step 3.d.i — fall through to process + // the current resource (no decrement). + break; + } - for (var i = _disposableResourceStack.Count - 1; i >= 0; i--) + _disposeResume = DisposeResumePoint.None; + return Advance(); + } + + private DisposeStepResult Advance() + { + while (_disposeIndex >= 0) { - var (value, hint, method) = _disposableResourceStack[i]; + var resource = _disposableResourceStack[_disposeIndex]; + var value = resource.ResourceValue; + var hint = resource.Hint; + var method = resource.DisposeMethod; - if (hint == DisposeHint.Sync && needsAwait && !hasAwaited) + // Step 3.d: implicit Await tick when a sync-dispose follows pending async resources. + if (hint == DisposeHint.Sync && _disposeNeedsAwait && !_disposeHasAwaited) { - _engine.RunAvailableContinuations(); - needsAwait = false; + _disposeHasAwaited = true; + _disposeNeedsAwait = false; + _disposeResume = DisposeResumePoint.AfterImplicitAwait; + // Don't decrement — current resource will be processed after resume. + return DisposeStepResult.Suspend(CreateResolvedUndefinedPromise()); } + // Step 3.e: call the dispose method. if (method is not null) { - var result = JsValue.Undefined; - JavaScriptException? exception = null; + JsValue methodResult; try { - result = method.Call(value); - if (hint == DisposeHint.Async) - { - hasAwaited = true; - try - { - result = result.UnwrapIfPromise(_engine.Options.Constraints.PromiseTimeout); - } - catch (PromiseRejectedException e) - { - exception = new JavaScriptException(e.RejectedValue); - } - catch (JavaScriptException e) - { - exception = e; - } - } + methodResult = method.Call(value); } catch (JavaScriptException e) { - exception = e; + HandleDisposeException(e.Error); + _disposeIndex--; + continue; + } + catch (PromiseRejectedException e) + { + HandleDisposeException(e.RejectedValue); + _disposeIndex--; + continue; } - if (exception is not null) + if (hint == DisposeHint.Async) { - if (c.Type == CompletionType.Throw) - { - var error = _engine.Intrinsics.SuppressedError.Construct(_engine.Intrinsics.SuppressedError, "", exception.Error, c.Value); - c = new Completion(CompletionType.Throw, error, c._source); - } - else - { - c = new Completion(CompletionType.Throw, exception.Error, c._source); - } + // Step 3.e.ii.1: Await the result before continuing. + _disposeHasAwaited = true; + _disposeResume = DisposeResumePoint.AfterMethodAwait; + return DisposeStepResult.Suspend(methodResult); } } else { - // Assert: hint is "async-dispose" - needsAwait = true; + // Step 3.f: null/undefined async-dispose resource — accumulates needsAwait. + _disposeNeedsAwait = true; } + + _disposeIndex--; } - if (needsAwait && !hasAwaited) + // Step 4: final implicit Await(undefined) if needsAwait was set but never satisfied. + if (_disposeNeedsAwait && !_disposeHasAwaited) { - NeedsAsyncTick = true; - _engine.RunAvailableContinuations(); + _disposeHasAwaited = true; + _disposeNeedsAwait = false; + _disposeResume = DisposeResumePoint.AfterImplicitAwait; + // _disposeIndex is already -1; ContinueDispose will Finish on resume. + return DisposeStepResult.Suspend(CreateResolvedUndefinedPromise()); } + return Finish(); + } + + private DisposeStepResult Finish() + { + var result = _disposeCompletion; _disposableResourceStack.Clear(); - return c; + _disposeResume = DisposeResumePoint.None; + return DisposeStepResult.Done(result); + } + + private void HandleDisposeException(JsValue error) + { + if (_disposeCompletion.Type == CompletionType.Throw) + { + var suppressed = _engine.Intrinsics.SuppressedError.Construct( + _engine.Intrinsics.SuppressedError, "", error, _disposeCompletion.Value); + _disposeCompletion = new Completion(CompletionType.Throw, suppressed, _disposeCompletion._source); + } + else + { + _disposeCompletion = new Completion(CompletionType.Throw, error, _disposeCompletion._source); + } + } + + private JsValue CreateResolvedUndefinedPromise() + { + return _engine.Realm.Intrinsics.Promise.PromiseResolve(JsValue.Undefined); } private readonly record struct DisposableResource(JsValue ResourceValue, DisposeHint Hint, ICallable? DisposeMethod); } - diff --git a/Jint/Native/Disposable/DisposeResourcesHelper.cs b/Jint/Native/Disposable/DisposeResourcesHelper.cs new file mode 100644 index 0000000000..bdb8852f34 --- /dev/null +++ b/Jint/Native/Disposable/DisposeResourcesHelper.cs @@ -0,0 +1,60 @@ +using Jint.Native.Promise; +using Jint.Runtime; +using Jint.Runtime.Descriptors; +using Jint.Runtime.Interop; +using Environment = Jint.Runtime.Environments.Environment; + +namespace Jint.Native.Disposable; + +/// +/// Drives 's state machine for call sites where the +/// async function/generator body has already finished — so we don't need to suspend +/// the function itself, only defer settlement of its return promise. Each +/// state-machine Suspend is chained via Promise.then, and the caller-provided +/// continueWith callback fires when the full dispose chain has completed. +/// +internal static class DisposeResourcesHelper +{ + public static void DisposeAndThen( + Engine engine, + Environment env, + Completion initial, + Action continueWith) + { + var step = env.BeginDisposeResources(initial); + DriveAsync(engine, env, step, continueWith); + } + + private static void DriveAsync( + Engine engine, + Environment env, + DisposeStepResult step, + Action continueWith) + { + while (!step.IsDone) + { + var pending = step.PendingPromise!; + var promise = pending as JsPromise + ?? (JsPromise) engine.Realm.Intrinsics.Promise.PromiseResolve(pending); + + var onFulfilled = new ClrFunction(engine, "", (_, args) => + { + var next = env.ContinueDisposeResources(args.At(0), awaitThrew: false); + DriveAsync(engine, env, next, continueWith); + return JsValue.Undefined; + }, 1, PropertyFlag.Configurable); + + var onRejected = new ClrFunction(engine, "", (_, args) => + { + var next = env.ContinueDisposeResources(args.At(0), awaitThrew: true); + DriveAsync(engine, env, next, continueWith); + return JsValue.Undefined; + }, 1, PropertyFlag.Configurable); + + PromiseOperations.PerformPromiseThen(engine, promise, onFulfilled, onRejected, null!); + return; // suspended — onFulfilled/onRejected will continue the drive + } + + continueWith(step.CompletedResult); + } +} diff --git a/Jint/Runtime/Environments/DeclarativeEnvironment.cs b/Jint/Runtime/Environments/DeclarativeEnvironment.cs index a613a35432..610575eca7 100644 --- a/Jint/Runtime/Environments/DeclarativeEnvironment.cs +++ b/Jint/Runtime/Environments/DeclarativeEnvironment.cs @@ -343,10 +343,19 @@ internal sealed override string[] GetAllBindingNames() internal sealed override Completion DisposeResources(Completion c) => _disposeCapability?.DisposeResources(c) ?? c; /// - /// True if the last DisposeResources call encountered an async-dispose resource - /// with no method (null/undefined value), requiring an implicit Await tick per spec. + /// Begin dispose via the state machine. Returns either Done with the final Completion, + /// or Suspend with a Promise the caller must await before calling + /// . If no resources are registered, returns + /// Done immediately. /// - internal bool NeedsAsyncDisposeTick => _disposeCapability?.NeedsAsyncTick == true; + internal sealed override DisposeStepResult BeginDisposeResources(Completion c) + => _disposeCapability?.BeginDispose(c) ?? DisposeStepResult.Done(c); + + /// + /// Continue dispose after a suspended Await settles. Returns either Done or the next Suspend. + /// + internal sealed override DisposeStepResult ContinueDisposeResources(JsValue awaitResult, bool awaitThrew) + => _disposeCapability!.ContinueDispose(awaitResult, awaitThrew); public void Clear() { diff --git a/Jint/Runtime/Environments/Environment.cs b/Jint/Runtime/Environments/Environment.cs index cc72f06a6c..a54e0fec83 100644 --- a/Jint/Runtime/Environments/Environment.cs +++ b/Jint/Runtime/Environments/Environment.cs @@ -1,6 +1,7 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using Jint.Native; +using Jint.Native.Disposable; namespace Jint.Runtime.Environments; @@ -109,6 +110,22 @@ public override bool Equals(JsValue? other) internal virtual Completion DisposeResources(Completion c) => c; + /// + /// Begin dispose via the state machine. Default returns Done immediately for + /// environments that don't track disposable resources. + /// + internal virtual DisposeStepResult BeginDisposeResources(Completion c) => DisposeStepResult.Done(c); + + /// + /// Continue dispose after an awaited promise settled. Must not be called on + /// an environment that didn't return a Suspend from BeginDisposeResources. + /// + internal virtual DisposeStepResult ContinueDisposeResources(JsValue awaitResult, bool awaitThrew) + { + Throw.InvalidOperationException("ContinueDisposeResources called without a pending Suspend."); + return default; + } + /// /// Helper to cache JsString/Key when environments use different lookups. /// diff --git a/Jint/Runtime/Interpreter/Expressions/JintAwaitExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintAwaitExpression.cs index 39009c9f31..78d18a66a8 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintAwaitExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintAwaitExpression.cs @@ -1,6 +1,7 @@ using Jint.Native; using Jint.Native.AsyncFunction; using Jint.Native.AsyncGenerator; +using Jint.Native.Disposable; using Jint.Native.Promise; using Jint.Runtime.Descriptors; using Jint.Runtime.Interop; @@ -242,10 +243,16 @@ internal static void AsyncFunctionResume(Engine engine, AsyncFunctionInstance as catch (JavaScriptException e) { var env = engine.ExecutionContext.LexicalEnvironment; - var disposeResult = env.DisposeResources(new Completion(CompletionType.Throw, e.Error, null!)); - engine.LeaveExecutionContext(); - asyncInstance._state = AsyncFunctionState.Completed; - asyncInstance._capability.Reject.Call(JsValue.Undefined, disposeResult.Value); + DisposeResourcesHelper.DisposeAndThen( + engine, + env, + new Completion(CompletionType.Throw, e.Error, null!), + final => + { + engine.LeaveExecutionContext(); + asyncInstance._state = AsyncFunctionState.Completed; + asyncInstance._capability.Reject.Call(JsValue.Undefined, final.Value); + }); return; } @@ -256,23 +263,23 @@ internal static void AsyncFunctionResume(Engine engine, AsyncFunctionInstance as } var lexEnv = engine.ExecutionContext.LexicalEnvironment; - result = lexEnv.DisposeResources(result); - - engine.LeaveExecutionContext(); - - asyncInstance._state = AsyncFunctionState.Completed; - - if (result.Type == CompletionType.Throw) + DisposeResourcesHelper.DisposeAndThen(engine, lexEnv, result, final => { - asyncInstance._capability.Reject.Call(JsValue.Undefined, result.Value); - } - else if (result.Type == CompletionType.Return) - { - asyncInstance._capability.Resolve.Call(JsValue.Undefined, result.Value); - } - else - { - asyncInstance._capability.Resolve.Call(JsValue.Undefined, JsValue.Undefined); - } + engine.LeaveExecutionContext(); + asyncInstance._state = AsyncFunctionState.Completed; + + if (final.Type == CompletionType.Throw) + { + asyncInstance._capability.Reject.Call(JsValue.Undefined, final.Value); + } + else if (final.Type == CompletionType.Return) + { + asyncInstance._capability.Resolve.Call(JsValue.Undefined, final.Value); + } + else + { + asyncInstance._capability.Resolve.Call(JsValue.Undefined, JsValue.Undefined); + } + }); } } diff --git a/Jint/Runtime/Interpreter/JintFunctionDefinition.cs b/Jint/Runtime/Interpreter/JintFunctionDefinition.cs index 4759bb8d2a..b25f7d3008 100644 --- a/Jint/Runtime/Interpreter/JintFunctionDefinition.cs +++ b/Jint/Runtime/Interpreter/JintFunctionDefinition.cs @@ -3,6 +3,7 @@ using Jint.Native.Function; using Jint.Native.AsyncFunction; using Jint.Native.AsyncGenerator; +using Jint.Native.Disposable; using Jint.Native.Generator; using Jint.Native.Promise; using Jint.Runtime.Environments; @@ -177,11 +178,18 @@ private static void AsyncBlockStart( } catch (JavaScriptException e) { - // Per spec: DisposeResources before rejecting + // Per spec: DisposeResources before rejecting. Use the helper so async-dispose + // resources are awaited via the state machine instead of sync-blocking. var env = engine.ExecutionContext.LexicalEnvironment; - var disposeResult = env.DisposeResources(new Completion(CompletionType.Throw, e.Error, null!)); - asyncInstance._state = AsyncFunctionState.Completed; - asyncInstance._capability.Reject.Call(JsValue.Undefined, disposeResult.Value); + DisposeResourcesHelper.DisposeAndThen( + engine, + env, + new Completion(CompletionType.Throw, e.Error, null!), + final => + { + asyncInstance._state = AsyncFunctionState.Completed; + asyncInstance._capability.Reject.Call(JsValue.Undefined, final.Value); + }); return; } @@ -193,29 +201,31 @@ private static void AsyncBlockStart( return; } - // Per spec AsyncBlockStart step 3.f: DisposeResources after body completes + // Per spec AsyncBlockStart step 3.f: DisposeResources after body completes. + // Settlement of the function's return promise is deferred until the dispose chain + // (which may itself await) finishes. var lexEnv = engine.ExecutionContext.LexicalEnvironment; - result = lexEnv.DisposeResources(result); - - // Completed - resolve or reject the async function's return promise - asyncInstance._state = AsyncFunctionState.Completed; - - if (result.Type == CompletionType.Throw) - { - asyncInstance._capability.Reject.Call(JsValue.Undefined, result.Value); - } - else if (result.Type == CompletionType.Normal) - { - asyncInstance._capability.Resolve.Call(JsValue.Undefined, JsValue.Undefined); - } - else if (result.Type == CompletionType.Return) - { - asyncInstance._capability.Resolve.Call(JsValue.Undefined, result.Value); - } - else + DisposeResourcesHelper.DisposeAndThen(engine, lexEnv, result, final => { - asyncInstance._capability.Reject.Call(JsValue.Undefined, result.Value); - } + asyncInstance._state = AsyncFunctionState.Completed; + + if (final.Type == CompletionType.Throw) + { + asyncInstance._capability.Reject.Call(JsValue.Undefined, final.Value); + } + else if (final.Type == CompletionType.Normal) + { + asyncInstance._capability.Resolve.Call(JsValue.Undefined, JsValue.Undefined); + } + else if (final.Type == CompletionType.Return) + { + asyncInstance._capability.Resolve.Call(JsValue.Undefined, final.Value); + } + else + { + asyncInstance._capability.Reject.Call(JsValue.Undefined, final.Value); + } + }); } /// diff --git a/Jint/Runtime/Interpreter/Statements/JintBlockStatement.cs b/Jint/Runtime/Interpreter/Statements/JintBlockStatement.cs index 475ca07a16..9e19ebccf2 100644 --- a/Jint/Runtime/Interpreter/Statements/JintBlockStatement.cs +++ b/Jint/Runtime/Interpreter/Statements/JintBlockStatement.cs @@ -1,5 +1,7 @@ using System.Threading; using Jint.Native; +using Jint.Native.AsyncFunction; +using Jint.Native.Disposable; using Jint.Native.Promise; using Jint.Runtime.Descriptors; using Jint.Runtime.Environments; @@ -102,20 +104,13 @@ public Completion ExecuteBlock(EvaluationContext context) Completion blockValue; - // If resuming from a disposal-caused suspension (await-using with null/undefined), - // skip re-executing the body — the block body already completed, we're just - // resuming after the implicit Await tick from DisposeResources. + // Resuming from a dispose-driven suspension: don't re-execute the body — advance + // the dispose state machine from where it suspended. if (suspendable is { IsResuming: true } && suspendable.Data.TryGet(this, out BlockSuspendData? disposeResumeData) - && disposeResumeData?.DisposalComplete == true) + && disposeResumeData?.DisposeInProgress == true) { - suspendable.IsResuming = false; - suspendable.Data.Clear(this); - if (oldEnv is not null) - { - engine.UpdateLexicalEnvironment(oldEnv); - } - return new Completion(CompletionType.Normal, JsValue.Undefined, _statement); + return ResumeDispose(context, blockEnv, oldEnv, disposeResumeData, suspendable); } if (_singleStatement is not null) @@ -140,62 +135,155 @@ public Completion ExecuteBlock(EvaluationContext context) } else { - // Return environment to cache for reuse (slots only exist when CanReuseEnvironment=true) - if (blockEnv._slots is not null) - { - Interlocked.Exchange(ref blockState._cachedEnv, blockEnv); - } + return CompleteDispose( + context, + blockEnv, + oldEnv, + suspendable, + blockEnv.BeginDisposeResources(blockValue)); + } + } - blockValue = blockEnv.DisposeResources(blockValue); + if (oldEnv is not null) + { + engine.UpdateLexicalEnvironment(oldEnv); + } - // Per spec Dispose step 3.a: await-using with null/undefined value requires - // an implicit Await tick. Suspend the async function to introduce the tick. - if (blockEnv.NeedsAsyncDisposeTick) - { - var asyncFn = engine.ExecutionContext.AsyncFunction; - if (asyncFn is not null) - { - var promise = (JsPromise) engine.Realm.Intrinsics.Promise.PromiseResolve(JsValue.Undefined); + return blockValue; + } - asyncFn._lastAwaitNode = this; - asyncFn._state = Native.AsyncFunction.AsyncFunctionState.SuspendedAwait; - asyncFn._savedContext = engine.ExecutionContext; + /// + /// Resume the dispose state machine after an awaited dispose promise settled. + /// + private Completion ResumeDispose( + EvaluationContext context, + DeclarativeEnvironment? blockEnv, + Environment? oldEnv, + BlockSuspendData data, + ISuspendable suspendable) + { + var engine = context.Engine; + suspendable.IsResuming = false; + + var asyncFn = engine.ExecutionContext.AsyncFunction!; + var awaitResult = asyncFn._resumeValue ?? JsValue.Undefined; + var awaitThrew = asyncFn._resumeWithThrow; + asyncFn._resumeValue = null; + asyncFn._resumeWithThrow = false; + asyncFn._isResuming = false; + asyncFn._lastAwaitNode = null; + + // Use the env captured at suspend time, in case the env-setup branch above + // didn't run on this re-entry (no declarations path). + blockEnv ??= data.BlockEnvironment!; + oldEnv ??= data.OuterEnvironment; + engine.UpdateLexicalEnvironment(blockEnv); + + return CompleteDispose( + context, + blockEnv, + oldEnv, + suspendable, + blockEnv.ContinueDisposeResources(awaitResult, awaitThrew)); + } - var onFulfilled = new ClrFunction(engine, "", (_, args) => - { - asyncFn._resumeValue = JsValue.Undefined; - asyncFn._resumeWithThrow = false; - JintAwaitExpression.AsyncFunctionResume(engine, asyncFn); - return JsValue.Undefined; - }, 1, PropertyFlag.Configurable); + /// + /// Drives the dispose state machine to completion. For each suspension, suspends + /// the surrounding async function (via the same machinery as await); when + /// the dispose promise settles, the function resumes and the block re-enters + /// with the dispose-in-progress flag set. + /// + private Completion CompleteDispose( + EvaluationContext context, + DeclarativeEnvironment blockEnv, + Environment? oldEnv, + ISuspendable? suspendable, + DisposeStepResult step) + { + var engine = context.Engine; - PromiseOperations.PerformPromiseThen(engine, promise, onFulfilled, null!, null!); + while (!step.IsDone) + { + var pending = step.PendingPromise!; + var asyncFn = engine.ExecutionContext.AsyncFunction; - if (suspendable is not null) - { - var data = suspendable.Data.GetOrCreate(this); - data.BlockEnvironment = blockEnv; - data.OuterEnvironment = oldEnv; - data.DisposalComplete = true; - } - if (oldEnv is not null) - { - engine.UpdateLexicalEnvironment(oldEnv); - } - return blockValue; - } + if (asyncFn is null) + { + // Non-async context fallback. `await using` is syntactically only valid + // in async, so this path is reached only by unusual hosts. Sync wait. + JsValue resolved; + bool threw = false; + try + { + resolved = pending.UnwrapIfPromise(engine.Options.Constraints.PromiseTimeout); + } + catch (PromiseRejectedException e) + { + resolved = e.RejectedValue; + threw = true; } + step = blockEnv.ContinueDisposeResources(resolved, threw); + continue; + } + + SetupDisposeSuspension(engine, asyncFn, pending); + + var data = suspendable!.Data.GetOrCreate(this); + data.BlockEnvironment = blockEnv; + data.OuterEnvironment = oldEnv; + data.DisposeInProgress = true; - suspendable?.Data.Clear(this); + if (oldEnv is not null) + { + engine.UpdateLexicalEnvironment(oldEnv); } + return new Completion(CompletionType.Normal, JsValue.Undefined, _statement); } + // Dispose finished — finalize: cache env, restore outer env, return. + suspendable?.Data.Clear(this); + if (blockEnv._slots is not null) + { + Interlocked.Exchange(ref _blockState._cachedEnv, blockEnv); + } if (oldEnv is not null) { engine.UpdateLexicalEnvironment(oldEnv); } + return step.CompletedResult; + } - return blockValue; + /// + /// Mirror of for the dispose path: + /// suspends the async function on the pending dispose promise, with handlers that + /// resume the function (which re-enters this block via ). + /// + private void SetupDisposeSuspension(Engine engine, AsyncFunctionInstance asyncFn, JsValue pendingPromise) + { + var promise = pendingPromise as JsPromise + ?? (JsPromise) engine.Realm.Intrinsics.Promise.PromiseResolve(pendingPromise); + + asyncFn._lastAwaitNode = this; + asyncFn._state = AsyncFunctionState.SuspendedAwait; + asyncFn._savedContext = engine.ExecutionContext; + + var onFulfilled = new ClrFunction(engine, "", (_, args) => + { + asyncFn._resumeValue = args.At(0); + asyncFn._resumeWithThrow = false; + JintAwaitExpression.AsyncFunctionResume(engine, asyncFn); + return JsValue.Undefined; + }, 1, PropertyFlag.Configurable); + + var onRejected = new ClrFunction(engine, "", (_, args) => + { + asyncFn._resumeValue = args.At(0); + asyncFn._resumeWithThrow = true; + JintAwaitExpression.AsyncFunctionResume(engine, asyncFn); + return JsValue.Undefined; + }, 1, PropertyFlag.Configurable); + + PromiseOperations.PerformPromiseThen(engine, promise, onFulfilled, onRejected, null!); } private Completion ExecuteSingle(EvaluationContext context) diff --git a/Jint/Runtime/Modules/SourceTextModule.cs b/Jint/Runtime/Modules/SourceTextModule.cs index 745924ce2c..4a2525b290 100644 --- a/Jint/Runtime/Modules/SourceTextModule.cs +++ b/Jint/Runtime/Modules/SourceTextModule.cs @@ -1,6 +1,7 @@ using System.Diagnostics; using Jint.Native; using Jint.Native.AsyncFunction; +using Jint.Native.Disposable; using Jint.Native.Object; using Jint.Native.Promise; using Jint.Runtime.Environments; @@ -406,11 +407,17 @@ internal override Completion ExecuteModule(PromiseCapability? capability = null) } catch (JavaScriptException e) { - result = _environment.DisposeResources(new Completion(CompletionType.Throw, e.Error, null!)); - _engine.LeaveExecutionContext(); - _tlaAsyncInstance._state = AsyncFunctionState.Completed; - capability!.Reject.Call(JsValue.Undefined, e.Error); - return result; + DisposeResourcesHelper.DisposeAndThen( + _engine, + _environment, + new Completion(CompletionType.Throw, e.Error, null!), + final => + { + _engine.LeaveExecutionContext(); + _tlaAsyncInstance._state = AsyncFunctionState.Completed; + capability!.Reject.Call(JsValue.Undefined, final.Value); + }); + return new Completion(CompletionType.Normal, JsValue.Undefined, null!); } // Check if we suspended at an await @@ -422,26 +429,29 @@ internal override Completion ExecuteModule(PromiseCapability? capability = null) return new Completion(CompletionType.Normal, JsValue.Undefined, null!); } - result = _environment.DisposeResources(result); - _engine.LeaveExecutionContext(); - - // Completed - resolve or reject via the capability - _tlaAsyncInstance._state = AsyncFunctionState.Completed; - - if (result.Type == CompletionType.Normal) + // Module body complete - dispose any (potentially async-dispose) resources + // before settling the capability. Settlement is deferred until the dispose + // chain finishes so top-level `await using` can suspend correctly. + DisposeResourcesHelper.DisposeAndThen(_engine, _environment, result, final => { - capability!.Resolve.Call(JsValue.Undefined, JsValue.Undefined); - } - else if (result.Type == CompletionType.Throw) - { - capability!.Reject.Call(JsValue.Undefined, result.Value); - } - else - { - capability!.Resolve.Call(JsValue.Undefined, result.Value); - } + _engine.LeaveExecutionContext(); + _tlaAsyncInstance._state = AsyncFunctionState.Completed; - return result; + if (final.Type == CompletionType.Normal) + { + capability!.Resolve.Call(JsValue.Undefined, JsValue.Undefined); + } + else if (final.Type == CompletionType.Throw) + { + capability!.Reject.Call(JsValue.Undefined, final.Value); + } + else + { + capability!.Resolve.Call(JsValue.Undefined, final.Value); + } + }); + + return new Completion(CompletionType.Normal, JsValue.Undefined, null!); } } } diff --git a/Jint/Runtime/SuspendData.cs b/Jint/Runtime/SuspendData.cs index 4b99650db4..45a62885b4 100644 --- a/Jint/Runtime/SuspendData.cs +++ b/Jint/Runtime/SuspendData.cs @@ -270,10 +270,11 @@ internal sealed class BlockSuspendData : SuspendData public Jint.Runtime.Environments.Environment? OuterEnvironment { get; set; } /// - /// Whether DisposeResources has already been called for this block. - /// When true, resumption should skip disposal and just continue. + /// True while this block is suspended mid-dispose. On resume, the block + /// must not re-execute its body — it must call ContinueDisposeResources + /// to advance the dispose state machine. /// - public bool DisposalComplete { get; set; } + public bool DisposeInProgress { get; set; } } /// From e821600fe65ba5bdb125221b0cef4fcf0e6c8fb9 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Sun, 17 May 2026 10:30:33 +0300 Subject: [PATCH 2/4] Fix AsyncDisposableStack tick ordering + restore zero-allocation fast path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI on the prior commit caught 4 regressions in AsyncDisposableStack/prototype/ disposeAsync where the legacy synchronous DisposeResources path no longer drained microtasks the way the previous in-loop RunAvailableContinuations did, so disposeAsync()'s capability was settled in the same tick as it was called and the spec-required Await(undefined) tick disappeared. Address both root causes: 1. Route AsyncDisposableStack.prototype.disposeAsync through Pattern B so each spec-defined Await consumes a real microtask tick via PerformPromiseThen rather than relying on the legacy sync drain. Add a DisposeCapability overload to DisposeResourcesHelper so DisposableStack can drive its own capability without owning an Environment. 2. Avoid the closure allocation that the helper otherwise forces on every async function/generator/module exit. Expose Environment.HasDisposeResources as a fast bool, and at each Pattern B call site take a fast path that calls the settle logic directly when no disposables are registered — preserving the pre-refactor zero-allocation hot path for the common case (no using declarations in scope). Tests: full Jint.Tests (3004 pass) and the Test262 async/dispose/for-loop sweep (3237 pass, including all 38 AsyncDisposableStack_prototype_disposeAsync cases that regressed on the prior commit). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../AsyncGenerator/AsyncGeneratorInstance.cs | 179 ++++++++++-------- .../AsyncDisposableStackPrototype.cs | 27 ++- Jint/Native/Disposable/DisposableStack.cs | 23 +++ Jint/Native/Disposable/DisposeCapability.cs | 6 + .../Disposable/DisposeResourcesHelper.cs | 30 +-- .../Environments/DeclarativeEnvironment.cs | 2 + Jint/Runtime/Environments/Environment.cs | 7 + .../Expressions/JintAwaitExpression.cs | 54 +++--- .../Interpreter/JintFunctionDefinition.cs | 56 ++++-- Jint/Runtime/Modules/SourceTextModule.cs | 73 ++++--- 10 files changed, 299 insertions(+), 158 deletions(-) diff --git a/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs b/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs index a0b623d27a..dcb13890c0 100644 --- a/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs +++ b/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs @@ -262,19 +262,16 @@ private void ResumeExecution(PromiseCapability promiseCapability) catch (JavaScriptException ex) { var env = _engine.ExecutionContext.LexicalEnvironment; + if (!env.HasDisposeResources) + { + SettleResumeRejection(ex.Error, promiseCapability); + return; + } DisposeResourcesHelper.DisposeAndThen( _engine, env, new Completion(CompletionType.Throw, ex.Error, null!), - final => - { - _engine.LeaveExecutionContext(); - _currentPromiseCapability = null; - _asyncGeneratorState = AsyncGeneratorState.Completed; - _completedAwaits = null; - AsyncGeneratorReject(final.Value, promiseCapability); - AsyncGeneratorResumeNext(); - }); + final => SettleResumeRejection(final.Value, promiseCapability)); return; } @@ -300,44 +297,61 @@ private void ResumeExecution(PromiseCapability promiseCapability) // Generator body completed — dispose resources before leaving context. var lexEnv = _engine.ExecutionContext.LexicalEnvironment; - DisposeResourcesHelper.DisposeAndThen(_engine, lexEnv, result, final => + if (!lexEnv.HasDisposeResources) { - _engine.LeaveExecutionContext(); + SettleResumeCompletion(result, promiseCapability); + return; + } + DisposeResourcesHelper.DisposeAndThen(_engine, lexEnv, result, + final => SettleResumeCompletion(final, promiseCapability)); + } - _currentPromiseCapability = null; + private void SettleResumeRejection(JsValue error, PromiseCapability promiseCapability) + { + _engine.LeaveExecutionContext(); + _currentPromiseCapability = null; + _asyncGeneratorState = AsyncGeneratorState.Completed; + _completedAwaits = null; + AsyncGeneratorReject(error, promiseCapability); + AsyncGeneratorResumeNext(); + } + + private void SettleResumeCompletion(Completion final, PromiseCapability promiseCapability) + { + _engine.LeaveExecutionContext(); + _currentPromiseCapability = null; - if (final.Type == CompletionType.Throw) + if (final.Type == CompletionType.Throw) + { + _asyncGeneratorState = AsyncGeneratorState.Completed; + _completedAwaits = null; + AsyncGeneratorReject(final.Value, promiseCapability); + } + else if (final.Type == CompletionType.Return) + { + // Per spec 13.10.1: "return;" (no expression) does NOT await the return value, + // but "return expr;" does call Await(exprValue) before returning. + // When the return statement has no argument, we can resolve directly. + if (final._source is ReturnStatement { Argument: null }) { _asyncGeneratorState = AsyncGeneratorState.Completed; _completedAwaits = null; - AsyncGeneratorReject(final.Value, promiseCapability); - } - else if (final.Type == CompletionType.Return) - { - // Per spec 13.10.1: "return;" (no expression) does NOT await the return value, - // but "return expr;" does call Await(exprValue) before returning. - // When the return statement has no argument, we can resolve directly. - if (final._source is ReturnStatement { Argument: null }) - { - _asyncGeneratorState = AsyncGeneratorState.Completed; - _completedAwaits = null; - AsyncGeneratorResolve(final.Value, true, promiseCapability); - } - else - { - _asyncGeneratorState = AsyncGeneratorState.AwaitingReturn; - AsyncGeneratorAwaitReturn(final.Value, promiseCapability); - } + AsyncGeneratorResolve(final.Value, true, promiseCapability); } - else if (final.Type == CompletionType.Normal) + else { - _asyncGeneratorState = AsyncGeneratorState.Completed; - _completedAwaits = null; - AsyncGeneratorResolve(Undefined, true, promiseCapability); + _asyncGeneratorState = AsyncGeneratorState.AwaitingReturn; + AsyncGeneratorAwaitReturn(final.Value, promiseCapability); } + } + else if (final.Type == CompletionType.Normal) + { + _asyncGeneratorState = AsyncGeneratorState.Completed; + _completedAwaits = null; + AsyncGeneratorResolve(Undefined, true, promiseCapability); + } - AsyncGeneratorResumeNext(); - }); + AsyncGeneratorResumeNext(); } /// @@ -461,54 +475,69 @@ internal void ResumeAfterDelegation(JsValue returnValue, PromiseCapability promi // Generator completed — dispose resources before leaving context. var lexEnv = _engine.ExecutionContext.LexicalEnvironment; - DisposeResourcesHelper.DisposeAndThen(_engine, lexEnv, bodyResult, final => + if (!lexEnv.HasDisposeResources) { - _engine.LeaveExecutionContext(); - - _currentPromiseCapability = null; - - if (final.Type == CompletionType.Throw) - { - _asyncGeneratorState = AsyncGeneratorState.Completed; - _completedAwaits = null; - if (promiseCapability is not null) - { - AsyncGeneratorReject(final.Value, promiseCapability); - } - } - else - { - _asyncGeneratorState = AsyncGeneratorState.Completed; - _completedAwaits = null; - if (promiseCapability is not null) - { - var completionValue = final.Type == CompletionType.Return ? final.Value : Undefined; - AsyncGeneratorResolve(completionValue, true, promiseCapability); - } - } - AsyncGeneratorResumeNext(); - }); + SettleDelegationCompletion(bodyResult, promiseCapability); + return; + } + DisposeResourcesHelper.DisposeAndThen(_engine, lexEnv, bodyResult, + final => SettleDelegationCompletion(final, promiseCapability)); } catch (JavaScriptException ex) { var env = _engine.ExecutionContext.LexicalEnvironment; + if (!env.HasDisposeResources) + { + SettleDelegationRejection(ex.Error, promiseCapability); + return; + } DisposeResourcesHelper.DisposeAndThen( _engine, env, new Completion(CompletionType.Throw, ex.Error, null!), - final => - { - _engine.LeaveExecutionContext(); - _currentPromiseCapability = null; - _asyncGeneratorState = AsyncGeneratorState.Completed; - _completedAwaits = null; - if (promiseCapability is not null) - { - AsyncGeneratorReject(final.Value, promiseCapability); - } - AsyncGeneratorResumeNext(); - }); + final => SettleDelegationRejection(final.Value, promiseCapability)); + } + } + + private void SettleDelegationCompletion(Completion final, PromiseCapability? promiseCapability) + { + _engine.LeaveExecutionContext(); + + _currentPromiseCapability = null; + + if (final.Type == CompletionType.Throw) + { + _asyncGeneratorState = AsyncGeneratorState.Completed; + _completedAwaits = null; + if (promiseCapability is not null) + { + AsyncGeneratorReject(final.Value, promiseCapability); + } + } + else + { + _asyncGeneratorState = AsyncGeneratorState.Completed; + _completedAwaits = null; + if (promiseCapability is not null) + { + var completionValue = final.Type == CompletionType.Return ? final.Value : Undefined; + AsyncGeneratorResolve(completionValue, true, promiseCapability); + } + } + AsyncGeneratorResumeNext(); + } + + private void SettleDelegationRejection(JsValue error, PromiseCapability? promiseCapability) + { + _engine.LeaveExecutionContext(); + _currentPromiseCapability = null; + _asyncGeneratorState = AsyncGeneratorState.Completed; + _completedAwaits = null; + if (promiseCapability is not null) + { + AsyncGeneratorReject(error, promiseCapability); } + AsyncGeneratorResumeNext(); } /// diff --git a/Jint/Native/Disposable/AsyncDisposableStackPrototype.cs b/Jint/Native/Disposable/AsyncDisposableStackPrototype.cs index d901381f01..b1c315efb1 100644 --- a/Jint/Native/Disposable/AsyncDisposableStackPrototype.cs +++ b/Jint/Native/Disposable/AsyncDisposableStackPrototype.cs @@ -59,8 +59,31 @@ private JsValue Dispose(JsValue thisObject) try { var stack = AssertDisposableStack(thisObject); - var result = stack.Dispose(); - capability.Resolve.Call(Undefined, result); + if (!stack.TryMarkDisposed()) + { + // Already disposed — resolve with undefined. + capability.Resolve.Call(Undefined, Undefined); + return capability.PromiseInstance; + } + + // Drive the dispose state machine via Promise.then chains so each + // spec-defined Await(...) consumes a real microtask tick. Settlement + // of the returned promise is deferred until the dispose chain finishes. + DisposeResourcesHelper.DisposeAndThen( + _engine, + stack.DisposeCapability, + new Completion(CompletionType.Normal, Undefined, _engine.GetLastSyntaxElement()), + final => + { + if (final.Type == CompletionType.Throw) + { + capability.Reject.Call(Undefined, final.Value); + } + else + { + capability.Resolve.Call(Undefined, Undefined); + } + }); } catch (JavaScriptException e) { diff --git a/Jint/Native/Disposable/DisposableStack.cs b/Jint/Native/Disposable/DisposableStack.cs index 884bb7ab76..c46f9d5960 100644 --- a/Jint/Native/Disposable/DisposableStack.cs +++ b/Jint/Native/Disposable/DisposableStack.cs @@ -24,6 +24,12 @@ public DisposableStack(Engine engine, DisposeHint hint) : base(engine) public DisposableState State { get; private set; } + /// + /// Exposes the internal dispose capability so async callers (AsyncDisposableStack) + /// can drive the state machine via . + /// + internal DisposeCapability DisposeCapability => _disposeCapability; + public JsValue Dispose() { if (State == DisposableState.Disposed) @@ -40,6 +46,23 @@ public JsValue Dispose() return completion.Value; } + /// + /// Marks the stack disposed and returns the dispose state machine's initial step + /// for the async path. The caller (AsyncDisposableStack.prototype.disposeAsync) + /// drives the state machine via so that each + /// spec-defined Await consumes a real microtask tick. + /// Returns null if the stack was already disposed. + /// + internal bool TryMarkDisposed() + { + if (State == DisposableState.Disposed) + { + return false; + } + State = DisposableState.Disposed; + return true; + } + public void Defer(JsValue onDispose) { AddDisposableResource(Undefined, _hint, onDispose.GetCallable(_engine.Realm)); diff --git a/Jint/Native/Disposable/DisposeCapability.cs b/Jint/Native/Disposable/DisposeCapability.cs index e62e4e6bfd..6bdb845721 100644 --- a/Jint/Native/Disposable/DisposeCapability.cs +++ b/Jint/Native/Disposable/DisposeCapability.cs @@ -46,6 +46,12 @@ public DisposeCapability(Engine engine) _engine = engine; } + /// + /// Fast check used by callers to avoid allocating dispose-completion closures + /// when nothing is registered. + /// + public bool HasResources => _disposableResourceStack.Count > 0; + public void AddDisposableResource(JsValue v, DisposeHint hint, ICallable? method = null) { DisposableResource resource; diff --git a/Jint/Native/Disposable/DisposeResourcesHelper.cs b/Jint/Native/Disposable/DisposeResourcesHelper.cs index bdb8852f34..9ff5514d1a 100644 --- a/Jint/Native/Disposable/DisposeResourcesHelper.cs +++ b/Jint/Native/Disposable/DisposeResourcesHelper.cs @@ -8,8 +8,8 @@ namespace Jint.Native.Disposable; /// /// Drives 's state machine for call sites where the -/// async function/generator body has already finished — so we don't need to suspend -/// the function itself, only defer settlement of its return promise. Each +/// async function/generator/module body has already finished — so we don't need to +/// suspend the function itself, only defer settlement of its return promise. Each /// state-machine Suspend is chained via Promise.then, and the caller-provided /// continueWith callback fires when the full dispose chain has completed. /// @@ -21,15 +21,23 @@ public static void DisposeAndThen( Completion initial, Action continueWith) { - var step = env.BeginDisposeResources(initial); - DriveAsync(engine, env, step, continueWith); + Drive(engine, env.BeginDisposeResources(initial), continueWith, env.ContinueDisposeResources); } - private static void DriveAsync( + public static void DisposeAndThen( Engine engine, - Environment env, - DisposeStepResult step, + DisposeCapability capability, + Completion initial, Action continueWith) + { + Drive(engine, capability.BeginDispose(initial), continueWith, capability.ContinueDispose); + } + + private static void Drive( + Engine engine, + DisposeStepResult step, + Action continueWith, + Func continueDispose) { while (!step.IsDone) { @@ -39,15 +47,15 @@ private static void DriveAsync( var onFulfilled = new ClrFunction(engine, "", (_, args) => { - var next = env.ContinueDisposeResources(args.At(0), awaitThrew: false); - DriveAsync(engine, env, next, continueWith); + var next = continueDispose(args.At(0), false); + Drive(engine, next, continueWith, continueDispose); return JsValue.Undefined; }, 1, PropertyFlag.Configurable); var onRejected = new ClrFunction(engine, "", (_, args) => { - var next = env.ContinueDisposeResources(args.At(0), awaitThrew: true); - DriveAsync(engine, env, next, continueWith); + var next = continueDispose(args.At(0), true); + Drive(engine, next, continueWith, continueDispose); return JsValue.Undefined; }, 1, PropertyFlag.Configurable); diff --git a/Jint/Runtime/Environments/DeclarativeEnvironment.cs b/Jint/Runtime/Environments/DeclarativeEnvironment.cs index 610575eca7..340d318dd2 100644 --- a/Jint/Runtime/Environments/DeclarativeEnvironment.cs +++ b/Jint/Runtime/Environments/DeclarativeEnvironment.cs @@ -342,6 +342,8 @@ internal sealed override string[] GetAllBindingNames() internal sealed override Completion DisposeResources(Completion c) => _disposeCapability?.DisposeResources(c) ?? c; + internal sealed override bool HasDisposeResources => _disposeCapability?.HasResources == true; + /// /// Begin dispose via the state machine. Returns either Done with the final Completion, /// or Suspend with a Promise the caller must await before calling diff --git a/Jint/Runtime/Environments/Environment.cs b/Jint/Runtime/Environments/Environment.cs index a54e0fec83..8ac4f03372 100644 --- a/Jint/Runtime/Environments/Environment.cs +++ b/Jint/Runtime/Environments/Environment.cs @@ -110,6 +110,13 @@ public override bool Equals(JsValue? other) internal virtual Completion DisposeResources(Completion c) => c; + /// + /// Fast bool check for whether this environment has any disposable resources to + /// process. Lets callers skip allocating a continueWith closure when there's no + /// dispose work to do — the dispose hot path on every async function/generator exit. + /// + internal virtual bool HasDisposeResources => false; + /// /// Begin dispose via the state machine. Default returns Done immediately for /// environments that don't track disposable resources. diff --git a/Jint/Runtime/Interpreter/Expressions/JintAwaitExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintAwaitExpression.cs index 78d18a66a8..64768488ec 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintAwaitExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintAwaitExpression.cs @@ -243,16 +243,18 @@ internal static void AsyncFunctionResume(Engine engine, AsyncFunctionInstance as catch (JavaScriptException e) { var env = engine.ExecutionContext.LexicalEnvironment; + if (!env.HasDisposeResources) + { + engine.LeaveExecutionContext(); + asyncInstance._state = AsyncFunctionState.Completed; + asyncInstance._capability.Reject.Call(JsValue.Undefined, e.Error); + return; + } DisposeResourcesHelper.DisposeAndThen( engine, env, new Completion(CompletionType.Throw, e.Error, null!), - final => - { - engine.LeaveExecutionContext(); - asyncInstance._state = AsyncFunctionState.Completed; - asyncInstance._capability.Reject.Call(JsValue.Undefined, final.Value); - }); + final => SettleAsyncResumeCompletion(engine, asyncInstance, final)); return; } @@ -263,23 +265,31 @@ internal static void AsyncFunctionResume(Engine engine, AsyncFunctionInstance as } var lexEnv = engine.ExecutionContext.LexicalEnvironment; - DisposeResourcesHelper.DisposeAndThen(engine, lexEnv, result, final => + if (!lexEnv.HasDisposeResources) { - engine.LeaveExecutionContext(); - asyncInstance._state = AsyncFunctionState.Completed; + SettleAsyncResumeCompletion(engine, asyncInstance, result); + return; + } + DisposeResourcesHelper.DisposeAndThen(engine, lexEnv, result, + final => SettleAsyncResumeCompletion(engine, asyncInstance, final)); + } - if (final.Type == CompletionType.Throw) - { - asyncInstance._capability.Reject.Call(JsValue.Undefined, final.Value); - } - else if (final.Type == CompletionType.Return) - { - asyncInstance._capability.Resolve.Call(JsValue.Undefined, final.Value); - } - else - { - asyncInstance._capability.Resolve.Call(JsValue.Undefined, JsValue.Undefined); - } - }); + private static void SettleAsyncResumeCompletion(Engine engine, AsyncFunctionInstance asyncInstance, Completion final) + { + engine.LeaveExecutionContext(); + asyncInstance._state = AsyncFunctionState.Completed; + + if (final.Type == CompletionType.Throw) + { + asyncInstance._capability.Reject.Call(JsValue.Undefined, final.Value); + } + else if (final.Type == CompletionType.Return) + { + asyncInstance._capability.Resolve.Call(JsValue.Undefined, final.Value); + } + else + { + asyncInstance._capability.Resolve.Call(JsValue.Undefined, JsValue.Undefined); + } } } diff --git a/Jint/Runtime/Interpreter/JintFunctionDefinition.cs b/Jint/Runtime/Interpreter/JintFunctionDefinition.cs index b25f7d3008..c194873ffd 100644 --- a/Jint/Runtime/Interpreter/JintFunctionDefinition.cs +++ b/Jint/Runtime/Interpreter/JintFunctionDefinition.cs @@ -179,8 +179,15 @@ private static void AsyncBlockStart( catch (JavaScriptException e) { // Per spec: DisposeResources before rejecting. Use the helper so async-dispose - // resources are awaited via the state machine instead of sync-blocking. + // resources are awaited via the state machine instead of sync-blocking. Skip + // the helper entirely if the env has no disposables — common-case hot path. var env = engine.ExecutionContext.LexicalEnvironment; + if (!env.HasDisposeResources) + { + asyncInstance._state = AsyncFunctionState.Completed; + asyncInstance._capability.Reject.Call(JsValue.Undefined, e.Error); + return; + } DisposeResourcesHelper.DisposeAndThen( engine, env, @@ -203,29 +210,36 @@ private static void AsyncBlockStart( // Per spec AsyncBlockStart step 3.f: DisposeResources after body completes. // Settlement of the function's return promise is deferred until the dispose chain - // (which may itself await) finishes. + // (which may itself await) finishes. Fast-path skip when no disposables registered. var lexEnv = engine.ExecutionContext.LexicalEnvironment; - DisposeResourcesHelper.DisposeAndThen(engine, lexEnv, result, final => + if (!lexEnv.HasDisposeResources) { - asyncInstance._state = AsyncFunctionState.Completed; + SettleAsyncFunctionCompletion(asyncInstance, result); + return; + } + DisposeResourcesHelper.DisposeAndThen(engine, lexEnv, result, final => SettleAsyncFunctionCompletion(asyncInstance, final)); + } - if (final.Type == CompletionType.Throw) - { - asyncInstance._capability.Reject.Call(JsValue.Undefined, final.Value); - } - else if (final.Type == CompletionType.Normal) - { - asyncInstance._capability.Resolve.Call(JsValue.Undefined, JsValue.Undefined); - } - else if (final.Type == CompletionType.Return) - { - asyncInstance._capability.Resolve.Call(JsValue.Undefined, final.Value); - } - else - { - asyncInstance._capability.Reject.Call(JsValue.Undefined, final.Value); - } - }); + private static void SettleAsyncFunctionCompletion(AsyncFunctionInstance asyncInstance, Completion final) + { + asyncInstance._state = AsyncFunctionState.Completed; + + if (final.Type == CompletionType.Throw) + { + asyncInstance._capability.Reject.Call(JsValue.Undefined, final.Value); + } + else if (final.Type == CompletionType.Normal) + { + asyncInstance._capability.Resolve.Call(JsValue.Undefined, JsValue.Undefined); + } + else if (final.Type == CompletionType.Return) + { + asyncInstance._capability.Resolve.Call(JsValue.Undefined, final.Value); + } + else + { + asyncInstance._capability.Reject.Call(JsValue.Undefined, final.Value); + } } /// diff --git a/Jint/Runtime/Modules/SourceTextModule.cs b/Jint/Runtime/Modules/SourceTextModule.cs index 4a2525b290..95679d9217 100644 --- a/Jint/Runtime/Modules/SourceTextModule.cs +++ b/Jint/Runtime/Modules/SourceTextModule.cs @@ -407,16 +407,18 @@ internal override Completion ExecuteModule(PromiseCapability? capability = null) } catch (JavaScriptException e) { - DisposeResourcesHelper.DisposeAndThen( - _engine, - _environment, - new Completion(CompletionType.Throw, e.Error, null!), - final => - { - _engine.LeaveExecutionContext(); - _tlaAsyncInstance._state = AsyncFunctionState.Completed; - capability!.Reject.Call(JsValue.Undefined, final.Value); - }); + if (!_environment.HasDisposeResources) + { + SettleTlaRejection(capability!, e.Error); + } + else + { + DisposeResourcesHelper.DisposeAndThen( + _engine, + _environment, + new Completion(CompletionType.Throw, e.Error, null!), + final => SettleTlaRejection(capability!, final.Value)); + } return new Completion(CompletionType.Normal, JsValue.Undefined, null!); } @@ -432,27 +434,44 @@ internal override Completion ExecuteModule(PromiseCapability? capability = null) // Module body complete - dispose any (potentially async-dispose) resources // before settling the capability. Settlement is deferred until the dispose // chain finishes so top-level `await using` can suspend correctly. - DisposeResourcesHelper.DisposeAndThen(_engine, _environment, result, final => + if (!_environment.HasDisposeResources) { - _engine.LeaveExecutionContext(); - _tlaAsyncInstance._state = AsyncFunctionState.Completed; - - if (final.Type == CompletionType.Normal) - { - capability!.Resolve.Call(JsValue.Undefined, JsValue.Undefined); - } - else if (final.Type == CompletionType.Throw) - { - capability!.Reject.Call(JsValue.Undefined, final.Value); - } - else - { - capability!.Resolve.Call(JsValue.Undefined, final.Value); - } - }); + SettleTlaCompletion(capability!, result); + } + else + { + DisposeResourcesHelper.DisposeAndThen(_engine, _environment, result, + final => SettleTlaCompletion(capability!, final)); + } return new Completion(CompletionType.Normal, JsValue.Undefined, null!); } } } + + private void SettleTlaCompletion(PromiseCapability capability, Completion final) + { + _engine.LeaveExecutionContext(); + _tlaAsyncInstance!._state = AsyncFunctionState.Completed; + + if (final.Type == CompletionType.Normal) + { + capability.Resolve.Call(JsValue.Undefined, JsValue.Undefined); + } + else if (final.Type == CompletionType.Throw) + { + capability.Reject.Call(JsValue.Undefined, final.Value); + } + else + { + capability.Resolve.Call(JsValue.Undefined, final.Value); + } + } + + private void SettleTlaRejection(PromiseCapability capability, JsValue error) + { + _engine.LeaveExecutionContext(); + _tlaAsyncInstance!._state = AsyncFunctionState.Completed; + capability.Reject.Call(JsValue.Undefined, error); + } } From 5964e6ef6a17b758bcb7f3b92eeeda766514ef5d Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Sun, 17 May 2026 12:43:11 +0300 Subject: [PATCH 3/4] Address review feedback + fix async-generator context leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review fixes: - Drop the redundant asyncFn._isResuming = false in JintBlockStatement.ResumeDispose (the suspendable.IsResuming setter already routes to the same field). - Remove the unreachable catch (PromiseRejectedException) in DisposeCapability.Advance; method.Call surfaces JS rejection as JavaScriptException, not as the C# type. - Document that DisposeCapability's state machine is non-reentrant on a single instance, matching Jint's single-thread-per-Engine assumption. Latent async-generator context-leak fixed: - AsyncGeneratorInstance.ResumeExecution's slow path (HasDisposeResources == true) used to defer LeaveExecutionContext until the dispose chain's continueWith fired, leaving the generator's context on the engine's execution-context stack while PerformPromiseThen scheduled the dispose tick. Any synchronous `await` in the caller that ran between ResumeExecution returning and the dispose tick firing saw `engine.ExecutionContext.AsyncGenerator` still set and mis-routed through SuspendForAwaitInAsyncGenerator, eventually NRE-ing on a captured (later nulled) _currentPromiseCapability. - Fix by leaving the generator's context BEFORE dispatching the dispose chain in both ResumeExecution and ResumeAfterDelegation; the Settle helpers no longer call LeaveExecutionContext (now a caller responsibility). New local tests: - AsyncDisposableStack.disposeAsync microtask ordering (mirrors Test262 explicit-await-for-null.js — caught the prior CI-regression locally). - Top-level `await using` in a TLA module (Pattern B in SourceTextModule). - `await using` in an async generator body (Pattern B in AsyncGeneratorInstance — caught the context leak above). Test pass after these changes: - Jint.Tests: 3007 passed / 0 failed. - Test262 regression sweep (Async*/Disposable*/awaitUsing/Using/forAwait*/ forStatement/forInStatement/forOfStatement/Modules/Generator): 5219 passed. Co-Authored-By: Claude Opus 4.7 (1M context) --- Jint.Tests/Runtime/InteropDisposeTests.cs | 73 +++++++++++++++++++ .../AsyncGenerator/AsyncGeneratorInstance.cs | 25 +++++-- Jint/Native/Disposable/DisposeCapability.cs | 11 +-- .../Statements/JintBlockStatement.cs | 3 +- 4 files changed, 97 insertions(+), 15 deletions(-) diff --git a/Jint.Tests/Runtime/InteropDisposeTests.cs b/Jint.Tests/Runtime/InteropDisposeTests.cs index f394e85df5..f54630b889 100644 --- a/Jint.Tests/Runtime/InteropDisposeTests.cs +++ b/Jint.Tests/Runtime/InteropDisposeTests.cs @@ -216,6 +216,79 @@ await engine.EvaluateAsync(""" disposable.Disposed.Should().BeTrue(); } + /// + /// AsyncDisposableStack.prototype.disposeAsync must consume the spec-mandated + /// Await(undefined) microtask tick even when the only resource is null. The + /// expected interleaving puts 'job 1' (two ticks deep) before 'dispose' (also + /// two ticks deep: one for the Await(undefined), one for the .then handler). + /// Without the tick, 'dispose' would fire synchronously and produce + /// ['dispose', 'job 1', 'job 2'] instead. + /// Mirrors built-ins/AsyncDisposableStack/prototype/disposeAsync/ + /// explicit-await-for-null.js from Test262. + /// + [Fact] + public async Task ShouldAsyncDisposableStackConsumeAwaitTickForNull() + { + var engine = new Engine(); + var result = await engine.EvaluateAsync(""" + (async function () { + const stack = new AsyncDisposableStack(); + const sequence = []; + stack.use(null); + await Promise.all([ + Promise.resolve().then(() => 0).then(() => { sequence.push('job 1'); }), + stack.disposeAsync().then(() => { sequence.push('dispose'); }), + Promise.resolve().then(() => 0).then(() => { sequence.push('job 2'); }) + ]); + return sequence.join(','); + })() + """); + result.AsString().Should().Be("job 1,dispose,job 2"); + } + + /// + /// Top-level `await using` in a module — exercises the Pattern B refactor in + /// SourceTextModule's TLA execution path. + /// + [Fact] + public void ShouldAsyncDisposeInTopLevelAwaitModule() + { + var engine = new Engine(o => o.ExperimentalFeatures = ExperimentalFeature.TaskInterop); + var disposable = new AsyncDisposable(); + engine.SetValue("getAsync", () => disposable); + + engine.Modules.Add("tla-dispose", "await using d = getAsync();"); + engine.Modules.Import("tla-dispose"); + + disposable.Disposed.Should().BeTrue(); + } + + /// + /// `await using` inside an async generator body — exercises the Pattern B + /// refactor in AsyncGeneratorInstance. + /// + [Fact] + public async Task ShouldAsyncDisposeInAsyncGenerator() + { + var engine = new Engine(o => o.ExperimentalFeatures = ExperimentalFeature.TaskInterop); + var disposable = new AsyncDisposable(); + engine.SetValue("getAsync", () => disposable); + + await engine.EvaluateAsync(""" + (async () => { + async function* gen() { + await using d = getAsync(); + yield 1; + } + const it = gen(); + await it.next(); + await it.next(); + })() + """); + + disposable.Disposed.Should().BeTrue(); + } + private class AsyncDisposable : IAsyncDisposable { public bool Disposed { get; private set; } diff --git a/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs b/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs index dcb13890c0..359d9ea53f 100644 --- a/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs +++ b/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs @@ -261,7 +261,12 @@ private void ResumeExecution(PromiseCapability promiseCapability) } catch (JavaScriptException ex) { + // Leave context up front so any async dispose work below runs (and its + // Promise.then callbacks fire) without the generator's execution context + // still on the stack — otherwise outer `await`s in the caller would + // mis-route through SuspendForAwaitInAsyncGenerator. var env = _engine.ExecutionContext.LexicalEnvironment; + _engine.LeaveExecutionContext(); if (!env.HasDisposeResources) { SettleResumeRejection(ex.Error, promiseCapability); @@ -295,8 +300,12 @@ private void ResumeExecution(PromiseCapability promiseCapability) return; } - // Generator body completed — dispose resources before leaving context. + // Generator body completed — leave the generator's execution context BEFORE + // dispatching dispose so its async Promise.then chain doesn't run with the + // generator's context still on the stack. Settlement helpers below assume + // the context has already been popped. var lexEnv = _engine.ExecutionContext.LexicalEnvironment; + _engine.LeaveExecutionContext(); if (!lexEnv.HasDisposeResources) { SettleResumeCompletion(result, promiseCapability); @@ -308,7 +317,8 @@ private void ResumeExecution(PromiseCapability promiseCapability) private void SettleResumeRejection(JsValue error, PromiseCapability promiseCapability) { - _engine.LeaveExecutionContext(); + // Note: caller (ResumeExecution / its callback) has already left the + // generator's execution context. _currentPromiseCapability = null; _asyncGeneratorState = AsyncGeneratorState.Completed; _completedAwaits = null; @@ -318,7 +328,6 @@ private void SettleResumeRejection(JsValue error, PromiseCapability promiseCapab private void SettleResumeCompletion(Completion final, PromiseCapability promiseCapability) { - _engine.LeaveExecutionContext(); _currentPromiseCapability = null; if (final.Type == CompletionType.Throw) @@ -473,8 +482,10 @@ internal void ResumeAfterDelegation(JsValue returnValue, PromiseCapability promi return; } - // Generator completed — dispose resources before leaving context. + // Generator completed — leave the generator's execution context BEFORE + // dispatching dispose (see ResumeExecution for the same rationale). var lexEnv = _engine.ExecutionContext.LexicalEnvironment; + _engine.LeaveExecutionContext(); if (!lexEnv.HasDisposeResources) { SettleDelegationCompletion(bodyResult, promiseCapability); @@ -486,6 +497,7 @@ internal void ResumeAfterDelegation(JsValue returnValue, PromiseCapability promi catch (JavaScriptException ex) { var env = _engine.ExecutionContext.LexicalEnvironment; + _engine.LeaveExecutionContext(); if (!env.HasDisposeResources) { SettleDelegationRejection(ex.Error, promiseCapability); @@ -501,8 +513,7 @@ internal void ResumeAfterDelegation(JsValue returnValue, PromiseCapability promi private void SettleDelegationCompletion(Completion final, PromiseCapability? promiseCapability) { - _engine.LeaveExecutionContext(); - + // Note: caller has already left the generator's execution context. _currentPromiseCapability = null; if (final.Type == CompletionType.Throw) @@ -529,7 +540,7 @@ private void SettleDelegationCompletion(Completion final, PromiseCapability? pro private void SettleDelegationRejection(JsValue error, PromiseCapability? promiseCapability) { - _engine.LeaveExecutionContext(); + // Note: caller has already left the generator's execution context. _currentPromiseCapability = null; _asyncGeneratorState = AsyncGeneratorState.Completed; _completedAwaits = null; diff --git a/Jint/Native/Disposable/DisposeCapability.cs b/Jint/Native/Disposable/DisposeCapability.cs index 6bdb845721..d6795d01dd 100644 --- a/Jint/Native/Disposable/DisposeCapability.cs +++ b/Jint/Native/Disposable/DisposeCapability.cs @@ -34,7 +34,10 @@ internal sealed class DisposeCapability private readonly Engine _engine; private readonly List _disposableResourceStack = []; - // State preserved across suspensions (3.e.ii.1, 3.d.i, 4.a). + // State preserved across suspensions (3.e.ii.1, 3.d.i, 4.a). Not reentrant — + // a single DisposeCapability instance can only drive one in-flight dispose at a + // time. This matches the engine's single-thread-per-Engine assumption: a JS + // dispose method cannot synchronously re-enter dispose on its own capability. private int _disposeIndex; private bool _disposeHasAwaited; private bool _disposeNeedsAwait; @@ -215,12 +218,6 @@ private DisposeStepResult Advance() _disposeIndex--; continue; } - catch (PromiseRejectedException e) - { - HandleDisposeException(e.RejectedValue); - _disposeIndex--; - continue; - } if (hint == DisposeHint.Async) { diff --git a/Jint/Runtime/Interpreter/Statements/JintBlockStatement.cs b/Jint/Runtime/Interpreter/Statements/JintBlockStatement.cs index 9e19ebccf2..991d604fd8 100644 --- a/Jint/Runtime/Interpreter/Statements/JintBlockStatement.cs +++ b/Jint/Runtime/Interpreter/Statements/JintBlockStatement.cs @@ -163,6 +163,8 @@ private Completion ResumeDispose( ISuspendable suspendable) { var engine = context.Engine; + // suspendable.IsResuming setter delegates to asyncFn._isResuming, so this + // single assignment clears both the interface flag and the underlying field. suspendable.IsResuming = false; var asyncFn = engine.ExecutionContext.AsyncFunction!; @@ -170,7 +172,6 @@ private Completion ResumeDispose( var awaitThrew = asyncFn._resumeWithThrow; asyncFn._resumeValue = null; asyncFn._resumeWithThrow = false; - asyncFn._isResuming = false; asyncFn._lastAwaitNode = null; // Use the env captured at suspend time, in case the env-setup branch above From 7d1662c179f10474ea87ae112b2beaeb188db165 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Sun, 17 May 2026 16:19:55 +0300 Subject: [PATCH 4/4] Address review feedback + fix for-of dispose suspension MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SourceTextModule TLA context leak (symmetry with AsyncGenerator fix): - ExecuteAsyncModule now leaves the module's execution context AND sets _tlaAsyncInstance._state = Completed BEFORE dispatching the dispose chain via DisposeResourcesHelper.DisposeAndThen, matching the AsyncGenerator fix in commit 5964e6ef6. Otherwise the Promise.then callbacks scheduled by the dispose chain run with the module's context still on the stack, and a sync `await` in subsequent code can mis-route via this module's _tlaAsyncInstance. - SettleTla simplified to a static helper that just resolves/rejects the capability based on the final completion type. JintForInForOfStatement: per-iteration dispose state machine: - Previously, iterationEnv?.DisposeResources(result) drove the dispose state machine synchronously via UnwrapIfPromise, susceptible to the same #2477 deadlock pattern for `for (await using x of asyncIter())` in an async function. - After body execution, when iterationEnv has dispose resources and we are in an async function (and the body didn't suspend), DriveDispose uses Pattern A — suspends the async function on each pending dispose promise via the same machinery as a JS `await`. - On resume, ResumeFromDispose advances the iteration env's dispose state machine with the awaited result, re-suspends if needed, then applies the same post-dispose handling the main loop applies (update accumulator, propagate Throw/Break/Return/Continue) and either returns the abrupt completion or re-enters BodyEvaluation for the next iteration with the saved iterator and accumulator. - ForOfSuspendData and ForAwaitSuspendData gain DisposeInProgress + DisposeResult fields; ForAwaitSuspendData also gains IterationEnv + OuterEnv to mirror what ForOfSuspendData already carried. - Setting close = false before returning the suspended completion prevents BodyEvaluation's finally block from clearing the suspend data we just stored — same pattern as the body-await suspension. - Sync / no-async-function contexts fall back to the legacy synchronous drive. Tests added (InteropDisposeTests): - ShouldAsyncDisposeInTopLevelAwaitModuleWithGenuinelyAsyncTask: TLA module with Task.Delay-based dispose to lock the SourceTextModule fix in place. - ShouldAsyncDisposeInForOfWithGenuinelyAsyncTask_SingleIteration and ShouldAsyncDisposeInForOfWithGenuinelyAsyncTask: for-of with truly-async dispose, 1 and 3 iterations, LIFO ordering verified. - ShouldAsyncDisposeInForOfPropagatesRejection: dispose-method rejection mid-loop aborts further iterations. Benchmark added (AsyncFunctionExitBenchmark): Three benchmarks measuring async function/generator exit cost — plain async fn exit, async fn with sync `using`, plain async generator exit. ShortRun comparison vs upstream/main: PlainAsyncFunctionExit_1000: 719us -> 682us, 1.99MB -> 2.01MB AsyncFunctionWithSyncUsing_1000: 1163us -> 1222us, 2.5MB -> 2.73MB PlainAsyncGeneratorExit_500: 2104us -> 1987us, 5.8MB -> 5.83MB Zero-alloc hot path (no `using`) preserved within noise — the HasDisposeResources fast path works as designed. Verified: - Jint.Tests net10.0: 3011 / 0 / 5 (passed / failed / skipped) - Jint.Tests net472: 2949 / 0 / 5 - Jint.Tests.Test262: 99,015 / 0 / 131 Co-Authored-By: Claude Opus 4.7 (1M context) --- Jint.Benchmark/AsyncFunctionExitBenchmark.cs | 61 ++++ Jint.Tests/Runtime/InteropDisposeTests.cs | 113 +++++++ .../Statements/JintForInForOfStatement.cs | 297 +++++++++++++++++- Jint/Runtime/Modules/SourceTextModule.cs | 54 ++-- Jint/Runtime/SuspendData.cs | 39 +++ 5 files changed, 535 insertions(+), 29 deletions(-) create mode 100644 Jint.Benchmark/AsyncFunctionExitBenchmark.cs diff --git a/Jint.Benchmark/AsyncFunctionExitBenchmark.cs b/Jint.Benchmark/AsyncFunctionExitBenchmark.cs new file mode 100644 index 0000000000..ac5113e775 --- /dev/null +++ b/Jint.Benchmark/AsyncFunctionExitBenchmark.cs @@ -0,0 +1,61 @@ +using BenchmarkDotNet.Attributes; + +namespace Jint.Benchmark; + +/// +/// Measures async-function and async-generator EXIT cost in two shapes: +/// - "Plain": no `await using` declarations in scope; the dispose state machine +/// is skipped via the HasDisposeResources fast path. Goal: zero allocation +/// beyond the function/promise plumbing itself. +/// - "WithSyncUsing": one `using` declaration with a sync dispose method; the +/// state machine runs but never suspends. +/// Compares against the pre-PR-2478 "everything inline-disposes" code path by +/// running many iterations so per-call allocation differences dominate. +/// +[MemoryDiagnoser] +public class AsyncFunctionExitBenchmark +{ + private Prepared