Skip to content

Adopt resolve-with-thenable on the event loop (fix async-ordering flake)#983

Merged
nickna merged 1 commit into
mainfrom
wrk/promise-thenable-eventloop-determinism
Jun 27, 2026
Merged

Adopt resolve-with-thenable on the event loop (fix async-ordering flake)#983
nickna merged 1 commit into
mainfrom
wrk/promise-thenable-eventloop-determinism

Conversation

@nickna

@nickna nickna commented Jun 27, 2026

Copy link
Copy Markdown
Owner

Background

Follow-up to the interpreted-Test262 flake investigation. After PR #982 removed the orphan-thread amplifier, I profiled the residual flake: the suite is fast (11,303/11,385 tests <50ms, 0 genuine timeouts in isolation), and worker oversubscription alone never reproduces a flip — but under genuine full-machine CPU saturation (a 12-core burner), one test reproducibly flipped: Promise/resolve-thenable-deferred.js (Fail→Pass). A Fail→Pass flip hard-fails the baseline gate ("new pass"), so this async-ordering non-determinism — not timeouts — is the residual run-to-run flake.

Root cause

Resolving a promise with another promise — resolve(thenable), or await of a promise that was resolved with a promise — adopted the inner promise via innerTask.ContinueWith(..., TaskScheduler.Default) (SharpTSPromiseClass.RunExecutor and Interpreter.AdoptThenable). That's a thread-pool hop: it settles the outer promise off the event-loop thread with nothing on the callback queue. When the inner promise is already settled (the "deferred" case: a pending promise resolved with an already-resolved one), the event loop can observe !HasActiveHandles && _callbackQueue.Count == 0 and exit before the adoption settles, so the outer promise's .then reaction never runs. Whether the loop wins that race depends on scheduling, so the test flips Pass/Fail under load.

(The CLI happened to win the race consistently; the Test262 runner — which runs each test on a dedicated thread — lost it in isolation, which is why the test was "pinned Fail.")

Fix (interpreter-only)

Route the adoption through the event loop via a new Interpreter.AdoptInnerPromise: deliver the settle as an event-loop callback using TaskContinuationOptions.ExecuteSynchronously. So:

  • an already-settled inner enqueues the settle callback inline (during resolve, before the loop starts) → the loop never starts idle → deterministic;
  • a never-settling inner enqueues nothing → the loop exits normally, matching Node (a pending adoption does not by itself keep the program alive);
  • the settle always runs on the event-loop thread, and completing the outer promise synchronously posts its .then continuations back onto the loop.

Used by both the new Promise executor resolve and the await-thenable path. SettleFromTask (subclass capability adoption) already used ExecuteSynchronously and is unchanged.

Validation

  • Promise/resolve-thenable-deferred.js: Fail → Pass, deterministic with and under the 12-core burner (the condition that originally flipped it). Test262 interpreted baseline: +1, 0 regressions.
  • Full xUnit suite 14412 / 0.
  • 2 new interpreter-only regression tests (PromiseMethodTests.ResolveWithThenable_*).

Notes

…erminism)

Resolving a promise with another promise (resolve(thenable) / await of a
promise resolved with a promise) adopted the inner promise via
`innerTask.ContinueWith(..., TaskScheduler.Default)` — a thread-pool hop that
settled the outer promise off the event-loop thread with nothing queued. When
the inner was already settled (e.g. a pending promise resolved with an
already-resolved one), the event loop could observe "no active handles AND empty
callback queue" and exit before the adoption settled, so the outer promise's
.then reaction never ran. Whether the loop won that race depended on machine
load, so Test262 Promise/resolve-thenable-deferred flipped Pass/Fail run-to-run.

Route the adoption through the event loop instead (new
Interpreter.AdoptInnerPromise): the settle is delivered as an event-loop callback
via ExecuteSynchronously, so an already-settled inner enqueues it inline (the
loop never starts idle) and a never-settling inner enqueues nothing (the loop
exits normally, matching Node — a pending adoption does not keep the program
alive). Used by both the new-Promise executor resolve (SharpTSPromiseClass) and
await-thenable adoption (AdoptThenable).

Interpreter-only — compiled mode emits its own event loop and has the same gap,
tracked separately. Test262 interpreted baseline: resolve-thenable-deferred
Fail->Pass, 0 regressions. Full xUnit suite 14412/0; +2 regression tests.
@nickna nickna merged commit 19e1e01 into main Jun 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant