Skip to content

Commit 351bef6

Browse files
committed
fix(runtime): keep error reporting alive on a terminating isolate
Building an error report runs JS (ToDetailString), which materializes an armed termination interrupt; the next V8 call then answers Nothing, and two ToChecked() sites in the formatter turned that into an unconditional CHECK abort - reachable whenever worker.terminate() lands while a genuine error is being reported. Both read through FromMaybe now, like the rest of the file. A spec pins the whole terminate-during-entry window: a worker whose .mjs entry parks forever in top-level await is terminated mid-pump, three times over, asserting no onerror fires on the dying worker and that a fresh worker still round-trips - termination ends the pump promptly and never masquerades as a timeout or an entry rejection.
1 parent e11c9c3 commit 351bef6

3 files changed

Lines changed: 67 additions & 2 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// An ES module worker entry that never finishes evaluating. The parent is told
2+
// the entry body started before the park, so its terminate() lands inside the
3+
// entry's bounded evaluation pump rather than after it.
4+
postMessage("never-settles:started");
5+
6+
await new Promise(function () {});
7+
8+
globalThis.onmessage = function () {
9+
postMessage("never-settles:unreachable");
10+
};

test-app/app/src/main/assets/app/tests/testWorkerEsmEntry.js

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,58 @@ describe("worker ES module entries", function () {
9090
worker.postMessage("ping");
9191
});
9292

93+
// The worker isolate is published before its entry runs, so terminate()
94+
// can interrupt an entry that is still evaluating - here one parked on a
95+
// promise that never settles, inside the pump that waits for it.
96+
it("survives terminate() while an entry is parked in top-level await", function (done) {
97+
var ITERATIONS = 3;
98+
var FALLBACK_TERMINATE = 700;
99+
var SETTLE_AFTER = 700;
100+
var errors = [];
101+
102+
function iteration(remaining) {
103+
if (remaining === 0) {
104+
expect(errors).toEqual([]);
105+
// A worker spawned after the terminated ones still works.
106+
var next = new Worker("~/tests/esmEntrySyncWorker.mjs");
107+
next.onmessage = function (msg) {
108+
expect(msg.data).toBe("esm-entry:ping");
109+
next.terminate();
110+
done();
111+
};
112+
next.postMessage("ping");
113+
return;
114+
}
115+
116+
var worker = new Worker("./esmEntryNeverSettlesWorker.mjs");
117+
var terminated = false;
118+
119+
function terminateOnce() {
120+
if (terminated) {
121+
return;
122+
}
123+
terminated = true;
124+
worker.terminate();
125+
setTimeout(function () {
126+
iteration(remaining - 1);
127+
}, SETTLE_AFTER);
128+
}
129+
130+
worker.onerror = function (e) {
131+
errors.push(String((e && e.message) || e));
132+
};
133+
worker.onmessage = function (msg) {
134+
expect(msg.data).toBe("never-settles:started");
135+
terminateOnce();
136+
};
137+
// The evaluation pump is bounded, so a start message that never
138+
// arrives must not push the terminate past the window it targets.
139+
setTimeout(terminateOnce, FALLBACK_TERMINATE);
140+
}
141+
142+
iteration(ITERATIONS);
143+
});
144+
93145
// WHATWG parity: the worker's message queue is enabled when its entry
94146
// script finishes evaluating, and from then on messages dispatch whether
95147
// or not a handler exists. A handler registered later (from a timer)

test-app/runtime/src/main/cpp/NativeScriptException.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,7 +1049,10 @@ string NativeScriptException::GetFullMessage(const TryCatch& tc,
10491049
} else {
10501050
ss << endl << "File: (<unknown>";
10511051
}
1052-
ss << ":" << message->GetLineNumber(context).ToChecked() << ":"
1052+
// An isolate that started terminating while this message was being
1053+
// formatted answers Nothing to every query below - a missing line number
1054+
// must degrade the report, not abort the process.
1055+
ss << ":" << message->GetLineNumber(context).FromMaybe(0) << ":"
10531056
<< message->GetStartColumn() << ")" << endl
10541057
<< endl;
10551058
ss << "StackTrace: " << endl << stackTraceMessage << endl;
@@ -1135,7 +1138,7 @@ string NativeScriptException::GetErrorMessage(const Local<Message>& message,
11351138
bool hasFullErrorMessage = false;
11361139
auto v8FullMessage = ArgConverter::ConvertToV8String(isolate, "fullMessage");
11371140
if (error->IsObject() &&
1138-
error.As<Object>()->Has(context, v8FullMessage).ToChecked()) {
1141+
error.As<Object>()->Has(context, v8FullMessage).FromMaybe(false)) {
11391142
hasFullErrorMessage = true;
11401143
Local<Value> errMsgVal;
11411144
error.As<Object>()->Get(context, v8FullMessage).ToLocal(&errMsgVal);

0 commit comments

Comments
 (0)