diff --git a/Jint.Tests.Test262/Test262Harness.settings.json b/Jint.Tests.Test262/Test262Harness.settings.json index aa4b7e8be1..c32763830f 100644 --- a/Jint.Tests.Test262/Test262Harness.settings.json +++ b/Jint.Tests.Test262/Test262Harness.settings.json @@ -44,7 +44,6 @@ "staging/sm/Date/dst-offset-caching-6-of-8.js", "staging/sm/Date/dst-offset-caching-7-of-8.js", "staging/sm/Date/dst-offset-caching-8-of-8.js", - "staging/sm/generators/delegating-yield-9.js", "staging/sm/regress/regress-610026.js", "staging/sm/regress/regress-619003-1.js", // Same shape, found by running the suite: each of these burns the whole 30-second budget and diff --git a/Jint.Tests/Runtime/GeneratorTests.cs b/Jint.Tests/Runtime/GeneratorTests.cs index f23e646c9f..76d82dbbe5 100644 --- a/Jint.Tests/Runtime/GeneratorTests.cs +++ b/Jint.Tests/Runtime/GeneratorTests.cs @@ -871,6 +871,261 @@ public void ShouldRaiseAdditionChainBigIntMixErrorBeforeLaterOperandSideEffects( _engine.Evaluate(Script).AsString().Should().Be("[0,0]"); } + [Fact(Timeout = 10000)] + public void ADelegatingYieldInsideAYieldStartsOverOnEveryLoopIteration() + { + // https://tc39.es/ecma262/#sec-generator-function-definitions-runtime-semantics-evaluation: + // every evaluation of `yield * AssignmentExpression` evaluates its operand and drives the + // resulting iterator to completion, so a loop that comes back round to the same yield* node + // starts a fresh delegation. Jint replays a generator body from the top on each resume and + // memoized what each yield node had already returned; the memo was never invalidated, so the + // second iteration answered the outer yield from the first iteration's value without ever + // evaluating the operand -- which abandoned the delegation and, because the operand carries + // the loop's own decrement here, left n unchanged and the loop running forever. + // staging/sm/generators/delegating-yield-9.js is this shape; SpiderMonkey and V8 both + // report eight results for countdown(3). + const string Script = """ + function* countdown(n) { + while (n > 0) { + yield (yield* countdown(--n)); + } + return 34; + } + + var results = []; + var it = countdown(3); + var result; + do { + result = it.next(); + results.push(result.value + ':' + result.done); + } while (!result.done && results.length < 100); + return results.join(' '); + """; + + // A regression here spins forever, and xUnit's Timeout cannot abort a synchronous test method + // on its own; the engine has to observe the test's token for the timeout to bite. + var engine = new Engine(options => options.CancellationToken(TestContext.Current.CancellationToken)); + + engine.Evaluate(Script).Should().Be("34:false 34:false 34:false 34:false 34:false 34:false 34:false 34:true"); + } + + [Fact(Timeout = 10000)] + public void ADelegatingYieldInsideAYieldKeepsItsPlaceWhenTheDecrementIsElsewhere() + { + // The same defect without the runaway loop: with the decrement in its own statement the loop + // still terminates, but the outer yield answered from the memo instead of yielding, so two of + // the eight results went missing. Kept separate because a fix that only stopped the hang + // would leave this one silently wrong. + const string Script = """ + function* countdown(n) { + while (n > 0) { + n = n - 1; + yield (yield* countdown(n)); + } + return 34; + } + + var results = []; + var it = countdown(3); + var result; + do { + result = it.next(); + results.push(result.value + ':' + result.done); + } while (!result.done && results.length < 100); + return results.join(' '); + """; + + var engine = new Engine(options => options.CancellationToken(TestContext.Current.CancellationToken)); + + engine.Evaluate(Script).Should().Be("34:false 34:false 34:false 34:false 34:false 34:false 34:false 34:true"); + } + + [Fact(Timeout = 10000)] + public void AnAsyncDelegatingYieldInsideAYieldStartsOverOnEveryLoopIteration() + { + // The async twin of the two tests above, and a second defect underneath the memo one they + // pin. Nothing recorded WHERE an async generator stood while a yield* delegation was in + // flight: a delegation's suspension point is tracked in its own field and only an ordinary + // yield's was published as the resume position, so every resume-aware statement re-ran its + // own test on the way back in. Here `countdown(--n)` had already moved n, so the `while` + // that was true when the iteration began answered false on resume: the loop was abandoned + // mid-iteration and the outer yield it still owed was never reached -- one result lost per + // nesting level, half the sequence for countdown(3). + // https://tc39.es/ecma262/#sec-asyncgeneratoryield suspends the generator AT the yield, and + // https://tc39.es/ecma262/#sec-generator-function-definitions-runtime-semantics-evaluation + // resumes a yield* delegation with the completion its caller sent; neither re-evaluates the + // iteration statement the yield* sits in. V8 and SpiderMonkey both report eight results for + // countdown(3), as they do for the synchronous countdown above. + const string Script = """ + async function* countdown(n) { + while (n > 0) { + yield (yield* countdown(--n)); + } + return 34; + } + + function makeIterator() { return countdown(3); } + """; + + Drain(Script, Async, TestContext.Current.CancellationToken).Should().Be("34:false 34:false 34:false 34:false 34:false 34:false 34:false 34:true"); + } + + [Fact(Timeout = 10000)] + public void AnAsyncDelegatingYieldInsideAYieldKeepsItsPlaceWhenTheDecrementIsElsewhere() + { + // The async twin of ADelegatingYieldInsideAYieldKeepsItsPlaceWhenTheDecrementIsElsewhere. + // Moving the decrement out of the operand changes nothing here, because what the resume lost + // was its place in the loop, not the operand. + const string Script = """ + async function* countdown(n) { + while (n > 0) { + n = n - 1; + yield (yield* countdown(n)); + } + return 34; + } + + function makeIterator() { return countdown(3); } + """; + + Drain(Script, Async, TestContext.Current.CancellationToken).Should().Be("34:false 34:false 34:false 34:false 34:false 34:false 34:false 34:true"); + } + + // The three pairs below are that same missing resume position reached without any recursion, one + // pair per resume-aware statement. Each is written so the statement's own test is already FALSE + // by the time the delegation suspends, which is what makes a re-evaluated test observable: the + // engine dropped the rest of the iteration the generator was in the middle of. The synchronous + // half is pinned beside the asynchronous one because the bookkeeping is per instance type, and + // the two have drifted apart once already. + + [Fact(Timeout = 10000)] + public void ADelegatingYieldSuspensionKeepsTheEnclosingWhileLoopFromRestarting() + { + Drain(WhileWithFalsifiedTest("function*"), Sync, TestContext.Current.CancellationToken).Should().Be("1:false after:true"); + } + + [Fact(Timeout = 10000)] + public void AnAsyncDelegatingYieldSuspensionKeepsTheEnclosingWhileLoopFromRestarting() + { + Drain(WhileWithFalsifiedTest("async function*"), Async, TestContext.Current.CancellationToken).Should().Be("1:false after:true"); + } + + [Fact(Timeout = 10000)] + public void ADelegatingYieldSuspensionKeepsTheEnclosingForLoopFromRestarting() + { + Drain(ForWithFalsifiedTest("function*"), Sync, TestContext.Current.CancellationToken).Should().Be("1:false body:true"); + } + + [Fact(Timeout = 10000)] + public void AnAsyncDelegatingYieldSuspensionKeepsTheEnclosingForLoopFromRestarting() + { + Drain(ForWithFalsifiedTest("async function*"), Async, TestContext.Current.CancellationToken).Should().Be("1:false body:true"); + } + + [Fact(Timeout = 10000)] + public void ADelegatingYieldSuspensionKeepsTheEnclosingIfFromTakingTheOtherBranch() + { + Drain(IfWithFlippedTest("function*"), Sync, TestContext.Current.CancellationToken).Should().Be("1:false then:true"); + } + + [Fact(Timeout = 10000)] + public void AnAsyncDelegatingYieldSuspensionKeepsTheEnclosingIfFromTakingTheOtherBranch() + { + Drain(IfWithFlippedTest("async function*"), Async, TestContext.Current.CancellationToken).Should().Be("1:false then:true"); + } + + private static string WhileWithFalsifiedTest(string kind) => $$""" + {{kind}} leaf() { yield 1; } + {{kind}} outer() { + var log = []; + var n = 2; + while (n > 0) { + n = 0; + yield* leaf(); + log.push('after'); + } + return log.join(','); + } + + function makeIterator() { return outer(); } + """; + + private static string ForWithFalsifiedTest(string kind) => $$""" + {{kind}} leaf() { yield 1; } + {{kind}} outer() { + var log = []; + for (var i = 0; i < 2; i++) { + i = 5; + yield* leaf(); + log.push('body'); + } + return log.join(','); + } + + function makeIterator() { return outer(); } + """; + + private static string IfWithFlippedTest(string kind) => $$""" + {{kind}} leaf() { yield 1; } + {{kind}} outer() { + var log = []; + var taken = true; + if (taken) { + taken = false; + yield* leaf(); + log.push('then'); + } else { + log.push('else'); + } + return log.join(','); + } + + function makeIterator() { return outer(); } + """; + + private const bool Sync = false; + private const bool Async = true; + + /// + /// Drives whatever the script's makeIterator() returns to completion, as + /// "value:done value:done ...". + /// + /// + /// The twenty-result cap is a guard rather than a limit: every sequence these tests assert is far + /// shorter, so a regression that never terminates comes back as a wrong string instead of as a + /// test run that hangs. xUnit's Timeout cannot abort a synchronous test method on its own, + /// so the engine is handed the test's token as well. + /// + private static string Drain(string generatorSource, bool async, CancellationToken cancellationToken) + { + var driver = async + ? """ + (async function () { + var results = []; + var it = makeIterator(); + var result; + do { + result = await it.next(); + results.push(result.value + ':' + result.done); + } while (!result.done && results.length < 20); + return results.join(' '); + })() + """ + : """ + var results = []; + var it = makeIterator(); + var result; + do { + result = it.next(); + results.push(result.value + ':' + result.done); + } while (!result.done && results.length < 20); + results.join(' '); + """; + + var engine = new Engine(options => options.CancellationToken(cancellationToken)); + return engine.Evaluate(generatorSource + Environment.NewLine + driver).UnwrapIfPromise().AsString(); + } + [Fact] public void GeneratorFunctionConstructorsInheritFromTheFunctionConstructor() { diff --git a/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs b/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs index 84c405b943..d147f4aaa4 100644 --- a/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs +++ b/Jint/Native/AsyncGenerator/AsyncGeneratorInstance.cs @@ -70,7 +70,13 @@ bool ISuspendable.IsResuming JsValue? ISuspendable.SuspendedValue => _suspendedValue; - object? ISuspendable.LastSuspensionNode => _lastYieldNode; + // The same reason as GeneratorInstance's identically shaped property, plus one an async + // generator has on its own: a delegation that COMPLETES also resumes through a full replay of + // the body (ResumeAfterDelegation), because the inner iterator's step settles on a later + // microtask and the stack that started the delegation is long gone. ClearDelegationIterator + // deliberately keeps _delegatingYieldNode across that hop so the replay re-enters at the yield*, + // which is exactly the node the enclosing statements need to be told about as well. + object? ISuspendable.LastSuspensionNode => _lastYieldNode ?? _delegatingYieldNode; bool ISuspendable.ReturnRequested => _returnRequested; diff --git a/Jint/Native/Generator/GeneratorInstance.cs b/Jint/Native/Generator/GeneratorInstance.cs index 21b36af2b3..4f4ffe0658 100644 --- a/Jint/Native/Generator/GeneratorInstance.cs +++ b/Jint/Native/Generator/GeneratorInstance.cs @@ -108,7 +108,19 @@ bool ISuspendable.IsResuming JsValue? ISuspendable.SuspendedValue => _suspendedValue; - object? ISuspendable.LastSuspensionNode => _lastYieldNode; + // A yield* delegation suspends AT the yield* expression, and its node is kept in + // _delegatingYieldNode rather than in _lastYieldNode: JintYieldExpression selects its delegation + // branch on the first field and its plain-yield branch on the second, so the two cannot share a + // slot. Only _lastYieldNode used to be published here, which made a resume taken in the middle + // of a delegation look to every resume-aware statement -- JintWhileStatement, JintForStatement, + // JintDoWhileStatement, JintIfStatement, JintSwitchStatement, JintTryStatement -- like a resume + // that had suspended nowhere. Each of them then re-ran its own test, and a test the delegated + // iteration had already falsified sent the generator down the other branch, abandoning the rest + // of the iteration it was actually in the middle of. The fallback cannot answer with a stale + // node: _lastYieldNode is cleared the moment the yield it names is resumed, so it is null for + // the whole time a delegation is in flight, and _delegatingYieldNode is cleared when the + // delegation ends. + object? ISuspendable.LastSuspensionNode => _lastYieldNode ?? _delegatingYieldNode; bool ISuspendable.ReturnRequested => _returnRequested; diff --git a/Jint/Runtime/ISuspendable.cs b/Jint/Runtime/ISuspendable.cs index adde6c8d81..6a2fca6412 100644 --- a/Jint/Runtime/ISuspendable.cs +++ b/Jint/Runtime/ISuspendable.cs @@ -31,6 +31,13 @@ internal interface ISuspendable /// The AST node where execution last suspended (yield or await expression). /// Unified property for tracking suspension location across all suspendable types. /// + /// + /// This is what tells a resume-aware statement that it is being re-entered rather than entered, + /// so it must not re-evaluate the test that chose the branch the suspension is inside; see + /// . A generator suspended + /// inside a yield* delegation reports the yield* expression, which is where the + /// specification says it is suspended. + /// object? LastSuspensionNode { get; } /// diff --git a/Jint/Runtime/Interpreter/Expressions/JintYieldExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintYieldExpression.cs index 09fa1114d8..7b208a5be2 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintYieldExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintYieldExpression.cs @@ -172,7 +172,19 @@ protected override object EvaluateInternal(EvaluationContext context) // Fall through to normal yield logic } - // Normal yield: evaluate argument and yield the value + // Normal yield: evaluate argument and yield the value. + // + // Reaching here is a *fresh* evaluation of this yield node, so whatever the node's previous + // evaluation produced stops being an answer to it. The memo above exists only to replay one + // evaluation - a statement that is re-executed from the top after a later suspension must + // not re-run the yields it already got past - and a loop that comes back round to the same + // node starts a new evaluation the memo has nothing to say about. Leaving the stale entry in + // place made `yield (yield* g())` inside a loop answer its second iteration with the first + // iteration's value without ever evaluating the operand, so the delegation was abandoned + // mid-flight (and, when the operand carried the loop's own decrement, the loop never ended). + generator?._yieldNodeValues?.Remove(_expression); + asyncGenerator?._yieldNodeValues?.Remove(_expression); + JsValue value; if (_argument is not null) {