fix: generator.return() properly handles yield in finally blocks

[taras]

Motivation

I'm trying to spike using structured concurrency in JavaScript with Effection to resolve the concurrency problems k6 users are experiencing.

#	Problem	K6 Issue	Impact
1	group() loses context across async boundaries	#2848 (grafana/k6#2848), #5435 (grafana/k6#5435)	Metrics get attributed to wrong groups when using async operations inside groups
2	WebSocket handlers lose async results	#5524 (grafana/k6#5524)	Fire-and-forget semantics - can't await results from WebSocket message handlers
3	Unhandled promise rejections don't fail tests	#5249 (grafana/k6#5249)	Silent failures - tests pass even when async code throws errors
4	No structured cleanup/teardown	(General limitation)	Resources leak, connections not closed properly on test abort or iteration end

To make the spike work, I need async clean up to work in finally block of function*. I might be able to make it work without it, but considering that this is ECMAScript spec, I figured it would be best solved by adding asynchronous clean up to Sobek.

Summary

This PR fixes a bug where generator.return() skips yield statements inside finally blocks, violating ECMAScript specification.

Fixes #114

The Problem

When generator.return(value) is called on a generator with a yield inside a finally block, Sobek incorrectly:

• Skips the yield entirely
• Immediately marks the generator as completed
• Returns {value: returnArg, done: true}

Expected behavior (per ECMAScript spec and V8/Node.js):

function* withCleanup() {
  try {
    yield 'working';
  } finally {
    yield 'cleanup';  // Should suspend here
  }
}

const gen = withCleanup();
gen.next();           // {value: 'working', done: false}
gen.return('X');      // {value: 'cleanup', done: false}  ← should suspend
gen.next();           // {value: 'X', done: true}         ← then complete

Actual behavior (before this fix):

gen.return('X');      // {value: 'X', done: true}  ← skips yield in finally!

The Solution

1. Yield handling during return sequence

New state fields on generatorObject:

• returning bool - tracks when we're in a return sequence
• retVal Value - stores the return value for use after all finally blocks complete

New methods:

• completeReturnYield() - handles yield suspension during finally block execution, mirroring the logic in step() for normal yields
• resumeReturn() - manages the return sequence across multiple next() calls when finally blocks yield, properly executing all finally blocks before completing

Modified next() method detects returning state and delegates to resumeReturn() instead of normal execution.

2. sp underflow fix for callback contexts

When generator.return() is called from K6's timer/WebSocket callback contexts, vm.sb can be corrupted to 0 by finally block code (native function calls temporarily modify sb for their own stack frames). This resulted in vm.sp = -1, causing a panic.

Fix: Save the caller's sp immediately after enterNext() (when vm.sb is still valid), and use that saved value in the completion cleanup:

// In resumeReturn(), right after enterNext():
callerSp := vm.sb - 1  // Save now, while vm.sb is still valid

// Later, in completion cleanup:
vm.sp = callerSp  // Use saved value instead of vm.sp = vm.sb - 1

3. Return-in-finally handling

When return is used inside a finally block during generator.return():

• Capture non-yield completion values when execution halts, so an explicit return in finally can override the original return argument
• Limit the finally-unwind loop to frames above the generator resume baseline (g.gen.tryStackLen), so the sentinel try frame is not consumed by the loop
• Only pop the synthetic pc==-2 callStack frame when it is actually present

4. Refactoring

Helper functions to reduce duplicated logic:

• captureReturnValue(retVal) - unified return-value precedence handling
• popSentinelCallFrame(vm) - unified sentinel frame cleanup
• Threaded callerSp into completeReturnYield() so both yield and non-yield return paths use the same stable caller SP restoration

Test Cases

test262-Format Tests (Primary)

Added 8 test262-format JavaScript test files in testdata/GeneratorPrototype/return/:

Test File	Scenario
try-finally-yield-triggered-by-return.js	Basic yield in finally during return
try-finally-yield-star-delegation-return.js	yield* delegation with finally yields
try-finally-nested-yields-triggered-by-return.js	Nested try-finally with yields
try-finally-yield-resume-value-during-return.js	Yield resume values in finally
try-finally-throw-before-yield-during-return.js	Exception before yield in finally
try-finally-throw-after-yield-during-return.js	Exception after yield in finally
try-finally-yield-star-during-return.js	yield* in finally during return
try-finally-return-overrides-during-return.js	Return in finally overrides value

These tests follow the tc39/test262 format and are candidates for upstream contribution.

Go Tests (Go-specific integration)

yield_return_callback_test.go:

• Callback context tests for sp underflow scenario
• Return+throw callbacks with job queue validation

These test Go-specific callback integration (SetAsyncContextTracker) that cannot be tested via JavaScript alone.

test262 Coverage Analysis

I examined test262 (commit cb4a6c8) and found this is a coverage gap in test262 itself - no existing test covers the scenario where .return() triggers a finally block containing a yield.

Existing test262 tests for GeneratorPrototype/return/ (22 tests):

Category	Tests	What they cover
State tests	from-state-suspended-start.js, from-state-completed.js, from-state-executing.js	Return from different generator states
Metadata	length.js, name.js, property-descriptor.js, not-a-constructor.js	Method properties
Error handling	this-val-not-object.js, this-val-not-generator.js	Invalid this values
try-catch	4 tests	.return() with try-catch (no finally)
try-finally	try-finally-before-try.js, try-finally-following-finally.js	Paused before/after try-finally
try-finally-within-try.js	Generator paused in try, .return() runs finally	But finally has NO yield
try-finally-within-finally.js	Generator already paused at yield in finally	Then .return() called - different scenario
Nested	5 tests	Various nested try-catch-finally combinations

Gap: No test covers this path:

paused in try → .return() → triggers finally → finally contains yield → suspends with done: false

Our tests fill this gap:

All 8 of our custom tests cover scenarios where .return() triggers execution of finally blocks that contain yields, which is the buggy path this PR fixes.

Additional Bug Fixed: Return-in-finally panic

While implementing the main fix, I discovered an additional bug: when a return statement appears inside a finally block during generator.return(), Sobek panicked.

Root cause:

• Corrupted return-unwind state when finally blocks complete without yielding
• Try frame consumption below the generator resume baseline
• Incorrect callStack frame cleanup

Fixed by:

• Capturing non-yield completion values so explicit return in finally can override the original return argument
• Limiting try frame unwinding to frames above g.gen.tryStackLen
• Only popping the synthetic pc==-2 callStack frame when present

Cross-Engine Validation

The test262-format tests were manually validated against all major JavaScript engines:

Engine	Tool	Result
Sobek (this PR)	go test	✅ All 8 tests PASS
V8	Node.js v22+	✅ All 8 tests PASS
V8	Deno	✅ All 8 tests PASS
V8	Playwright Chromium	✅ All 8 tests PASS
SpiderMonkey	Playwright Firefox 146	✅ All 8 tests PASS
JavaScriptCore	macOS jsc CLI	✅ All 8 tests PASS
JavaScriptCore	Playwright WebKit 26	✅ All 8 tests PASS

Validation Methodology

1. Sobek: go test -run "TestGeneratorPrototypeReturn" -v
2. Node.js/Deno: Ran each test file with a minimal assert harness
3. JavaScriptCore CLI: Used /System/Library/Frameworks/JavaScriptCore.framework/Versions/Current/Helpers/jsc
4. Playwright Browsers: Used Playwright to execute tests in Chromium, Firefox, and WebKit browser contexts

All engines produce identical results, confirming the implementation matches ECMAScript specification.

Verification

# Run all generator tests
go test -run "TestGenerator" -v
# All tests pass

# Run full test suite  
go test ./...
# Full test suite passes

# Run new test262-format tests specifically
go test -run "TestGeneratorPrototypeReturn" -v
# All 8 test262-format tests pass

K6 integration validation:

• @effectionx/k6 - K6 extension for Effection - includes tests and API for using structured concurrency in K6. It requires changes in this PR.

[mstoykov]

I've opened an upstream PR that increases the test262 and there is a test there that fails without the changes here.

I would prefer to get this merged in goja and then merge but if that takes longer than Monday I will make a new PR from https://github.com/grafana/sobek/compare/bumpTest262. You can potentially use that as your base for now to fix any future conflicts and remove the exceptions in the tc39_test.go file that are stopping the tests.

I definitely prefer to update it than to copy-paste test back from it:)

[taras]

I'm not in a major hurry. I'll check with folks at work if they want to start using the @effectionx/k6 package, but that might take a while anyway.

Let me know if there is anything I can do to help.

[mstoykov]

You can now rebase on the main branch

[taras]

Rebased

[mstoykov]

I think the tests you've added under testdata are direct copies from the tc39 suite, so you can now remove them and related changes. We do run the test262 test suite. You need to run ./.tc39_test262_checkout.sh

Also of note is that we try to keep in sync(;)) with goja from which this is a fork. This means that either we or you (if you desire will have to make a PR for there as well. Sometimes we prefer to make you do the original change there, but I am no certain that will be faster in this case

[taras]

Sorry I didn't make this clearer earlier but the tests under testdata/GeneratorPrototype/return/ are new test not covered by test262.

test262 coverage

Category	Coverage
.return() before generator starts	✅ from-state-suspended-start.js
.return() on completed generator	✅ from-state-completed.js
.return() with try-catch (no finally)	✅ 4 tests
.return() paused before/after try-finally	✅ 2 tests
.return() triggers finally with no yield	✅ try-finally-within-try.js
.return() when already paused in finally	✅ try-finally-within-finally.js
.return() triggers finally that contains yield	⚠️ - introduced in this PR
.return() with yield* in finally	❌
.return() with nested finally yields	❌
.return() with throw after yield in finally	❌
.return() with return-in-finally override	❌

I ran the new tests against the following environments to verify that they are in fact supported by the platforms.

Engine	Tool	Result
Sobek (this PR)	go test	✅ All 8 tests PASS
V8	Node.js v22+	✅ All 8 tests PASS
V8	Deno	✅ All 8 tests PASS
V8	Playwright Chromium	✅ All 8 tests PASS
SpiderMonkey	Playwright Firefox 146	✅ All 8 tests PASS
JavaScriptCore	macOS jsc CLI	✅ All 8 tests PASS
JavaScriptCore	Playwright WebKit 26	✅ All 8 tests PASS

I wasn't sure if you'd prefer for me to contribute them to test262 first. I never interacted with TC39, so I wasn't sure if I should go down that route.

[mstoykov]

I think if those are new tests - they should be added to the test262 suite then. I kind of do not want to try to be certain they are actually following the specificaiton or if just happens to be what all of thsoe engines do - unlikely as that is

[taras]

That makes sense. Ok, I'll add them upstream. Wish me luck :)