feat: add AbortController and AbortSignal - #2025
Conversation
Install the DOM abort primitives as globals in every isolate, modeled on Node's internal/abort_controller.js: AbortController, and AbortSignal with the abort/timeout/any statics, onabort with HTML event-handler semantics, and WebIDL-shaped interfaces (enumerable members, Symbol.toStringTag, brand-checked accessors). The builtin (internal/abort-signal.js) runs from Events::Init right after the Event/EventTarget builtin it is layered on. Deviations from Node, documented in docs/abort-signal.md: no DOMException (default reasons are Error instances with name patched to AbortError/TimeoutError, the same stand-in performance.js and structured-clone.js use) and no WeakRef bookkeeping (a timeout() timer holds its signal until it fires; any() links source -> dependent strongly and unlinks as soon as either side aborts). Adds RangeError and NumberIsInteger to primordials and the eslint restriction lists, and a 20-spec Jasmine suite. Mirrors the same commit on the iOS runtime (NativeScript/ios#447).
Match Node's memory behavior: internal references never keep an unobservable signal alive and never drop an observable abort. - timeout() timers close over a WeakRef; a FinalizationRegistry cancels the pending native timer when the signal is collected. - any() links are WeakRefs in both directions with prune registries, so per-request composites never accumulate on a long-lived source and a composite whose sources all died stops being retained. - A gcPersistentSignals set strong-holds exactly the signals whose abort someone can still observe: live timeout signals and non-empty composites while they have abort listeners, plus timeout sources a composite follows until their timer fires. The listener accounting comes from a new symbol-keyed listener-mutation hook in events.js, called from every listener-list mutation path (add, remove, once-splice during dispatch) and handed to the abort builtin in a one-shot through its binding, so it cannot be bypassed via a captured EventTarget.prototype.addEventListener. Adds WeakRef/FinalizationRegistry captures to primordials and the eslint restriction lists, and 8 GC specs driven by __collect() plus a finalization-registry substrate canary. Mirrors the same commit on the iOS runtime (NativeScript/ios#447).
Add a sixth fixed wrapper parameter, `internals`: one plain per-isolate
object (stored in the BuiltinRealm per-runtime state) handed identically
to every builtin and reachable from nowhere else. Producers publish
during their init, consumers read during theirs, so the PrepareV8Runtime
ordering is the dependency graph and a missing key fails loudly at init.
Both existing ad-hoc channels migrate onto it: events.js publishes the
kListenerChanged hook key (read by abort-signal.js, previously a one-shot
relayed through the abort builtin's binding) and setListenerErrorReporter
(called by error-events.js, previously the _installListenerErrorReporter
one-shot on the app-reachable global target). No capability ever sits on
an app-reachable object anymore, even transiently.
Documented in the js README as an interim mechanism: if cross-builtin
needs outgrow one shared object, migrate to a Node-style private
internal-module tier (require("internal/...") resolved for builtins
only) and fold internals into it.
Mirrors the same commit on the iOS runtime (NativeScript/ios#447).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughChangesThe runtime adds global AbortController and AbortSignal
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR adds AbortController and AbortSignal runtime support without any identified concrete correctness, security, availability, or integration issue; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant App
participant AbortController
participant AbortSignal
participant EventTarget
App->>AbortController: create controller
AbortController->>AbortSignal: expose stable signal
App->>AbortController: abort(reason)
AbortController->>AbortSignal: set aborted state and reason
AbortSignal->>EventTarget: dispatch abort event
EventTarget-->>App: invoke abort listeners
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mirrors NativeScript/ios#447 on Android (same three commits, same file shapes).
What
Installs the DOM Standard's abort primitives as globals in every isolate (main and workers), modeled on Node's
internal/abort_controller.js:AbortController—controller.signal(stable identity) andcontroller.abort(reason?).AbortSignalextending the runtime'sEventTarget, withaborted,reason,throwIfAborted(), theabortevent, andonabortwith HTML event-handler semantics.new AbortSignal()throwsTypeError: Illegal constructor.AbortSignal.abort(reason?),AbortSignal.timeout(delay)(Node's delay validation), andAbortSignal.any(signals)(composite flattening, first-aborted-reason-wins, spec-ordered state flips before events fire).Symbol.toStringTag, brand-checked accessors (via private fields).How
The builtin (
test-app/runtime/src/main/cpp/js/abort-signal.js) runs fromEvents::Initimmediately after theEvent/EventTargetbuiltin it is layered on.This PR also introduces a sixth fixed wrapper parameter,
internals: one plain per-isolate object (aBuiltinRealmslot in the per-runtime state) handed identically to every builtin and reachable from nowhere app code can see — the private channel for cross-builtin capabilities. Both previously ad-hoc channels now ride it: thekListenerChangedhook key (events → abort-signal) andsetListenerErrorReporter(error-events → events), replacing the_installListenerErrorReporterone-shot that transiently sat on the app-reachable global target. Documented in the js README as an interim mechanism, with the intended end-state being a Node-style private internal-module tier (require("internal/…")for builtins only).RangeError,NumberIsInteger,WeakRef, andFinalizationRegistryare added to primordials (and the eslint restriction lists).GC contract (Node-equivalent, documented in
docs/abort-signal.md)Internal references never keep an unobservable signal alive and never drop an observable abort:
timeout()timers close over aWeakRef; aFinalizationRegistrycancels the pending native timer if the signal is collected first.any()links areWeakRefs in both directions with prune registries, so per-request composites never accumulate on a long-lived source, and a composite whose sources all died stops being retained.gcPersistentSignalsset strong-holds exactly the signals whose abort someone can still observe: live timeout signals and non-empty composites while they have abort listeners, plus timeout sources a composite follows until their timer fires. The listener accounting comes from a new symbol-keyed listener-mutation hook inevents.js(called from add, remove, and theonce-splice during dispatch), published on the builtin-onlyinternalschannel — so it cannot be bypassed via a capturedEventTarget.prototype.addEventListener.Deviation from Node
No
DOMExceptionin this runtime: default reasons areErrorinstances withnamepatched to"AbortError"/"TimeoutError", the same stand-inperformance.jsandstructured-clone.jsuse.Tests
null), throwing listeners not stopping dispatch,oncelisteners,onabortset/replace/clear, delay validation,any()flattening/dedup/no-double-fire, arbitrary iterables, toStringTag, brand checks.__collect(): a finalization-registry substrate canary, collectability of unobserved timeout signals and composites, survival (and delivery) for listened ones, a composite keeping a dropped timeout source alive until it fires, release of a listened composite once its last source dies, and release on last-listener removal.The spec files are byte-identical to the iOS suite; the only source divergence from iOS is one comment line naming the Android init path (
PrepareV8Runtime).Summary by CodeRabbit
New Features
AbortControllerandAbortSignalAPIs.throwIfAborted(), timeouts, and combining multiple signals.Bug Fixes
Documentation
Tests