ReviewOS

zig-utils/zig-js

Promises: make reaction scheduling OOM-atomic

#885
Closed chris opened this 9 days ago ·
9 days ago

Parent: #474 Related: #480, #493, #884

Problem

Promise reaction scheduling has fallible state changes on both sides of settlement:

  • settle commits the Promise state and moves every reaction out of the Promise before appending reaction jobs to the microtask queue. If one append returns OutOfMemory, already-appended jobs remain while the current and remaining reactions are discarded by cleanup. Native resolving functions have already set their shared once-only bit, so the exact settlement cannot be retried.
  • performThenReactions marks a rejected source handled and, for a late handler, publishes rejectionHandled before appending the already-settled reaction job. If that append fails, the handler never runs even though tracker state says handling succeeded.

Bounded-heap recovery makes this observable after the current evaluation aborts: dependent Promises may remain pending forever, some sibling reactions may run while later ones disappear, and a rejected source may lose its unhandled status without an installed handler.

#884 removes the allocator boundary from host rejection-tracker publication. This issue owns the distinct Promise-to-microtask scheduling transaction.

Required implementation

  • Establish an exact transaction between Promise state/reaction ownership and all required microtask queue publications.
  • Before consuming a once-only resolving capability or committing a settled-source handler, guarantee capacity for every job that must become observable, or use an allocation-free scheduling representation with proved ownership.
  • Preserve FIFO ordering across existing jobs, multiple reactions, thenable assimilation, async/generator continuations, shared-realm no-GIL producers, and reentrant microtask checkpoints.
  • Keep the canonical queue/Promise lock order explicit and compatible with concurrent marking, teardown, and bounded allocator recovery.
  • Do not partially enqueue a settlement batch, discard unscheduled reactions, reset once-only state for an unsafe retry, globally serialize JavaScript, cap reaction fanout, or swallow OOM.

Acceptance

  • Deterministic allocator-fault witnesses cover failure before the first job, between sibling jobs, and during an already-settled .then.
  • After every injected failure, Promise state, reaction ownership, dependent Promise state, rejection-tracker flags, and microtask membership agree.
  • Recovery delivers every successfully committed reaction exactly once in registration order, with no permanently pending dependent Promise caused by lost scheduling.
  • No-GIL concurrent attachment/settlement cannot deadlock, duplicate, reorder, or lose jobs.
  • Moving/concurrent GC, bounded-heap recovery, async/generator paths, private ABI, threadfuzz, full units, and suppression-free TSan pass.
  • Exact affected Test262 Promise/async coverage has zero regressions and documentation makes no unsupported OOM guarantee.

Security boundary

Reaction fanout and allocator pressure are script-controlled. OOM must fail closed at a transaction boundary rather than leave partially scheduled engine state that can be observed after recovery.

9 days ago

Follow-up ownership audit split existing queue-to-queue and batch-to-queue loss boundaries into #886. This issue remains scoped to Promise state/reaction ownership → initial microtask publication; #886 owns Thread-local queue transfer and abrupt-drain suffix restoration after jobs already exist. Neither scope relaxes the other.

9 days ago

Implementation checkpoint:

  • 4a1e5d82 introduced paired reaction ownership, single-descriptor settlement batches, queue reservations before [[AlreadyResolved]], OOM-atomic late .then, and deterministic failure/recovery witnesses.
  • 19e5d72b closes the remaining resolving race: GC roots are registered before the once-bit is consumed, and reservation OOM rechecks a concurrently committed bit. It also adds a 129-handler shared-realm race witness.

Evidence now green:

  • full normal units: 2,311 passed, 1 skipped, 0 failed, 0 leaked (2,312 linked tests)
  • focused allocator witness: normal + suppression-free TSan
  • concurrent attachment/resolution witness: normal + suppression-free TSan; all 129 callbacks observed exactly once
  • affected Test262 subtrees: 1,828 -> 1,828 passing, zero failures, identical before/after logs (built-ins/Promise, statement/expression async functions and async generators, async arrows)

Keeping this issue open until private ABI, broader async/generator integration, threadfuzz, and the full suppression-free TSan acceptance run are complete.

9 days ago

While validating #887's prepared Thread completion transaction, source review identified the next OOM ownership audit needed before this issue can close:

  • promise.runJob's .thenable branch calls nativeResolveReject and registers temporary roots with try after the job has already been dequeued. OOM at those preparation steps returns without invoking the thenable or settling its target.
  • Both Interpreter.runOneMicrotask and drainJobQueue clear the current job on that error; the batch path correctly restores only the untouched suffix (#886). A deterministic fault witness must check that this already-committed thenable target cannot be left pending after its sole adoption job is discarded.
  • The reaction branch also needs an exact check for OOM while publishing the dependent Promise's result after its handler has already run. Retrying a user handler would violate the once-only requirement; this needs prepared result/rejection publication ownership.
  • Native resolving closures reserve scheduling capacity before consuming their once-only bit, but resolution still performs fallible Get("then") work afterward. Its abrupt engine-error path needs the same audit, separately from an ordinary JS getter throw (which resolution already turns into rejection).

This is a source-level audit finding, not a new measured failure count or a completed fix. The final Thread completion source is currently frozen for validation; no unrelated Promise-job execution edits are being mixed into it. The prepared settlement API from #887 should be evaluated for reuse once that batch lands, with explicit current-job ownership and no user callback replay.

9 days ago

The reusable prepared settlement path is now landed: fix(threads): commit prepared asyncJoin completion (#887).

PreparedSettlement retains reserved queue capacity on resolution failure and exposes a rejection commit that does not grow Promise/queue storage. LockedSettlement is shared with ordinary Promise settlement, so the Thread fix does not maintain a second state/publication algorithm. The full Debug suite, 35 focused tests in Debug/ReleaseSafe/suppression-free TSan, and exact-parent Promise/async diagnostics pass; full evidence is in #887's landed update.

The previously noted runJob/native resolving-function OOM audit remains outstanding here. The Thread transaction only commits initial resolution or job ownership; it does not claim to fix allocation failure during later thenable/reaction job execution. Next work should establish exact current-job/result ownership and a deterministic fault witness before extending this API to those paths.

9 days ago

Job-execution checkpoint (implementation under validation):

  • The deterministic allocator witness reproduced a dequeued thenable target left pending after preparation OOM on 9fb57fa0.
  • The current change transfers the detached batch's reserved publication slot to each running job; the single-job/await path reserves before dequeue. Intrinsic reaction/thenable failure can therefore reject without another queue/root allocation and without replaying the handler.
  • Native resolution now commits failure after its shared once-only claim, and duplicate resolving functions check that claim before attempting fresh roots.
  • All 12 focused job checks pass in Debug, including an actual nursery move during a failing returned-thenable getter, exact reserved OOM identity, custom capability abrupt behavior, suffix order after consuming a publication slot, and retained adoption after a once-claimed thenable subsequently fails.

The full Debug/TSan suites, ReleaseSafe focused checks, exact-parent Promise/async diagnostics, threadfuzz, and ABI fixtures are still running sequentially. This is not a landed or completed acceptance claim.

A distinct host boundary exposed by the witness is tracked in #896: Context.evaluate/module checkpoints suppress unowned engine OOM even when the direct drain correctly reports it. That issue owns host error reporting and primary-error precedence; this issue owns Promise/job completion and publication.

9 days ago

Current #885/#895 candidate on d4c2a63e25ab577fcca3b5f93dbb079e28af3cbb has passed 51 distinct focused tests (57 executions because selections overlap) in each of Debug, ReleaseSafe, and suppression-free TSan.

Coverage includes the 11 new Promise job/OOM/root-relocation regressions, the 3 capped-admission regressions from #895, existing Thread startup/completion/asyncJoin checks, thenable resolving, prepared settlement, and microtask transfer/restoration tests. The previously measured setup-OOM witness leaves a pending target on the old implementation; the candidate completes that exact target without losing or replaying the untouched suffix.

The full Debug suite is now running with 2,349 linked tests across 10 shards. Separate clean-parent and Promise-only Test262 binaries are preserved for the per-commit before/after comparison. Lifecycle, fuzzing, ABI checks, and the full TSan suite remain pending; this is focused validation, not final acceptance.

9 days ago

Landed fix(promises): preserve dequeued job completion on OOM (#885), alongside #895. Both commits are authored as Chris, use conventional subjects and measured descriptions, and contain no trailers.

The dequeued job owns a publication slot before its callback runs. Intrinsic completion failures reject the exact target; custom capabilities retain ordinary abrupt behavior. Current-job results/errors and the reserved OOM value remain precise relocation roots. Batch restoration accounts for consumed publication slots without replaying callbacks.

Measured on the pre-rebase candidate: 51 distinct focused tests pass in each of Debug, ReleaseSafe, and suppression-free TSan; full Debug reports 2,348 passed, 1 skipped, 0 failed, 0 leaked. All 14 selected GIL/no-GIL PR-249 runs, 400 default fuzz programs, 300 verify programs, and the Debug ABI fixtures pass. Exact clean-parent / Promise-only / combined Test262 results are identical: Promise 677, async-function statements 74 and expressions 93, async-generator statements 301 and expressions 623, async-arrow expressions 60: 1,828 passes, zero flips or regressions in those subtrees.

The rebase over 7ba86e4c preserved both patches exactly (git range-diff reports =). Post-rebase focused/conformance checks and the remaining lifecycle, ABI, and full TSan matrix are still pending. The active mid-GC run uses its already-built pre-rebase binary; its provenance is kept separate. This issue remains open for final acceptance.

9 days ago

Final integration verification for fix(promises): preserve dequeued job completion on OOM (#885) is complete on the pushed engine source in 09279120. The already-measured publication/job-execution allocation faults, precise moving roots, once-only resolving claims, custom-capability behavior, and exact untouched suffix recovery now have the remaining full-suite TSan, lifecycle, and ABI coverage.

  • Final local Debug suite: 2,380 passed, one Linux-only skip, zero failures/leaks.
  • Public C, Home/private ABI lifecycle/readable-stream, and Bun C API extension fixtures/audits pass in Debug, ReleaseSafe, and unsuppressed TSan.
  • Linux CI 34287554681 passes every full Debug/TSan unit shard, no-GIL Debug/ReleaseSafe/TSan corpus shard, ABI job, and normal/TSan fuzz profile. The sole red job was the stale #903 reference diagnostic, reconciled separately in 656f4d81; this is not a claim that that whole CI run was green.
  • Exact paired Test262 diagnostics remain identical across the ten measured Promise/Atomics/async/module/weak subtrees: 2,893 passed, zero failures, zero flips/regressions.
  • Linux mid-GC: 900 programs from seed 1, zero failures; suppression-free TSan mid-GC: 90 programs, zero failures.

Closing this transaction issue. The earlier local incomplete-sweep observation is preserved and remains under investigation in #907; #896's remaining private exception-box admission audit and #906's finalization cleanup ownership work remain separate open tasks. No timeout, workload, expected failure, or sanitizer suppression was relaxed to obtain this evidence.

Sign in to comment on this issue.