Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions test-app/app/src/main/assets/app/mainpage.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ shared.runWeakRefTests();
shared.runRuntimeTests();
shared.runWorkerTests();
require("./tests/testWebAssembly");
require("./tests/testEventLoop");
require("./tests/testMultithreadedJavascript");
require("./tests/testInterfaceDefaultMethods");
require("./tests/testInterfaceStaticMethods");
Expand Down
3 changes: 3 additions & 0 deletions test-app/app/src/main/assets/app/tests/eventLoopEchoWorker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
onmessage = function (msg) {
postMessage(msg.data);
};
257 changes: 257 additions & 0 deletions test-app/app/src/main/assets/app/tests/testEventLoop.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,257 @@
// V8 delivers these resolutions as platform foreground tasks, so they only
// settle if the runtime pumps its foreground task runner (EventLoopHandler).
describe("event loop foreground tasks", function () {
it("resolves Atomics.waitAsync when notified on the same thread", function (done) {
const sab = new SharedArrayBuffer(4);
const i32 = new Int32Array(sab);

const result = Atomics.waitAsync(i32, 0, 0);
expect(result.async).toBe(true);

result.value.then(value => {
expect(value).toBe("ok");
done();
}).catch(e => {
done.fail("Atomics.waitAsync promise rejected: " + e);
});

const woken = Atomics.notify(i32, 0);
expect(woken).toBe(1);
});

it("resolves Atomics.waitAsync with 'timed-out' after the timeout", function (done) {
const sab = new SharedArrayBuffer(4);
const i32 = new Int32Array(sab);

const result = Atomics.waitAsync(i32, 0, 0, 50);
expect(result.async).toBe(true);

result.value.then(value => {
expect(value).toBe("timed-out");
done();
}).catch(e => {
done.fail("Atomics.waitAsync promise rejected: " + e);
});
});

it("resolves Atomics.waitAsync synchronously on value mismatch", function () {
const sab = new SharedArrayBuffer(4);
const i32 = new Int32Array(sab);
i32[0] = 42;

const result = Atomics.waitAsync(i32, 0, 0);
expect(result.async).toBe(false);
expect(result.value).toBe("not-equal");
});

it("keeps ordinary promise chains working alongside foreground tasks", function (done) {
const sab = new SharedArrayBuffer(4);
const i32 = new Int32Array(sab);
const order = [];

Atomics.waitAsync(i32, 0, 0).value.then(() => {
order.push("waitAsync");
return Promise.resolve();
}).then(() => {
order.push("chained");
expect(order).toEqual(["waitAsync", "chained"]);
done();
}).catch(e => {
done.fail("promise chain failed: " + e);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +59 to +61

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

done.fail does not exist in the pinned Jasmine version.

This test app pins Jasmine 2.0.1. In that version done is a plain function with no .fail property. Line 60 therefore throws a TypeError inside the rejection handler. done is never called, the handler's own rejection goes unhandled, and the spec fails by timeout with no message. That is the same failure mode this handler was added to prevent.

Use the explicit two-handler form: assert on the error, then call done(). In Jasmine 2.0.1 addExpectationResult records the failure without throwing, so done() is always reached and the spec reports the real error.

💚 Proposed fix
-        }).catch(e => {
-            done.fail("promise chain failed: " + e);
-        });
+        }).catch(e => {
+            expect(e).toBeUndefined();
+            done();
+        });

Based on learnings: "done is a plain function with no .fail property, so done.fail is undefined", and "The correct async test pattern for this version is the explicit two-handler form rather than done.fail".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
}).catch(e => {
done.fail("promise chain failed: " + e);
});
}).catch(e => {
expect(e).toBeUndefined();
done();
});
🤖 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/app/src/main/assets/app/tests/testEventLoop.js` around lines 59 -
61, Replace the unsupported done.fail call in the promise rejection handler of
testEventLoop with Jasmine 2.0.1’s explicit failure assertion, then call done()
afterward so the handler always completes and reports the original error.

Source: Learnings


Atomics.notify(i32, 0);
});
});

// The ordered lane rides the Java MessageQueue, so these callbacks must be
// strict macrotasks: after the current turn's microtasks, FIFO with timers.
describe("event loop ordered macrotasks", function () {
it("__ns__queueMacrotask runs the callback asynchronously", function (done) {
let ran = false;
__ns__queueMacrotask(() => {
ran = true;
done();
});
expect(ran).toBe(false);
});

it("runs after the current turn's microtasks", function (done) {
const order = [];
__ns__queueMacrotask(() => {
order.push("macrotask");
expect(order).toEqual(["microtask", "macrotask"]);
done();
});
Promise.resolve().then(() => order.push("microtask"));
});

// native timers (__ns__*): the app-level `setTimeout` global in this test
// app is an old Handler-based polyfill, not the runtime timers
it("stays FIFO-ordered with native setTimeout(0)", function (done) {
const order = [];
__ns__queueMacrotask(() => order.push("macro1"));
__ns__setTimeout(() => order.push("timeout"), 0);
__ns__queueMacrotask(() => {
order.push("macro2");
expect(order).toEqual(["macro1", "timeout", "macro2"]);
done();
});
});

it("rejects non-function arguments", function () {
expect(() => __ns__queueMacrotask("nope")).toThrowError(TypeError);
expect(() => __ns__queueMacrotask()).toThrowError(TypeError);
});

it("runs on the main thread when posted from a background JS thread", function (done) {
const mainThreadId = java.lang.Thread.currentThread().getId();
new java.lang.Thread(new java.lang.Runnable({
run() {
expect(java.lang.Thread.currentThread().getId()).not.toEqual(mainThreadId);
__ns__queueMacrotask(() => {
expect(java.lang.Thread.currentThread().getId()).toEqual(mainThreadId);
done();
});
}
})).start();
});
});

// clearTimeout leaves a tombstone in the merged ordered domain, so the
// cleared timer's already-queued token consumes its own slot as a no-op
// instead of running a later-scheduled item ahead of Java messages queued
// between the two tokens' positions.
describe("event loop ordered tombstones", function () {
it("cleared timeout's token does not run a later timer ahead of java posts", function (done) {
const order = [];
const handler = new android.os.Handler(android.os.Looper.myLooper());
const t1 = __ns__setTimeout(() => order.push("cleared"), 0);
__ns__clearTimeout(t1);
handler.post(new java.lang.Runnable({
run: () => order.push("java")
}));
__ns__setTimeout(() => {
order.push("t2");
expect(order).toEqual(["java", "t2"]);
done();
}, 0);
});

it("cleared timeout's token does not run a queued macrotask ahead of java posts", function (done) {
const order = [];
const handler = new android.os.Handler(android.os.Looper.myLooper());
const t1 = __ns__setTimeout(() => order.push("cleared"), 0);
__ns__clearTimeout(t1);
handler.post(new java.lang.Runnable({
run: () => order.push("java")
}));
__ns__queueMacrotask(() => {
order.push("macro");
expect(order).toEqual(["java", "macro"]);
done();
});
});
});

// Long (>=32ms) timers carry an identified token whose clear removes the
// queued wakeup; short timers carry a native claim cell whose clear is a
// single CAS. Both must keep exact clear semantics under any thread.
describe("event loop token cancellation", function () {
it("cleared identified (long) timeout never fires and later timers are unaffected", function (done) {
let fired = false;
const t = __ns__setTimeout(() => { fired = true; }, 100);
__ns__clearTimeout(t);
__ns__setTimeout(() => {
expect(fired).toBe(false);
done();
}, 150);
});

it("background-thread clear racing dispatch neither jumps java posts nor ghost-fires", function (done) {
let remaining = 30;
(function iter() {
const order = [];
const handler = new android.os.Handler(android.os.Looper.myLooper());
const t1 = __ns__setTimeout(() => order.push("t1"), 0);
new java.lang.Thread(new java.lang.Runnable({
run() {
__ns__clearTimeout(t1);
}
})).start();
handler.post(new java.lang.Runnable({
run: () => order.push("java")
}));
__ns__setTimeout(() => {
order.push("t2");
const observed = order.join(">");
// t1 either fired before the clear landed (at its own legal
// slot, ahead of "java") or never; t2 must never jump "java"
expect(observed === "java>t2" || observed === "t1>java>t2").toBe(true);
if (--remaining === 0) {
done();
} else {
iter();
}
}, 5);
Comment on lines +177 to +196

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Confirm that the background-thread clear operation ran.

The test does not wait for __ns__clearTimeout(t1) to execute. The assertion can pass with "java>t2" while the background thread has not started. Add an AtomicBoolean or equivalent completion signal. Assert that the thread set it before calling done().

Proposed fix
             const order = [];
+            const cleared = new java.util.concurrent.atomic.AtomicBoolean(false);
             const handler = new android.os.Handler(android.os.Looper.myLooper());
             const t1 = __ns__setTimeout(() => order.push("t1"), 0);
             new java.lang.Thread(new java.lang.Runnable({
                 run() {
                     __ns__clearTimeout(t1);
+                    cleared.set(true);
                 }
             })).start();
@@
                 // slot, ahead of "java") or never; t2 must never jump "java"
                 expect(observed === "java>t2" || observed === "t1>java>t2").toBe(true);
+                expect(cleared.get()).toBe(true);
                 if (--remaining === 0) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
new java.lang.Thread(new java.lang.Runnable({
run() {
__ns__clearTimeout(t1);
}
})).start();
handler.post(new java.lang.Runnable({
run: () => order.push("java")
}));
__ns__setTimeout(() => {
order.push("t2");
const observed = order.join(">");
// t1 either fired before the clear landed (at its own legal
// slot, ahead of "java") or never; t2 must never jump "java"
expect(observed === "java>t2" || observed === "t1>java>t2").toBe(true);
if (--remaining === 0) {
done();
} else {
iter();
}
}, 5);
const order = [];
const cleared = new java.util.concurrent.atomic.AtomicBoolean(false);
const handler = new android.os.Handler(android.os.Looper.myLooper());
const t1 = __ns__setTimeout(() => order.push("t1"), 0);
new java.lang.Thread(new java.lang.Runnable({
run() {
__ns__clearTimeout(t1);
cleared.set(true);
}
})).start();
handler.post(new java.lang.Runnable({
run: () => order.push("java")
}));
__ns__setTimeout(() => {
order.push("t2");
const observed = order.join(">");
// t1 either fired before the clear landed (at its own legal
// slot, ahead of "java") or never; t2 must never jump "java"
expect(observed === "java>t2" || observed === "t1>java>t2").toBe(true);
expect(cleared.get()).toBe(true);
if (--remaining === 0) {
done();
} else {
iter();
}
}, 5);
🤖 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/app/src/main/assets/app/tests/testEventLoop.js` around lines 177 -
196, Update the background-thread clear flow in the test iteration to use an
AtomicBoolean or equivalent completion signal set after __ns__clearTimeout(t1)
runs. Before completing the iteration with done(), assert that the signal
confirms the clear operation executed, while preserving the existing order
assertions and retry behavior.

})();
});

it("clearing an identified interval stops it", function (done) {
let ticks = 0;
const iv = __ns__setInterval(() => {
ticks++;
if (ticks === 2) {
__ns__clearInterval(iv);
__ns__setTimeout(() => {
expect(ticks).toBe(2);
done();
}, 120);
}
}, 40);
});
});

describe("event loop internal lane", function () {
// Regression for the eventfd unit-accounting bug: a worker reply's wakeup
// arriving while an overdue waitAsync timeout is still unsignaled must not
// be spent on the timeout entry, or the reply starves.
it("delivers worker messages whose wakeup raced an overdue waitAsync timeout", function (done) {
const worker = new Worker("./eventLoopEchoWorker.js");
let warm = false;
worker.onmessage = function (msg) {
if (msg.data === "warmup") {
warm = true;
const i32 = new Int32Array(new SharedArrayBuffer(4));
Atomics.waitAsync(i32, 0, 0, 50);
worker.postMessage("ping");
// block the looper until both the timeout and the reply are
// pending, so their wakeups are serviced from the same poll
const start = Date.now();
while (Date.now() - start < 150) { }
} else {
expect(warm).toBe(true);
expect(msg.data).toBe("ping");
worker.terminate();
done();
}
};
worker.postMessage("warmup");
});

it("keeps event loops healthy across worker churn", function (done) {
let remaining = 8;
(function cycle() {
const worker = new Worker("./eventLoopEchoWorker.js");
worker.onmessage = function () {
worker.terminate();
if (--remaining === 0) {
__ns__queueMacrotask(done);
} else {
cycle();
}
};
worker.postMessage("alive");
})();
});
});
5 changes: 2 additions & 3 deletions test-app/runtime/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ set(RUNTIME_BUILTIN_JS
${RUNTIME_BUILTIN_JS_DIR}/events.js
${RUNTIME_BUILTIN_JS_DIR}/inspect.js
${RUNTIME_BUILTIN_JS_DIR}/json-helper.js
${RUNTIME_BUILTIN_JS_DIR}/message-loop-timer.js
${RUNTIME_BUILTIN_JS_DIR}/node-util.js
${RUNTIME_BUILTIN_JS_DIR}/ns-util.js
${RUNTIME_BUILTIN_JS_DIR}/primordials.js
Expand Down Expand Up @@ -150,6 +149,7 @@ add_library(
src/main/cpp/Constants.cpp
src/main/cpp/DirectBuffer.cpp
src/main/cpp/ErrorEvents.cpp
src/main/cpp/EventLoop.cpp
src/main/cpp/Events.cpp
src/main/cpp/FieldAccessor.cpp
src/main/cpp/File.cpp
Expand All @@ -163,9 +163,7 @@ add_library(
src/main/cpp/JsArgToArrayConverter.cpp
src/main/cpp/JSONObjectHelper.cpp
src/main/cpp/Logger.cpp
src/main/cpp/LooperTasks.cpp
src/main/cpp/ManualInstrumentation.cpp
src/main/cpp/MessageLoopTimer.cpp
src/main/cpp/MetadataMethodInfo.cpp
src/main/cpp/MetadataNode.cpp
src/main/cpp/MetadataReader.cpp
Expand All @@ -176,6 +174,7 @@ add_library(
src/main/cpp/ModuleInternal.cpp
src/main/cpp/ModuleInternalCallbacks.cpp
src/main/cpp/NativeScriptException.cpp
src/main/cpp/NativeScriptPlatform.cpp
src/main/cpp/NsBuiltinModules.cpp
src/main/cpp/NumericCasts.cpp
src/main/cpp/ObjectManager.cpp
Expand Down
Loading