| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
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.
|
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: 60b0333c-963b-4f32-b17b-71ba2ced859e 📥 CommitsReviewing files that changed from the base of the PR and between 9399899 and 495f751. 📒 Files selected for processing (10)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 Walkthrough WalkthroughThe runtime adds global AbortController and AbortSignal implementations with abort, timeout, composite-signal, event, validation, and documentation support. Runtime initialization and js2c build inputs now include the new builtin. ChangesAbortSignal runtime support
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to 495f7 The PR adds AbortController and AbortSignal support with targeted tests and documentation; no actionable merge-blocking risk remains, so it is merge-ready after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant EventsInit
participant BuiltinLoader
participant AbortSignalBuiltin
participant GlobalThis
EventsInit->>BuiltinLoader: RunBuiltin(kAbortSignal)
BuiltinLoader->>AbortSignalBuiltin: load internal/abort-signal.js
AbortSignalBuiltin->>GlobalThis: expose AbortController and AbortSignal
Suggested reviewers: nathanwalker Poem 🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)
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. ❤️ ShareComment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
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.
…ilities
Add a sixth fixed wrapper parameter, `internals`: one plain per-isolate
object (stored via Caches::StateFor) handed identically to every builtin
and reachable from nowhere else. Producers publish during their init,
consumers read during theirs, so the Runtime::Init 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.
| Back | FazBrowse Home | New Git URL |
What
Installs the DOM Standard's abort primitives as globals in every isolate (main and workers), modeled on Node's internal/abort_controller.js:
How
The builtin (NativeScript/runtime/js/abort-signal.js) runs from Events::Init immediately after the Event/EventTarget builtin it is layered on.
This PR also introduces a sixth fixed wrapper parameter, internals: one plain per-isolate object (a Caches::StateFor slot) 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: the kListenerChanged hook key (events → abort-signal) and setListenerErrorReporter (error-events → events), replacing the _installListenerErrorReporter one-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, and FinalizationRegistry are 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:
Deviation from Node
No DOMException in this runtime: default reasons are Error instances with name patched to "AbortError" / "TimeoutError", the same stand-in performance.js and structured-clone.js use.
Tests
Full iOS suite passes with 0 failures.