GeezerVison

coderabbitai Bot left a comment

coderabbitai Bot left a comment

coderabbitai Bot left a comment

V8 platform foreground tasks (async WASM compilation callbacks,
Atomics.waitAsync wakeups, GC finalization tasks) sat in the default
platform's internal queues, which nothing pumped outside the WASM-scoped
MessageLoopTimer (an ALooper fd fed by a detached 100ms-polling thread)
and the inspector pause loops. Atomics.waitAsync promises never resolved
at all.

Wrap the default platform in NativeScriptPlatform: worker-thread
scheduling, jobs, time and tracing still delegate to libplatform, but
GetForegroundTaskRunner serves a per-isolate ForegroundTaskRunner that
delivers tasks through a dedicated com.tns.EventLoopHandler bound to the
runtime thread's Looper - the same anonymous-token scheme Timers use, so
platform tasks are strictly FIFO-ordered with Handler.post runnables and
JS timers on the same looper:

- each posted task enqueues into a native queue (immediate deque plus a
  due-time-sorted delayed map) and posts one "task due" token; a token
  runs the earliest due task, then performs a microtask checkpoint,
  since a task may resolve promises without entering JS (e.g.
  Atomics.waitAsync), which kAuto's depth-0 drain never sees
- delayed tasks ride sendMessageAtTime at ceil(dueTime), so a token
  never arrives before its due time
- v8 requests the runner during Isolate::New, before the home thread is
  known, so the runner starts unbound and buffers; PrepareV8Runtime
  binds it to the thread's Looper and flushes one token per buffered
  task; posts are accepted from any thread
- inspector pause loops can't receive tokens (the Java looper isn't
  spinning), so they drain nestable tasks directly; non-nestable tasks
  keep their queued tokens until the pause unwinds, and leftover tokens
  no-op like cleared-timer tokens
- the runner shuts down in DestroyRuntime and is unregistered after
  isolate disposal, so workers can churn without leaking map entries

MessageLoopTimer, its polling thread and the WebAssembly method proxies
in message-loop-timer.js are removed: async WASM promises now resolve
promptly through the runner with no start/stop windows.

The runner is also the seam for future macrotask dispatch (e.g.
performance API observer callbacks). Microtask policy is deliberately
untouched.

Adds Atomics.waitAsync regression tests (notify, timeout, sync
mismatch, promise-chain ordering); the async cases hang without this
change.

…d lane)

Restructure the foreground task runner into a per-runtime EventLoop, the
Android analogue of the iOS runtime's ExecuteOnRunLoop seam, routing work
by ordering contract:

- ordered lane: work whose ordering is observable against app-level Java
  messages rides the Java MessageQueue via EventLoopHandler tokens,
  strictly FIFO with Handler.post and JS timers. First producer:
  __ns__queueMacrotask(cb), the seam future spec'd macrotasks
  (performance observers etc.) will use.
- internal lane: work in its own ordering domain - v8 platform foreground
  tasks, worker->parent messages, unhandled-rejection drains - rides an
  EFD_SEMAPHORE eventfd plus a timerfd for delayed tasks on the thread's
  ALooper. No JNI on the post path, so v8's non-JVM worker threads post
  without attaching to the JVM. One eventfd unit runs one entry per
  looper callback, keeping bursts fair with Java messages.

LooperTasks is consolidated into the internal lane (worker messaging and
exception-drain call sites ported 1:1, keeping the weak_ptr child
semantics and drop-after-shutdown behavior). Timers stays separate: it is
the ordered lane specialized with sub-millisecond ordering machinery.

Also addresses review findings: ordered-lane token posts and destructor
now synchronize on the loop mutex; the inspector pause drain is bounded
to the entries present at call time so a self-reposting task cannot
wedge the CDP read; ~Runtime guards the platform instance and isolate
against early construction failure; EventLoopHandler fails loudly when
constructed on a thread with no prepared Looper; the async waitAsync
test chain got its missing rejection handler.

Adds ordered-lane tests: async delivery, runs-after-microtasks, FIFO
interleaving with setTimeout(0), TypeError on non-function.

…ign review

Two defects found by deep review of the scheduler:

- internal-lane unit starvation: an eventfd unit written for an
  immediate entry could be consumed by a due-but-unsignaled delayed
  entry (whose own timerfd unit hadn't been issued yet); the timer fire
  then found nothing due and issued nothing, leaving the lane
  permanently off-by-one - the newest entry always waited for a future
  post. The unit-consuming path now skips unsignaled delayed entries;
  nested (unit-free) drains and the ordered lane are unaffected, since
  ordered entries carry their token from post time.
- stale loop registry across isolate-pointer reuse: the registry erase
  ran in ~Runtime, several JNI calls after Isolate::Dispose freed the
  address. A concurrently created worker isolate could reuse the
  pointer, inherit the dead runtime's stopped loop (silently dropping
  all its work), and then lose its own entry to the late destructor.
  The erase now happens immediately after Dispose and only while the
  entry still maps to the disposing runtime's loop; PrepareV8Runtime
  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
  also redirects runners v8 already holds.

Also from review: the inspector-pause drain no longer lets C++
exceptions unwind through v8 inspector frames, and fd callbacks ignore
spurious wakeups instead of consuming an entry.

Tests: worker reply racing an overdue Atomics.waitAsync timeout (unit
accounting), worker churn smoke, and __ns__queueMacrotask posted from a
background JS thread landing on the main thread (multithreaded JS).

…inThread through the internal lane

Timers merge (with tombstones):

- Timers no longer owns a Java Handler: each scheduled timer posts one
  anonymous token through the EventLoop's ordered lane, and the token
  drain runs the earliest due item across timers and ordered macrotasks
  - one due-ordered domain, still strictly FIFO with Handler.post on
  the same looper. Token 'when' computation is unchanged, so the
  quiescent setTimeout-vs-Handler.post contract is preserved exactly.
- clearTimeout/clearInterval tombstone the sorted entry instead of
  erasing it: the cleared timer's already-queued token consumes its own
  slot as a no-op, so no token gains surplus capacity to run a
  later-scheduled item (timer or macrotask) ahead of foreign Java
  messages queued between the two token positions. This also fixes the
  pre-existing congestion deviation where a leftover token could fire a
  later timer early.
- FireTimer's internals (sub-ms sorted list, chromium-style interval
  catch-up, nesting clamp, TryCatch discipline) are untouched; the
  check-and-run happens in one OrderedTaskSource::RunIfEarliest call
  under a single Locker acquisition, because background threads mutate
  the timer bookkeeping through setTimeout under multithreaded JS.
- TimerHandler.java is deleted.

__runOnMainThread promotion:

- The 2MB main-looper pipe and RunOnMainThreadFdCallback are replaced
  by bare internal-lane entries on the main runtime's EventLoop. Bare
  entries skip the loop's Locker/checkpoint: the closure locks the
  CALLER's isolate (a worker's, under multithreaded JS), and taking the
  main isolate's Locker first would nest Lockers across isolates and
  can deadlock against worker->main JNI entry paths. Delivery stays
  one-per-poll, matching the old fd callback.
- The callback cache is now mutex-guarded: it was written from
  arbitrary threads under different isolates' Lockers, which provide no
  mutual exclusion; RemoveIsolateEntries also no longer erases while
  range-iterating.
- Uncaught exceptions in the callbacks now surface as pending Java
  exceptions via the loop's guard instead of unwinding C++ through the
  ALooper callback frame.

Tests: tombstone ordering specs (cleared timer's token vs java posts,
for both a later timer and a queued macrotask), against the native
__ns__ timers - the test app's global setTimeout is an old
Handler-based polyfill with colliding ids, not the runtime timers.

…tive gate, identified long-timer removal)

Cancelled timers no longer leave stale wakeups. Two tiers by remaining
delay, both preserving exact clear semantics from any thread
(multithreaded JS can schedule and clear on non-looper threads):

- short timers (<32ms): the token carries a native claim cell - a slot
  in a fixed per-loop atomic table indexed by timer id, with the id
  embedded in the cell word so cancellation can never hit a recycled
  cell. clearTimeout is a single native CAS (zero JNI): winning proves
  the token dead everywhere, so the sorted entry is erased outright;
  losing means dispatch owns the token, so a tombstone is left for it.
  EventLoopHandler claims cells through a @CriticalNative CAS (the
  annotation is public API in current SDKs; where ART doesn't apply it
  the method degrades to a plain JNI call with identical semantics)
  before entering the runtime, so a cancelled token dies in Java in
  nanoseconds - without acquiring the isolate Locker, which previously
  let a stale token park the main thread behind a long background JS
  turn. Only the gate retires cells, and cell tokens are never
  removeMessages()ed, so each cell sees exactly one gate pass; a busy
  slot (interval re-arm racing its previous token, or id collision
  beyond 1024 in-flight) just downgrades the token to plain+tombstone.
- long timers (>=32ms, debounce territory): the token carries a Java
  AtomicBoolean peer, claimed in handleMessage. clearTimeout CASes the
  peer and on winning removeMessages()es the queued token: a cleared
  debounce timer produces no wakeup at all. The peer and its Message
  are GC-owned, which makes the removal-vs-in-flight-dequeue race
  harmless - a lost race costs at most one no-op wakeup, never an
  ordering violation. Below the cutoff a stale wakeup lands within two
  frames of the interaction that scheduled it (the app is provably
  awake), so the zero-allocation cell path applies instead.

Only the newest token of an interval is cancellable; older tokens
orphaned by a re-arm keep functioning anonymously through their own
carriers, so token/slot parity holds under the anonymous-dispatch
shuffle. SetTimer now converts a failed token post into a JS exception
instead of unwinding a NativeScriptException through the V8 callback
frame.

Verified on device: ordering probes 100% across all scenarios
(timer FIFO ties, clear-vs-Handler.post in both orders, orphan gap,
triple-clear, clearInterval-from-callback, starvation), and the full
suite (78 suites / 668 specs) green, including new specs for identified
clear, background-thread clear racing dispatch, and interval stop.

…ks from review

- @CriticalNative is ignored below API 26, where ART calls the method
  through the standard JNI ABI - binding the critical-convention
  function there would misread its arguments (minSdk is 21).
  Registration now binds a standard-ABI twin on api < 26, so the gate
  behaves identically on every supported API level. RegisterNatives
  failure no longer asserts: it clears the pending exception and gates
  PostTimerToken to plain tokens, so the unbound native can never be
  reached.
- a failed JNI token post no longer leaks state: PostTimerToken
  releases the claim cell (no dispatch gate will ever retire it), and
  addTask erases the just-inserted sorted slot and map entry before
  rethrowing - a tokenless slot would otherwise consume another
  token's dispatch (live) or starve the item behind it (tombstoned).
- RunOnMainThreadCallback resolves the main event loop before caching
  the callback, so a pre-init call can't pin the closure in the cache
  with no post to consume it.
- tests: done.fail does not exist in the pinned jasmine 2.0.1 (it
  would TypeError inside the rejection handler and time out silently) -
  replaced with record-then-done; the background-clear race spec now
  counts only iterations whose clear provably ran (AtomicBoolean
  signal, bounded attempts), so it can't pass without racing.

The RunMainThreadEntry isolate-liveness window flagged by review is
byte-for-byte the removed pipe implementation's behavior and needs
teardown-spanning liveness; deferred to the teardown-coordination work
queued with the kExplicit follow-up.