GeezerVison is showing github.com — fix(timers): order timers with the Java MessageQueue instead of ALooper fds by edusperoni · Pull Request #1967 · NativeScript/android
Conversation
edusperoni
commented
Jun 11, 2026
•
edited by coderabbitai
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Timers were delivered through a pipe fd registered with ALooper_addFd.
Android services fd callbacks inside MessageQueue.nativePollOnce at every
message boundary, so timer callbacks and Handler messages lived in two
queues with no mutual ordering: a setTimeout(0) could fire before or
interleave around an already-queued Handler.postDelayed(0) runnable.
Timers now ride the Java MessageQueue itself via a dedicated per-runtime
TimerHandler bound to the isolate's Looper:
- Each scheduled timer enqueues one message with sendMessageAtTime, so
timers share a single queue with Handler.post/postDelayed and fire in
exact MessageQueue order.
- Messages are anonymous "due tokens": a native list sorted by exact
(sub-millisecond) due time picks the earliest due timer per token,
preserving the previous relative ordering of JS timers despite the
millisecond-quantized Java queue.
- Due-now timers post at (long)now so they tie (FIFO) with a
postDelayed(0) made in the same millisecond; future timers post at
ceil(dueTime) so they never fire early.
- The background watcher thread, pipe, mutex and condition variables are
removed; the MessageQueue does all delayed scheduling and everything
runs on the isolate's thread with no locking. Worker isolates get the
same behavior on their own loopers.
- The HTML-spec nesting clamp (>=5 levels -> 4ms minimum) is now opt-in
via the NS_TIMERS_NESTING_CLAMP define (off by default): it exists to
keep browser pages from spinning the event loop, which is rarely the
desired behavior in a native app.
Also adds ordering regression tests (timers vs Handler posts), scheduled
from a java-posted runnable to avoid the >=5-level nesting clamp that
jasmine's spec chaining otherwise triggers.
Fixes timer/Handler ordering so setTimeout(0) reliably yields behind
already-queued main-thread work.
Summary by CodeRabbit
-
Tests
- Added timer execution order validation tests.
-
New Features
- Improved Chrome DevTools debugging with enhanced source-map support.
- Added DevTools protocol support for network resource loading and streaming.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…kResource
Chrome DevTools no longer fetches external source maps itself when
debugging remote targets: it issues Network.loadNetworkResource to the
target and reads the result back through IO.read/IO.close. None of these
embedder-side CDP domains are implemented by V8's inspector, so external
source maps failed and apps had to fall back to bloated
inline-source-map builds.
- Handle Network.loadNetworkResource natively: resolve the URL back to a
file on disk and reply with a stream handle (success:false +
net::ERR_FILE_NOT_FOUND when missing).
- Implement IO.read (1MB base64 chunks; eof only on a final empty read,
since the frontend discards data accompanying eof) and IO.close.
- Reply with a JSON-RPC error for unsupported schemes (e.g. https) so
DevTools keeps its existing fallback of fetching from the host.
- Rewrite sourceMapURL in outgoing Debugger.scriptParsed /
Debugger.scriptFailedToParse events from relative/file:// URLs to a
custom nsruntime:// scheme. DevTools hard-excludes file:, data: and
devtools: URLs from loading through the target, so without the rewrite
it would never send Network.loadNetworkResource and instead try (and
fail) to read device files from the host machine. data: and http(s)
URLs are left untouched, keeping inline source maps working.
- Allow opting out via nativescript.config.ts:
android.disableSourceMapURLRewrite (or the same key at the top level).
- Serve these messages on the websocket read thread (new native
handleMessageOnSocketThread), since the main-thread queue is
unavailable exactly when DevTools needs source maps: the pause loop
bypasses dispatchMessage and a busy isolate never drains the queue.
The handler is V8-free and returns the response for Java to send on
the receiving socket.
- Make Debugger.pause interrupt busy JS via Isolate::RequestInterrupt,
skipped while already paused in the nested loop to avoid a spurious
re-pause after resume.
- Vendor nlohmann/json v3.12.0 (third_party/json.hpp, header-only) for
CDP message handling outside V8.
Ports NativeScript/ios#385 and NativeScript/ios#378 to Android.
Refs: nodejs/node#58077
…er fds
Timers were delivered through a pipe fd registered with ALooper_addFd.
Android services fd callbacks inside MessageQueue.nativePollOnce at every
message boundary, so timer callbacks and Handler messages lived in two
queues with no mutual ordering: a setTimeout(0) could fire before or
interleave around an already-queued Handler.postDelayed(0) runnable.
Timers now ride the Java MessageQueue itself via a dedicated per-runtime
TimerHandler bound to the isolate's Looper:
- Each scheduled timer enqueues one message with sendMessageAtTime, so
timers share a single queue with Handler.post/postDelayed and fire in
exact MessageQueue order.
- Messages are anonymous "due tokens": a native list sorted by exact
(sub-millisecond) due time picks the earliest due timer per token,
preserving the previous relative ordering of JS timers despite the
millisecond-quantized Java queue.
- Due-now timers post at (long)now so they tie (FIFO) with a
postDelayed(0) made in the same millisecond; future timers post at
ceil(dueTime) so they never fire early.
- The background watcher thread, pipe, mutex and condition variables are
removed; the MessageQueue does all delayed scheduling and everything
runs on the isolate's thread with no locking. Worker isolates get the
same behavior on their own loopers.
Also adds ordering regression tests (timers vs Handler posts), scheduled
from a java-posted runnable to avoid the >=5-level nesting clamp that
jasmine's spec chaining otherwise triggers.
Fixes timer/Handler ordering so setTimeout(0) reliably yields behind
already-queued main-thread work.
coderabbitai
Bot
commented
Jun 11, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|

📝 Walkthrough
Walkthrough
This PR refactors the NativeScript Android runtime's timer dispatch mechanism from a background looper thread model to a Java Handler message queue, and adds DevTools socket-thread protocol handling with source-map URL rewriting and resource streaming support. The changes introduce a new TimerHandler class, update timer scheduling and firing logic with JNI integration, add new inspector socket-thread message handlers for DevTools resource loading, and validate timer ordering with new test cases.
Changes
Timer Infrastructure Refactoring
| Layer / File(s) |
Summary |
Timer data model and header updates test-app/runtime/src/main/cpp/Timers.h |
Timers.h adds JNI includes, conditionalizes nestingLevel_ under NS_TIMERS_NESTING_CLAMP, changes sortedTimers_ to store TimerReference directly instead of shared pointers, guards nesting state under compile flag, and adds JNI handler member fields with documentation reflecting Java message-queue semantics instead of background threads. |
TimerHandler Java implementation test-app/runtime/src/main/java/com/tns/TimerHandler.java |
New Android Handler subclass bound to the runtime thread's Looper with a dedicated MSG_FIRE_TIMER message type; post(uptimeMillis) enqueues timer-fire messages at absolute uptime via sendMessageAtTime, release() clears pending messages, handleMessage() invokes the native nativeFireTimer(nativeTimersPtr) JNI entry point. |
Timer JNI integration and scheduling test-app/runtime/src/main/cpp/Timers.cpp |
Timers::Init caches TimerHandler JNI class/method IDs and creates a global handler instance; addTask and postTimer update scheduling to enqueue timers into sortedTimers_ and call handler.post() with computed due times. |
Timer firing and cleanup test-app/runtime/src/main/cpp/Timers.cpp |
Timers::FireTimer() executes on isolate thread, locks V8, drops leftover tokens for not-yet-due timers, fires due callbacks, re-arms repeating timers, and removes cleared tasks; removeTask matches timers by due-time/id before erasing; Destroy uses stopped_ flag to release handler and clear containers; JNI_nativeFireTimer bridges Java to FireTimer() and translates C++ exceptions to NativeScriptException. |
DevTools Inspector Socket-Thread Message Handling
| Layer / File(s) |
Summary |
Inspector header and resource streaming state test-app/runtime/src/main/cpp/JsV8InspectorClient.h |
JsV8InspectorClient.h adds <atomic>, <map>, <mutex> headers; declares handleMessageOnSocketThread() method; introduces ResourceStream struct and supporting state: resourceStreams_ map guarded by resourceStreamsMutex_, lastStreamId_ counter, and isPausedNestedLoop_ atomic flag. |
Source-map URL rewriting and helpers test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp |
Anonymous-namespace helpers add safe JSON serialization, base64 encoding, package.json config parsing to detect source-map rewriting, and MaybeRewriteSourceMapURL that rewrites outgoing Debugger.scriptParsed/scriptFailedToParse notifications to use custom nsruntime:// URLs; pause flag management sets/clears isPausedNestedLoop_ on nested loop entry/exit. |
Socket-thread protocol message handler test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp |
handleMessageOnSocketThread() parses DevTools protocol messages and routes to handlers: Network.loadNetworkResource serves nsruntime:// and file:// resources from disk with stream IDs; IO.read returns base64-chunked payloads with EOF semantics; IO.close deletes streams; Debugger.pause requests V8 interrupt that schedules schedulePauseOnNextStatement (skipped if already in nested pause loop). |
Java-side inspector fast-path integration test-app/app/src/main/java/com/tns/AndroidJsV8Inspector.java, test-app/runtime/src/main/cpp/com_tns_AndroidJsV8Inspector.cpp |
AndroidJsV8Inspector declares native handleMessageOnSocketThread(String) and calls it early in WebSocket onMessage() to send immediate responses and skip normal queueing; JNI bridge converts jstring messages, delegates to C++, returns jstring response when non-empty, or nullptr with exception suppression. |
Timer Ordering Test Validation
| Layer / File(s) |
Summary |
Native timer ordering tests test-app/app/src/main/assets/app/tests/testNativeTimers.js |
Two new Jasmine specs validate timer callback ordering when Java Handler posts and setTimeout calls interleave: first spec checks 1,2,3 sequence with postDelayed(..., 0); second schedules 100 setTimeout callbacks in two batches around a single handler post, asserts handler lands at index 50, and confirms total count of 101. |
🎯 4 (Complex) | ⏱️ ~45 minutes
A rabbit hops through queues both Java and V8,
Timers fire in order, source maps rewrite straight,
DevTools now fetch resources through the native gate,
Where Handler and isolate together cooperate. 🐰✨
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name |
Status |
Explanation |
Resolution |
| Docstring Coverage |
⚠️ Warning |
Docstring coverage is 20.83% which is insufficient. The required threshold is 80.00%. |
Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name |
Status |
Explanation |
| Title check |
✅ Passed |
The title directly and accurately describes the main change: timers are refactored to use the Java MessageQueue instead of ALooper file descriptors for ordering. |
| 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. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ 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 and usage tips.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick comments (2)
test-app/runtime/src/main/cpp/Timers.cpp (1)
282-290: 💤 Low value
Consider replacing VLA with std::vector for C++ compliance.
Local<Value> argv[argc] is a variable-length array (VLA), a C99 feature that is not part of standard C++. While GCC/Clang support it as an extension, it's technically non-portable. Consider using std::vector<Local<Value>> if strict C++ compliance is desired.
♻️ Suggested fix
auto argc = task->args_.get() == nullptr ? 0 : task->args_->size();
if (argc > 0) {
- Local<Value> argv[argc];
+ std::vector<Local<Value>> argv(argc);
for (int i = 0; i < argc; i++) {
argv[i] = task->args_->at(i)->Get(isolate);
}
- cb->Call(context, context->Global(), argc, argv);
+ cb->Call(context, context->Global(), argc, argv.data());
} else {
🤖 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 `@test-app/runtime/src/main/cpp/Timers.cpp` around lines 282 - 290, The code
uses a C99 VLA Local<Value> argv[argc] (in the block that builds arguments for
cb->Call) which is non-standard in C++; replace it with a
std::vector<Local<Value>> argv; resize it to argc (or use reserve+push_back) and
fill entries from task->args_->at(i)->Get(isolate), then pass argv.data() (or
nullptr when argc==0) into cb->Call(context, context->Global(), argc,
argv.data()); update the branch that currently calls cb->Call(..., 0, nullptr)
to use the same vector logic to avoid VLAs and keep behavior identical.
test-app/runtime/src/main/cpp/com_tns_AndroidJsV8Inspector.cpp (1)
32-44: 💤 Low value
Consider logging suppressed exceptions for debuggability.
The catch-all block silently swallows exceptions, which is correct to prevent crashing the websocket thread. However, this makes debugging harder when something goes wrong. Consider logging to logcat when Runtime.isDebuggable() is true.
🔧 Suggested enhancement
} catch (...) {
// must never propagate a native exception into the websocket thread
+ // Log for debugging when enabled
+ __android_log_print(ANDROID_LOG_WARN, "JsV8Inspector",
+ "Exception suppressed in handleMessageOnSocketThread");
}
This requires #include <android/log.h> and checking debug mode appropriately.
🤖 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 `@test-app/runtime/src/main/cpp/com_tns_AndroidJsV8Inspector.cpp` around lines
32 - 44, The catch-all in
Java_com_tns_AndroidJsV8Inspector_handleMessageOnSocketThread silently swallows
errors; modify it to log suppressed exceptions to logcat when the app is
debuggable: add `#include` <android/log.h>, change the handler to catch const
std::exception& e (and a fallback catch(...)) and use __android_log_print to
emit e.what() (or a generic message) and include context (e.g., "JsV8Inspector
handleMessageOnSocketThread error"), and only perform logging when
Runtime.isDebuggable() is true (check debuggable via JNI from this function
before logging).
🤖 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.
Nitpick comments:
In `@test-app/runtime/src/main/cpp/com_tns_AndroidJsV8Inspector.cpp`:
- Around line 32-44: The catch-all in
Java_com_tns_AndroidJsV8Inspector_handleMessageOnSocketThread silently swallows
errors; modify it to log suppressed exceptions to logcat when the app is
debuggable: add `#include` <android/log.h>, change the handler to catch const
std::exception& e (and a fallback catch(...)) and use __android_log_print to
emit e.what() (or a generic message) and include context (e.g., "JsV8Inspector
handleMessageOnSocketThread error"), and only perform logging when
Runtime.isDebuggable() is true (check debuggable via JNI from this function
before logging).
In `@test-app/runtime/src/main/cpp/Timers.cpp`:
- Around line 282-290: The code uses a C99 VLA Local<Value> argv[argc] (in the
block that builds arguments for cb->Call) which is non-standard in C++; replace
it with a std::vector<Local<Value>> argv; resize it to argc (or use
reserve+push_back) and fill entries from task->args_->at(i)->Get(isolate), then
pass argv.data() (or nullptr when argc==0) into cb->Call(context,
context->Global(), argc, argv.data()); update the branch that currently calls
cb->Call(..., 0, nullptr) to use the same vector logic to avoid VLAs and keep
behavior identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8731d186-0b37-46e9-be6e-7577bcf91bb6
📥 Commits
Reviewing files that changed from the base of the PR and between d0afd1e and a730305.
📒 Files selected for processing (9)
test-app/app/src/main/assets/app/tests/testNativeTimers.js
test-app/app/src/main/java/com/tns/AndroidJsV8Inspector.java
test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp
test-app/runtime/src/main/cpp/JsV8InspectorClient.h
test-app/runtime/src/main/cpp/Timers.cpp
test-app/runtime/src/main/cpp/Timers.h
test-app/runtime/src/main/cpp/com_tns_AndroidJsV8Inspector.cpp
test-app/runtime/src/main/cpp/third_party/json.hpp
test-app/runtime/src/main/java/com/tns/TimerHandler.java
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jun 11, 2026
This was referenced Jul 23, 2026
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|