Skip to content

feat: per-runtime EventLoop - v8 platform tasks + two-lane scheduler (CFRunLoop timer/source) - #439

Merged
NathanWalker merged 10 commits into
mainfrom
feat/v8-platform-event-loop
Aug 12, 2026
Merged

feat: per-runtime EventLoop - v8 platform tasks + two-lane scheduler (CFRunLoop timer/source)#439
NathanWalker merged 10 commits into
mainfrom
feat/v8-platform-event-loop

Conversation

@edusperoni

@edusperoni edusperoni commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Problem

Nothing pumps the V8 platform's foreground task queues. v8::platform::PumpMessageLoop only ran inside the two debugger pause loops (JsV8InspectorClient/WorkerInspectorClient). In normal execution, every task V8 posts to its foreground runner just sat there:

  • Atomics.waitAsync promises never resolved (their wakeup is a foreground task).
  • GC finalization / heap tasks never ran.
  • Streaming-compilation merge-backs never landed.

Separately, "get work onto the JS thread" had grown four bespoke mechanisms: tns::ExecuteOnRunLoop (CFRunLoopPerformBlock + wakeup), two raw perform-block bypasses in NativeScriptException.mm, Timers.cpp's per-timer CFRunLoopTimers, and ConcurrentQueue's runloop source for inbound worker messages. No shared ordering domain, and no per-entry microtask handling: with MicrotasksPolicy::kAuto, work that resolves a promise without entering JS (exactly the Atomics.waitAsync shape) stalls until unrelated JS runs.

Change: a per-runtime EventLoop with two lanes

Port of the Android runtime's event loop (NativeScript/android#2003), same API and contracts, CFRunLoop-native implementation. Each Runtime owns an EventLoop, bound to its home thread in CreateIsolate, accepting posts from any thread and buffering until bound. Work is routed by ordering contract:

Ordered lane — work whose ordering is observable against app-level runloop work. Every post is an anonymous "task due" token on ONE CFRunLoopTimer armed at the earliest pending due time; a due-now token gets a past fire date and fires on the next runloop pass, one token per fire. This keeps tokens in fire-date order with foreign NSTimers (exactly like the per-timer CFRunLoopTimers this replaces) and guarantees a full runloop pass between consecutive tokens — measured via the runtime's own before-waiting rejection-drain observer, a rejection injected mid-chain drains between steps (step 5 of 50), identical to the NSTimer-chain reference, so self-rescheduling setTimeout(0) chains starve neither rendering commits nor the autorelease pool nor the drain. Each token runs the earliest due item across the ordered entries and the OrderedTaskSource (Timers) — one due-ordered domain. First producer: __ns__queueMacrotask(cb), the seam future spec'd macrotasks (e.g. performance-observer callbacks) will use.

Internal lane — work in its own ordering domain: v8 platform foreground tasks (Atomics.waitAsync wakeups, GC tasks), worker→parent messages, exception/rejection deliveries. Rides a version-0 CFRunLoopSource plus one CFRunLoopTimer for delayed work. The source's perform callback runs exactly one due entry and re-signals itself while more are due, so bursts interleave with other runloop work instead of draining in one go — the CFRunLoop analogue of Android's one-eventfd-unit-per-poll. (Because the drain is queue-state-driven rather than unit-counted, Android's eventfd unit-accounting hazard has no iOS equivalent; the regression test is ported anyway.)

NativeScriptPlatform wraps the default platform (workers/jobs/time/tracing delegate to libplatform) and serves GetForegroundTaskRunner(isolate) from the isolate's EventLoop. The loop starts unbound and buffers (v8 requests the runner during Isolate::New); binding flushes. Each executed non-bare entry runs under Locker + isolate/handle/context scopes and ends with a microtask checkpoint — the kAuto-stall fix. Shutdown drops queued work and late posts.

Lane mapping rationale (vs Android's Java Handler + eventfd): iOS has no single app-level message queue — CFRunLoop services performed blocks and timers in different phases, so the ordered lane cannot be FIFO with both. It rides the timer phase: that is where the pre-existing per-timer CFRunLoopTimers lived, so cross-ordering against app NSTimers is preserved (fire-date order), and a due timer still yields a full runloop pass between fires. (A performed-block carrier was prototyped and rejected: blocks would be FIFO with ExecuteOnRunLoop-style posts, but same-instant ordering against NSTimers would flip phase — timers were never ordered against blocks on main either, so the timer phase is the least-surprise choice.) The internal lane is a version-0 CFRunLoopSource: the natural "own ordering domain" primitive, serviced with no ordering contract against blocks or timers, exactly the lane's contract. Both lanes are real mechanisms, not a collapsed queue, so subsystem code ports between the platforms without rethinking which lane it belongs to.

Timers merged into the ordered lane (with tombstones)

The per-timer CFRunLoopTimers (and the retain/release TimerContext machinery) are gone; timers post anonymous tokens through the EventLoop, and the token drain runs the earliest due item across timers and ordered macrotasks under one Locker acquisition (Timers implements OrderedTaskSource::RunIfEarliest). clearTimeout recalls the timer's token outright when it is still future-due (removing the armed wakeup — the same immediate cancellation CFRunLoopTimerInvalidate gave), and leaves a tombstone when the token has already matured (or was posted due-now), so no token gains surplus capacity to run a later-scheduled item ahead of foreign runloop work queued between the two token positions. The token wakeup timer runs one matured token per fire (re-armed with the past due time, firing again next pass), so foreign timers and blocks due between two matured tokens interleave by due time — the one-token-one-message fairness of the Android MessageQueue design. Interval catch-up semantics (NextTime: realign to startTime + k*frequency, matching the skipped-fire behavior of repeating CFRunLoopTimers) are preserved; intervals re-arm before the callback runs so a throwing callback can't kill them.

Consolidated onto the loop

  • Worker.mm worker→parent message delivery and WorkerWrapper's two error-forwarding paths: ExecuteOnRunLoop → internal lane (1:1, including drop-after-shutdown; the sync flavor is preserved via a posted semaphore and Post*'s new bool posted/dropped return).
  • ClassBuilder's cross-thread GcProtect/GcUnprotect: raw perform blocks → bare internal-lane entries (the closures do their own Locker ceremony; no per-entry checkpoint added on this hot path).
  • NativeScriptException's two deferred @throw sites: bare entries that run with no V8 scopes and outside the loop's exception guard, so the NSException unwinds into the runloop frame exactly as before (the guard deliberately has no catch(...) — on Darwin that would swallow NSExceptions).
  • Inspector pause loops: PumpMessageLoopRunNestableV8Tasks() — bounded to entries present at call time; only nestable v8 tasks run while JS frames are on the stack; everything else fires from its own wakeup after resume.
  • ModuleInternal's top-level-await spin pumps nestable tasks each iteration under the same contract (Atomics.waitAsync wakeups are non-nestable per v8's futex-emulation, so a TLA module blocked on one returns a TDZ namespace from synchronous require() and completes from the loop right after the turn — on main it never completed; dynamic import settles normally).
  • Dead SetTimeout.cpp/.h (never initialized, main-queue-hardcoded) deleted.
  • Left as-is: ConcurrentQueue (parent→worker inbound delivery on the worker's own loop — the Android port also kept its equivalent); the rejection drain stays observer-based (kCFRunLoopBeforeWaiting); the inspector's hardcoded CFRunLoopGetMain() waits.

Teardown / isolate-pointer reuse

~Runtime shuts the loop down inside its Locker block before any handle disposal; posts from other threads start dropping there. The platform registry entry is removed by a matched erase (IsolateDisposed(isolate, loop)) so a worker isolate that reuses a disposed isolate's address can't be evicted by the previous tenant's late destructor; CreateIsolate additionally refreshes a stopped loop found under its key, and the v8 task runner resolves the loop through the registry on every post so a refresh redirects already-handed-out runners.

Post-review hardening

CodeRabbit + an independent deep review; fixes applied: inspector pause loops resolve the loop via a non-creating lookup (a stale pause iteration could otherwise mint a registry entry for a disposed isolate); PostOrderedToken returns the key it actually recorded and clearTimeout recalls by it (an overdue timer's clear otherwise degraded to tombstone + spurious wakeup); the sync worker-post wait is released via an RAII completion even when Shutdown drops the entry unrun; the bind-time flush replays buffered tokens under their original keys so the recall contract survives pre-bind posts; null guards in Timers::Init/postToken.

Known/accepted notes from review: v8 tasks posted during Isolate::New for a worker that reuses a disposed isolate's address land in the stale stopped loop and are dropped until the immediate post-creation refresh (unfixable without a v8 hook; same shape as android#2003, and at-worst-parity with main where no foreground task ever ran). The tombstone branch is only reachable on an exact due-time tie (sub-nanosecond double), so the tombstone specs exercise token recall; the machinery is verified by review. Same open question as android#2003: microtask checkpoints now run after nestable tasks during debugger pauses (Blink parity).

Not in this PR

  • Microtask policy is untouched (kAuto; each loop entry adds an explicit checkpoint). The kExplicit / continuations-always-resolve-on-the-runtime-thread design builds on this seam later.
  • The NAPI branch (feat: Node-API (napi) surface for plugin developers #437) posts TSFN/async-work/finalizer blocks through ExecuteOnRunLoop; it migrates onto this loop separately.
  • ExecuteOnRunLoop itself stays exported (no in-repo callers remain).

Tests

EventLoopTests.js (+ echo/waitAsync workers): Atomics.waitAsync notify/timeout/mismatch/promise-chain — the async cases hang without this change; natively-resolved promise microtasks run before a later macrotask (the kAuto-stall assertion); __ns__queueMacrotask async delivery, runs-after-microtasks, FIFO among itself and with __ns__setTimeout(0), TypeError on non-function; clear-tombstone specs (clear + re-schedule doesn't reorder, overdue clear doesn't fire a later timer early, interval survives unrelated clear); worker specs (message ordering, waitAsync inside a worker's own loop, reply racing an overdue waitAsync timeout, terminate-with-queued-work, 8× churn exercising the registry-refresh path).

Also covered: tombstone probes posting a real CFRunLoopPerformBlock between two timer tokens, an NSTimer interleave spec (a foreign timer due between two overdue native timers fires between them), and a yield spec driving a 50-step immediate-timer chain while asserting a foreign NSTimer, a main-queue block and the before-waiting rejection drain all land mid-chain.

Full suite: 1077 specs green before, 1101 green after (same simulator).

Wrap the default platform in NativeScriptPlatform and serve
GetForegroundTaskRunner from a per-isolate EventLoop bound to the
runtime's CFRunLoop, so v8 foreground tasks (Atomics.waitAsync wakeups,
GC tasks, streaming-compilation merge-backs) actually run instead of
sitting in never-pumped libplatform queues. Every non-bare entry ends
with a microtask checkpoint, fixing the kAuto stall for promises
resolved from pure native.

Two lanes by ordering contract: the ordered lane rides
CFRunLoopPerformBlock tokens (FIFO with foreign performed blocks) and
backs the new __ns__queueMacrotask seam; the internal lane rides a
version-0 CFRunLoopSource that runs one due entry per pass and
re-signals while more are due, hosting v8 tasks, worker->parent
messages and the deferred-throw sites. The registry survives
isolate-pointer reuse across worker churn via matched erase plus
refresh-on-create, and the v8 runner resolves its loop per post.

Migrated 1:1 onto the loop: Worker/WorkerWrapper ExecuteOnRunLoop call
sites (internal lane), ClassBuilder's cross-thread GcProtect/GcUnprotect
and NativeScriptException's deferred @throw blocks (bare entries, own
ceremony, outside the loop's exception guard), and both inspector pause
loops (PumpMessageLoop -> RunNestableV8Tasks, nestable-only, bounded).
Replace the per-timer CFRunLoopTimers (and the TimerContext
retain/release machinery) with the Android runtime's token scheme:
scheduled timers keep an exact sub-millisecond sorted list and post one
anonymous due token through the EventLoop, whose drain runs the
earliest due item across timers and ordered macrotasks - one due-ordered
domain, FIFO with performed blocks on the same runloop.

clearTimeout/clearInterval leave a tombstone whose own token consumes
it as a no-op, so no token gains surplus capacity to run a
later-scheduled item ahead of foreign runloop work queued between the
two token positions. Intervals realign to startTime + k*frequency and
re-arm before the callback runs, preserving the skipped-fire and
throwing-callback behavior of the repeating CFRunLoopTimers this
replaces.
Atomics.waitAsync notify/timeout/mismatch/promise-chain (the async
cases hang without the event loop), the kAuto-stall assertion
(microtasks of a natively-resolved promise beat a later macrotask),
__ns__queueMacrotask ordering and argument validation, clear-tombstone
coverage, and worker specs: message ordering, waitAsync inside a
worker's own loop, a reply racing an overdue waitAsync timeout,
terminate-with-queued-work, and create/terminate churn exercising the
isolate-pointer-reuse registry path.
clearTimeout on a future-due timer now removes its anonymous token from
the loop's own bookkeeping and re-arms the wakeup timer, restoring the
immediate-cancellation behavior of the CFRunLoopTimerInvalidate path
this branch replaced - a cleared long timer no longer wakes the runloop
at its original due time just to swallow a tombstone. Tokens are
anonymous and counted, so removing one token plus one sorted entry
keeps slots 1:1 regardless of which producer's token is physically
removed. Tokens already sent out as performed blocks cannot be
recalled; those slots keep the tombstone.

Also run only ONE matured token per ordered-timer fire, re-arming with
the already-past due time so the remainder fires on the next runloop
pass: foreign timers and blocks due between two matured tokens now
interleave by due time instead of waiting out a batch, matching the
one-token-one-message fairness of the Android MessageQueue design.
Two tombstone probes posting a real CFRunLoopPerformBlock between two
timer tokens (the cleared timer's token must not run the later timer or
a queued macrotask ahead of the native block), and an NSTimer
interleave spec: a foreign timer due between two overdue native timers
fires between them, not after the batch.
Route every ordered token through the single token CFRunLoopTimer - a
due-now token gets a past fire date and fires on the next runloop pass -
instead of delivering due-now tokens as performed blocks. This keeps
same-instant tokens in fire-date order with foreign NSTimers, exactly
like the per-timer CFRunLoopTimers the event loop replaced, and each
fire yields a full runloop pass, so a self-rescheduling setTimeout(0)
chain cannot monopolize block servicing.

The yield spec measures this through the runtime's own before-waiting
observer (the rejection drain): a rejection injected mid-chain must be
reported while the chain is still running, alongside a foreign NSTimer
and a GCD main-queue block. CFRunLoopObserver handlers created from JS
never fire, so the drain is the only trustworthy probe.
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cdcfebbb-6b69-4788-a8eb-782478dc6f63

📥 Commits

Reviewing files that changed from the base of the PR and between dc2081a and 7c72b7f.

📒 Files selected for processing (9)
  • NativeScript/inspector/JsV8InspectorClient.mm
  • NativeScript/inspector/WorkerInspectorClient.mm
  • NativeScript/runtime/EventLoop.h
  • NativeScript/runtime/EventLoop.mm
  • NativeScript/runtime/Runtime.mm
  • NativeScript/runtime/Timers.cpp
  • NativeScript/runtime/Timers.hpp
  • NativeScript/runtime/WorkerWrapper.mm
  • TestRunner/app/tests/EventLoopTests.js
🚧 Files skipped from review as they are similar to previous changes (7)
  • NativeScript/inspector/JsV8InspectorClient.mm
  • NativeScript/runtime/WorkerWrapper.mm
  • TestRunner/app/tests/EventLoopTests.js
  • NativeScript/runtime/EventLoop.h
  • NativeScript/runtime/Runtime.mm
  • NativeScript/runtime/EventLoop.mm
  • NativeScript/inspector/WorkerInspectorClient.mm

📝 Walkthrough

Walkthrough

The runtime adds a CFRunLoop-based EventLoop and connects it to V8 foreground tasks, timers, workers, exceptions, inspectors, top-level await, and ordered macrotasks. The previous SetTimeout implementation is removed. New tests cover scheduling and lifecycle behavior.

Changes

Runtime event loop integration

Layer / File(s) Summary
EventLoop scheduler
NativeScript/runtime/EventLoop.h, NativeScript/runtime/EventLoop.mm
Adds internal and ordered task lanes, delayed scheduling, cancellation, timer-source arbitration, V8 task draining, exception handling, thread binding, and shutdown.
Platform and runtime wiring
NativeScript/runtime/NativeScriptPlatform.*, NativeScript/runtime/Runtime.*, NativeScript/runtime/Caches.h, v8ios.xcodeproj/project.pbxproj
Maps isolates to event loops, manages loop lifecycle, exposes GetEventLoop() and __ns__queueMacrotask, and updates project sources while removing SetTimeout.
Runtime feature migration
NativeScript/runtime/Timers.*, NativeScript/runtime/Worker.*, NativeScript/runtime/WorkerWrapper.mm, NativeScript/runtime/NativeScriptException.mm, NativeScript/runtime/ClassBuilder.mm, NativeScript/runtime/ModuleInternal.mm, NativeScript/inspector/*
Routes timers, worker callbacks, deferred throws, GC callbacks, top-level-await polling, and inspector pause tasks through the event loop.
Event-loop validation
TestRunner/app/tests/EventLoopTests.js, TestRunner/app/tests/*Worker.js, TestRunner/app/tests/esm/*, TestRunner/app/tests/index.js
Adds coverage for foreground tasks, macrotask ordering, timer tombstones, top-level await, worker messaging, wakeups, termination, and worker churn.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Runtime
  participant NativeScriptPlatform
  participant EventLoop
  participant V8
  Runtime->>NativeScriptPlatform: Register isolate event loop
  V8->>NativeScriptPlatform: Request foreground task runner
  NativeScriptPlatform->>EventLoop: Post V8 task
  EventLoop->>V8: Run task and microtasks
Loading

Possibly related issues

Possibly related PRs

  • NativeScript/ios#386: Changes the same inspector pause-loop implementations and nested V8 task processing.
  • NativeScript/ios#409: Changes related runtime, cache, exception, and event-loop scheduling paths.
  • NativeScript/ios#437: Uses related runtime-loop integration in Runtime and ModuleInternal.mm.

Suggested reviewers: nathanwalker

Poem

A rabbit sends tasks through the loop,
V8 work joins the queue in a group.
Timers wait in ordered rows,
Workers answer as each task goes.
Old timeout code sleeps in hay.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: a per-runtime EventLoop, V8 platform task handling, and a two-lane CFRunLoop scheduler.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Never initialized (its Init call has been commented out since the
timers rework) and hardcoded to the main queue; the event loop's
ordered lane is the replacement seam.
The module evaluation spin only performed microtask checkpoints, so an
await whose resolution arrives as a v8 foreground task could never
settle within the wait. Drain the event loop's nestable tasks each
iteration - JS frames are on the stack, so like the inspector pause
loops non-nestable tasks (e.g. Atomics.waitAsync wakeups, which v8
posts as non-nestable) stay queued and run from their own wakeups
after the turn; the specs pin both behaviors.
@edusperoni
edusperoni force-pushed the feat/v8-platform-event-loop branch from da59eec to dc2081a Compare August 12, 2026 18:24
@edusperoni
edusperoni marked this pull request as ready for review August 12, 2026 19:07

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
NativeScript/runtime/ClassBuilder.mm (1)

321-342: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Inconsistent null handling for GetEventLoop() across migrated call sites. WorkerWrapper::PostToRuntimeLoop (NativeScript/runtime/WorkerWrapper.mm:26-29) and ScheduleDeferredThrow (NativeScript/runtime/NativeScriptException.mm:230-233) both treat a null loop as reachable, but these three migrated sites dereference the result directly. Apply the same guard at each site.

  • NativeScript/runtime/ClassBuilder.mm#L321-L342: check Runtime::GetRuntime(isolate) before RuntimeLoop(), and check GetEventLoop() before PostInternalBare. Apply the same change to the swizzled release at lines 379-384.
  • NativeScript/runtime/Worker.mm#L314-L314: store runtime->GetEventLoop() in a local, return early when it is null, then call PostInternal.
  • NativeScript/runtime/Timers.cpp#L231-L237: check Runtime::GetRuntime(isolate) and the returned loop before calling SetTimerSource, and keep postToken consistent with that check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@NativeScript/runtime/ClassBuilder.mm` around lines 321 - 342, Add consistent
null guards for runtime and event-loop access: in
NativeScript/runtime/ClassBuilder.mm#L321-L342 and the swizzled release at lines
379-384, validate Runtime::GetRuntime(isolate) before RuntimeLoop() and
GetEventLoop() before PostInternalBare; in
NativeScript/runtime/Worker.mm#L314-L314, store GetEventLoop() locally, return
when null, then call PostInternal; in NativeScript/runtime/Timers.cpp#L231-L237,
validate the runtime and loop before SetTimerSource and keep postToken aligned
with that guarded path.
🧹 Nitpick comments (1)
NativeScript/runtime/ClassBuilder.mm (1)

379-401: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse gcUnprotect in the same-thread branch.

Lines 386-401 repeat the body of the gcUnprotect lambda defined at line 362. The retain path already calls its lambda directly in the same-thread branch. Call gcUnprotect() here so the two paths cannot diverge.

♻️ Proposed refactor
             } else {
-              auto innerCache = isolateWrapper.GetCache();
-              auto it = innerCache->Instances.find(self);
-              if (it != innerCache->Instances.end()) {
-                v8::Locker locker(isolate);
-                Isolate::Scope isolate_scope(isolate);
-                HandleScope handle_scope(isolate);
-                if (it->second != nullptr) {
-                  Local<Value> value = it->second->Get(isolate);
-                  BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
-                  if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) {
-                    ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
-                    objcWrapper->GcUnprotect();
-                  }
-                }
-              }
+              gcUnprotect();
             }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@NativeScript/runtime/ClassBuilder.mm` around lines 379 - 401, In the
same-thread branch of the runtime callback, replace the duplicated cache lookup,
V8 scope setup, and ObjC wrapper unprotect logic with a direct call to the
existing gcUnprotect lambda. Preserve the surrounding runtime-loop branching and
keep the existing bare PostInternalBare(gcUnprotect) path unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@NativeScript/inspector/JsV8InspectorClient.mm`:
- Around line 440-442: Update JsV8InspectorClient::runMessageLoopOnPause() to
exit its main pause-loop iteration before Runtime::~Runtime() removes the
isolate’s event-loop entry; coordinate shutdownRuntime() with that loop so no
iteration calls GetEventLoop() after runtime_ is destroyed, rather than relying
on a null guard.

In `@NativeScript/runtime/EventLoop.h`:
- Around line 94-97: Update the ordered-lane documentation at
NativeScript/runtime/EventLoop.h:94-97 to describe delivery through the single
due-ordered timer and its FIFO contract, not CFRunLoopPerformBlock; at
NativeScript/runtime/EventLoop.h:108-116 remove the performed-block case and
retain only the matured case. In NativeScript/runtime/Runtime.mm:589-590,
describe the ordered-lane timer behavior and state that FIFO is maintained
against JS timers through the shared due-ordered domain.

In `@NativeScript/runtime/Timers.cpp`:
- Around line 231-237: Guard the event loop before calling SetTimerSource in
Timers::Init, and add the same null handling to postToken where eventLoop_ is
dereferenced. Handle a null Runtime::GetRuntime(isolate) or event loop with the
existing predictable failure behavior, while preserving TimerState cleanup
semantics.
- Around line 111-115: Update the timer bookkeeping shared by postToken and
removeTask so cancellation tracks the effective ordered-token key actually
posted: use now when dueTime is overdue, otherwise dueTime, and pass that same
key to TryCancelOrderedToken. Preserve one-to-one token accounting for equal-due
macrotasks.

In `@NativeScript/runtime/WorkerWrapper.mm`:
- Around line 34-41: Update the PostInternal completion flow in WorkerWrapper to
ensure every successfully posted task signals or otherwise releases the waiting
worker when EventLoop::Shutdown() drops the entry; alternatively replace the
unbounded dispatch_semaphore_wait with an appropriate bounded wait and handle
timeout. Preserve normal fn execution and completion signaling for tasks that
run.

In `@TestRunner/app/tests/EventLoopTests.js`:
- Around line 378-390: Update the worker onmessage handler in the “survives
terminating a worker with queued loop work” test to use a one-shot guard,
ensuring the message-posting, terminate call, and delayed done callback execute
only on the first delivery; ignore subsequent messages.

---

Outside diff comments:
In `@NativeScript/runtime/ClassBuilder.mm`:
- Around line 321-342: Add consistent null guards for runtime and event-loop
access: in NativeScript/runtime/ClassBuilder.mm#L321-L342 and the swizzled
release at lines 379-384, validate Runtime::GetRuntime(isolate) before
RuntimeLoop() and GetEventLoop() before PostInternalBare; in
NativeScript/runtime/Worker.mm#L314-L314, store GetEventLoop() locally, return
when null, then call PostInternal; in NativeScript/runtime/Timers.cpp#L231-L237,
validate the runtime and loop before SetTimerSource and keep postToken aligned
with that guarded path.

---

Nitpick comments:
In `@NativeScript/runtime/ClassBuilder.mm`:
- Around line 379-401: In the same-thread branch of the runtime callback,
replace the duplicated cache lookup, V8 scope setup, and ObjC wrapper unprotect
logic with a direct call to the existing gcUnprotect lambda. Preserve the
surrounding runtime-loop branching and keep the existing bare
PostInternalBare(gcUnprotect) path unchanged.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 213b5345-cf4e-4f1a-baf4-dc5911f8c796

📥 Commits

Reviewing files that changed from the base of the PR and between 32ed9e5 and dc2081a.

📒 Files selected for processing (25)
  • NativeScript/inspector/JsV8InspectorClient.mm
  • NativeScript/inspector/WorkerInspectorClient.mm
  • NativeScript/runtime/Caches.h
  • NativeScript/runtime/ClassBuilder.mm
  • NativeScript/runtime/EventLoop.h
  • NativeScript/runtime/EventLoop.mm
  • NativeScript/runtime/ModuleInternal.mm
  • NativeScript/runtime/NativeScriptException.mm
  • NativeScript/runtime/NativeScriptPlatform.h
  • NativeScript/runtime/NativeScriptPlatform.mm
  • NativeScript/runtime/Runtime.h
  • NativeScript/runtime/Runtime.mm
  • NativeScript/runtime/SetTimeout.cpp
  • NativeScript/runtime/SetTimeout.h
  • NativeScript/runtime/Timers.cpp
  • NativeScript/runtime/Timers.hpp
  • NativeScript/runtime/Worker.mm
  • NativeScript/runtime/WorkerWrapper.mm
  • TestRunner/app/tests/EventLoopTests.js
  • TestRunner/app/tests/EventLoopWaitAsyncWorker.js
  • TestRunner/app/tests/esm/tla-foreground-task-import.mjs
  • TestRunner/app/tests/esm/tla-foreground-task.mjs
  • TestRunner/app/tests/eventLoopEchoWorker.js
  • TestRunner/app/tests/index.js
  • v8ios.xcodeproj/project.pbxproj
💤 Files with no reviewable changes (2)
  • NativeScript/runtime/SetTimeout.cpp
  • NativeScript/runtime/SetTimeout.h

Comment thread NativeScript/inspector/JsV8InspectorClient.mm Outdated
Comment thread NativeScript/runtime/EventLoop.h Outdated
Comment thread NativeScript/runtime/Timers.cpp Outdated
Comment thread NativeScript/runtime/Timers.cpp Outdated
Comment thread NativeScript/runtime/WorkerWrapper.mm
Comment thread TestRunner/app/tests/EventLoopTests.js
- Inspector pause loops resolve the loop via LookupEventLoop with a
  null guard: GetEventLoop creates on miss, so a stale pause iteration
  could mint a registry entry for an isolate whose runtime is gone.
- PostOrderedToken returns the key it actually recorded (an overdue due
  time is clamped to the loop's now); Timers stores it per cycle as
  postedTokenTime_ and cancels by it, so clearing an overdue timer
  recalls the token instead of leaving a tombstone plus a wakeup.
- The sync PostToRuntimeLoop wait is released even when Shutdown drops
  the entry unrun: a shared_ptr deleter signals the semaphore when the
  posted closure is destroyed, whichever way that happens.
- Timers::Init and postToken guard a null runtime/event loop, degrading
  to inert timers instead of crashing.
- Stale performed-block wording in the ordered-lane docs replaced with
  the timer-phase contract.
- One-shot guard in the terminate-with-queued-work spec: late echoes
  must not re-drive the handler and double-invoke done().
PostOrderedToken returns the recorded key as the producer's recall
handle; the bind-time flush re-clamped buffered keys against the bind
time, so a pre-bind poster whose due time had passed could never recall
its token. Insert buffered keys verbatim - a past key simply fires on
the next runloop pass. Also note the reused-isolate-address window
(tasks v8 posts during Isolate::New land in the stale stopped loop
until the refresh right after) and finish the stale performed-block
wording in Timers.
@NathanWalker
NathanWalker merged commit 4a90925 into main Aug 12, 2026
9 checks passed
@NathanWalker
NathanWalker deleted the feat/v8-platform-event-loop branch August 12, 2026 20:01
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.

2 participants