From 1f141da24f310d56335eeeb5532cd0b719a3c981 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 7 Oct 2026 16:35:04 -0300 Subject: [PATCH 1/3] fix(worker): terminate() reaches a worker still inside its entry script The worker's isolate was published to its wrapper only after the startup function returned, i.e. after the entry script had finished evaluating, so Terminate() found nothing to interrupt for a worker still in its entry: a synchronous loop there ran forever, and a parked top-level await ended only when the loader's settle deadline expired. Node and the web both interrupt the running entry. The isolate is now published right after the worker runtime is initialized, before the entry runs, and the startup function honors a terminate() that landed earlier by checking the flag before any app code. A termination that lands inside the entry unwinds RunModule, which no longer rethrows or reports on a terminating isolate, and the thread goes straight to its existing teardown path. The module runner and the exception constructor name a caught termination for what it is instead of formatting the TryCatch, and the settle pump checks for one after its loop too, so a termination is never reported as a timeout or as an entry rejection. Also documents the real resourceLimits and ios.priority Worker options in docs/worker-threads.md, whose node:worker_threads table claimed the runtime imposed no limits. Suite 1735/0, also under AddressSanitizer. Fixes #445 --- NativeScript/runtime/DataWrapper.h | 43 ++++--- NativeScript/runtime/ModuleInternal.mm | 71 +++++++---- NativeScript/runtime/NativeScriptException.mm | 13 +- NativeScript/runtime/Worker.mm | 36 ++++-- NativeScript/runtime/WorkerWrapper.mm | 48 ++++---- TestRunner/app/tests/WorkerTerminateTests.js | 112 ++++++++++++++++++ TestRunner/app/tests/index.js | 3 + .../tests/workerTerminate/busyEntryWorker.js | 4 + .../workerTerminate/parkedEntryWorker.mjs | 5 + docs/worker-threads.md | 67 ++++++++++- 10 files changed, 328 insertions(+), 74 deletions(-) create mode 100644 TestRunner/app/tests/WorkerTerminateTests.js create mode 100644 TestRunner/app/tests/workerTerminate/busyEntryWorker.js create mode 100644 TestRunner/app/tests/workerTerminate/parkedEntryWorker.mjs diff --git a/NativeScript/runtime/DataWrapper.h b/NativeScript/runtime/DataWrapper.h index c094b27e..23f84d72 100644 --- a/NativeScript/runtime/DataWrapper.h +++ b/NativeScript/runtime/DataWrapper.h @@ -550,12 +550,20 @@ class WorkerWrapper : public BaseDataWrapper { void CreateInspector(v8::Isolate* isolate, const std::string& scriptPath); void DestroyInspector(); + // `func` runs on the worker thread: it creates the worker's runtime, hands + // the isolate to PublishIsolate once the runtime is initialized, and runs + // the entry script. void Start(std::shared_ptr> poWorker, - std::function func, + std::function func, std::optional qualityOfService = std::nullopt); + // Worker thread, once the runtime is initialized and before the entry script + // runs. From here Terminate() reaches V8: it interrupts whatever JS the + // worker runs, the entry script included, and flags the module pumps so a + // parked graph stops waiting. A terminate() that landed earlier is honored by + // the caller checking IsTerminating() right after this, before any app code. + void PublishIsolate(v8::Isolate* isolate); // Both reporters take the isolate from their caller, which is running on - // it: they are reachable while the entry script is still evaluating, before - // workerIsolate_ is published. + // it: they are reachable while the entry script is still evaluating. void CallOnErrorHandlers(v8::Isolate* isolate, v8::TryCatch& tc); // Reports a rejected entry-evaluation promise. A rejection carries a reason // rather than a TryCatch, so it cannot go through CallOnErrorHandlers, but it @@ -599,11 +607,6 @@ class WorkerWrapper : public BaseDataWrapper { // used to name the cap in the message forwarded to the parent. void WatchHeapLimit(v8::Isolate* isolate, const std::string& scriptPath, std::optional maxOldGenerationSizeBytes); - // Whether the heap cap was hit. The worker startup path checks this to stop - // before running anything else in an isolate V8 is terminating. - inline bool HeapLimitExceeded() const { - return heapLimitExceeded_.load(std::memory_order_acquire); - } // The JS Worker object is a GC root from a successful start until the worker // ends, so a running worker is reachable the way a browser's is rather than // depending on its finalizer to keep it. Both of these run on the main @@ -622,6 +625,9 @@ class WorkerWrapper : public BaseDataWrapper { const int Id(); const bool IsRunning(); const bool IsClosing(); + // Set by Terminate() from any thread, by the near-heap-limit callback, and by + // the worker thread itself once a close() takes effect. One-way. + const bool IsTerminating(); const int WorkerId(); const inline v8::Isolate* GetMainIsolate() { return mainIsolate_; } // The only route from the worker thread to the parent: see mainLoop_. @@ -650,9 +656,13 @@ class WorkerWrapper : public BaseDataWrapper { enum class Holders : uint8_t { Parent, Both, WorkerThread }; v8::Isolate* mainIsolate_; - // Written by the worker thread only: published once the worker's startup - // function returns, withdrawn before the worker's runtime is deleted. Any - // other thread reads and uses it under workerIsolateMutex_. + // Written by the worker thread only: published by PublishIsolate once the + // worker's runtime is initialized and before its entry script runs, + // withdrawn before the runtime is deleted. Any other thread reads and uses + // it under workerIsolateMutex_. Null while the runtime is being set up, so a + // Terminate() in that window cannot interrupt the builtins Runtime::Init + // evaluates; the startup function checks the terminating flag right after + // publishing instead. v8::Isolate* workerIsolate_; std::mutex workerIsolateMutex_; std::atomic isRunning_; @@ -682,10 +692,11 @@ class WorkerWrapper : public BaseDataWrapper { // thread) and DestroyInspector() (worker thread) agree on liveness. v8_inspector::WorkerInspectorClient* inspector_ = nullptr; std::mutex inspectorMutex_; - // The worker isolate as seen from the heap-limit callback. Separate from - // workerIsolate_, which BackgroundLooper only publishes once the entry script - // has finished evaluating — the point at which a heap cap is most likely to - // be hit is inside that entry. + // The isolate the near-heap-limit callback is armed on. Worker thread only. + // Kept apart from workerIsolate_ because the callback is armed before the + // isolate is published and can fire during Runtime::Init, and because + // removing a callback V8 never had registered is fatal: non-null here means + // exactly "armed, remove at teardown". v8::Isolate* heapLimitIsolate_ = nullptr; std::string heapLimitMessage_; std::string heapLimitSource_; @@ -707,7 +718,7 @@ class WorkerWrapper : public BaseDataWrapper { // task would run on. std::shared_ptr> selfRef_; - void BackgroundLooper(std::function func); + void BackgroundLooper(std::function func); void DrainPendingTasks(); void ForwardErrorPayloadToMain(const std::string& message, const std::string& source, diff --git a/NativeScript/runtime/ModuleInternal.mm b/NativeScript/runtime/ModuleInternal.mm index 29a94f3d..4dbad283 100644 --- a/NativeScript/runtime/ModuleInternal.mm +++ b/NativeScript/runtime/ModuleInternal.mm @@ -293,6 +293,11 @@ throw NativeScriptException( success = requireFunc->Call(context, globalObject, 1, args).ToLocal(&result); if (!success || tc.HasCaught()) { + // A termination is caught like an exception but carries no error value; + // naming it as the failure keeps it from being reported as one. + if (tc.HasTerminated()) { + throw NativeScriptException("Module evaluation interrupted by isolate termination: " + path); + } if (RuntimeConfig.IsDebug) { Log(@"***** JavaScript exception occurred *****"); Log(@"Error in require() call:"); @@ -882,6 +887,10 @@ throw NativeScriptException(isolate, moduleFunc->Call(context, thiz, sizeof(requireArgs) / sizeof(Local), requireArgs) .ToLocal(&result); if (!success || tc.HasCaught()) { + if (tc.HasTerminated()) { + throw NativeScriptException("Module evaluation interrupted by isolate termination: " + + modulePath); + } throw NativeScriptException(isolate, tc, "Error calling module function"); } } @@ -1277,6 +1286,13 @@ throw NativeScriptException( Local result; if (!module->Evaluate(context).ToLocal(&result)) { RemoveModuleFromRegistry(isolate, canonicalPath); + // Same rule as the pump below: a termination outranks any failure detail, + // and reading the TryCatch as an error would run JS on the dying isolate. + if (tcEval.HasTerminated() || isolate->IsExecutionTerminating()) { + LogEsmPhase(canonicalPath, "evaluate", "terminated"); + throw NativeScriptException("Module evaluation interrupted by isolate termination: " + + canonicalPath); + } const char* classification = "unknown"; if (tcEval.HasCaught()) { Local msg = tcEval.Message(); @@ -1364,32 +1380,38 @@ throw NativeScriptException("ES module " + canonicalPath + NSDate* deadline = [NSDate dateWithTimeIntervalSinceNow:options.deadlineSeconds]; bool settled = false; + // Termination outranks a settled result and a timeout alike. Handing back a + // namespace would send the caller on to run more JS — enabling a queue, + // draining messages — on an isolate V8 has already been told to stop, and + // reporting a timeout would name a reason that is not the real one. + // + // Three signals are consulted: V8 only reports a termination it has already + // materialized, which needs JS to run, and a graph parked on a promise + // nothing settles never gives it any; one that materialized inside a + // checkpoint lands in promiseTc. Message-only exception: building a V8 + // error on a terminating isolate is not allowed. + auto throwIfTerminating = [&]() { + if (!promiseTc.HasTerminated() && !isolate->IsExecutionTerminating() && + (runtime == nullptr || !runtime->IsTerminationRequested())) { + return; + } + LogEsmPhase(canonicalPath, "evaluate", "terminated"); + // Probed, not consumed: only a still-pending promise leaves a + // half-evaluated module in the registry. One that already settled is + // complete, and evicting it would throw away a good entry for no reason + // — the result simply goes unused. + if (promise->State() == Promise::kPending) { + RemoveModuleFromRegistry(isolate, canonicalPath); + } + throw NativeScriptException("Module evaluation interrupted by isolate termination: " + + canonicalPath); + }; + // State is checked before the first pump: a synchronous graph's // evaluation promise is already settled when Evaluate() returns, so it // exits here without paying for a runloop slice. while (!promiseTc.HasCaught()) { - // Termination outranks a settled result. Handing back a namespace here - // would send the caller on to run more JS — enabling a queue, draining - // messages — on an isolate V8 has already been told to stop, so a - // termination seen at the loop head always throws, settled or not. - // - // Both signals are consulted: V8 only reports a termination it has already - // materialized, which needs JS to run, and a graph parked on a promise - // nothing settles never gives it any. Message-only exception: building a - // V8 error on a terminating isolate is not allowed. - if (isolate->IsExecutionTerminating() || - (runtime != nullptr && runtime->IsTerminationRequested())) { - LogEsmPhase(canonicalPath, "evaluate", "terminated"); - // Probed, not consumed: only a still-pending promise leaves a - // half-evaluated module in the registry. One that already settled is - // complete, and evicting it would throw away a good entry for no reason - // — the result simply goes unused. - if (promise->State() == Promise::kPending) { - RemoveModuleFromRegistry(isolate, canonicalPath); - } - throw NativeScriptException("Module evaluation interrupted by isolate termination: " + - canonicalPath); - } + throwIfTerminating(); Promise::PromiseState state = promise->State(); if (state != Promise::kPending) { @@ -1411,6 +1433,11 @@ throw NativeScriptException("Module evaluation interrupted by isolate terminatio } } + // The loop leaves through its condition when a pump materializes the + // termination, and through the deadline when the request arrived during the + // final slice. + throwIfTerminating(); + if (!settled && promise->State() == Promise::kPending) { LogEsmPhase(canonicalPath, "evaluate", "promise-timeout"); if (options.timeoutBehavior == ModuleEvaluationOptions::TimeoutBehavior::kThrow) { diff --git a/NativeScript/runtime/NativeScriptException.mm b/NativeScript/runtime/NativeScriptException.mm index e3f8ebb9..d124ec1e 100644 --- a/NativeScript/runtime/NativeScriptException.mm +++ b/NativeScript/runtime/NativeScriptException.mm @@ -85,6 +85,15 @@ static void ConsiderStackCandidate(PendingErrorDisplay& state, v8::Isolate* isol NativeScriptException::NativeScriptException(Isolate* isolate, TryCatch& tc, const std::string& message) { + // A caught termination has no exception value, message or stack to read: + // V8 hands back a sentinel, and formatting it would run JS on an isolate + // that must not run any. + if (tc.HasTerminated()) { + this->javascriptException_ = nullptr; + this->message_ = message; + this->name_ = "NativeScriptException"; + return; + } Local error = tc.Exception(); this->javascriptException_ = new Persistent(isolate, tc.Exception()); this->message_ = GetErrorMessage(isolate, error, message); @@ -333,7 +342,7 @@ static void ScheduleDeferredThrow(Isolate* isolate, NSException* e) { if (error->IsObject()) { auto errObject = error.As(); auto fullMessageString = tns::ToV8String(isolate, "fullMessage"); - if (errObject->HasOwnProperty(context, fullMessageString).ToChecked()) { + if (errObject->HasOwnProperty(context, fullMessageString).FromMaybe(false)) { // check if we have a "fullMessage" on the error, and log that instead - since it includes // more info about the exception. v8::Local fullMessage_; @@ -765,7 +774,7 @@ static bool GiveWorkerOnErrorAChance(Isolate* isolate, Local context, L std::string errMessage; bool hasFullErrorMessage = false; auto v8FullMessage = tns::ToV8String(isolate, "fullMessage"); - if (error->IsObject() && error.As()->Has(context, v8FullMessage).ToChecked()) { + if (error->IsObject() && error.As()->Has(context, v8FullMessage).FromMaybe(false)) { hasFullErrorMessage = true; Local errMsgVal; bool success = error.As()->Get(context, v8FullMessage).ToLocal(&errMsgVal); diff --git a/NativeScript/runtime/Worker.mm b/NativeScript/runtime/Worker.mm index 5e718684..a0af0bc0 100644 --- a/NativeScript/runtime/Worker.mm +++ b/NativeScript/runtime/Worker.mm @@ -433,7 +433,7 @@ throw NativeScriptException( // vocabulary updates). tns::LoaderVocabulary inheritedVocabulary = tns::CaptureLoaderVocabulary(isolate); - std::function func([worker, workerPath, inheritedVocabulary, resourceLimits]() { + std::function func([worker, workerPath, inheritedVocabulary, resourceLimits]() { // Name the looper thread after its entry script so a crash report // identifies which worker died instead of an anonymous NSOperationQueue // thread. Darwin caps thread names at 63 bytes; keep the basename only. @@ -477,6 +477,14 @@ throw NativeScriptException( // the worker's scripts are visible to the debugger from the start. worker->CreateInspector(isolate, resolvedPath); + // From here terminate() interrupts this isolate. A terminate() that + // landed before now had nothing to interrupt, so it is honored here, + // before any app code runs: the thread goes straight to teardown. + worker->PublishIsolate(isolate); + if (worker->IsTerminating()) { + return; + } + TryCatch tc(isolate); // If the script can be determined missing up-front, report it through @@ -491,7 +499,7 @@ throw NativeScriptException( worker->PassUncaughtExceptionFromWorkerToMain( "Worker script does not exist: " + resolvedPath, resolvedPath, "", 1, true); worker->Terminate(); - return isolate; + return; } } @@ -500,16 +508,22 @@ throw NativeScriptException( } catch (NativeScriptException& ex) { // Re-arm the failure as the pending V8 exception (the original JS // error when one was captured) so the tc.HasCaught() path below - // routes it to worker.onerror with full detail. - Isolate::Scope isolate_scope(isolate); - HandleScope handle_scope(isolate); - ex.ReThrowToV8(isolate); + // routes it to worker.onerror with full detail. Not on an isolate + // that is terminating: the failure then is the termination itself, + // and throwing on such an isolate is not allowed. + if (!worker->IsTerminating()) { + Isolate::Scope isolate_scope(isolate); + HandleScope handle_scope(isolate); + ex.ReThrowToV8(isolate); + } } - // The near-heap-limit callback has already reported to the parent and - // asked V8 to terminate this isolate; everything below would run JS on it. - if (worker->HeapLimitExceeded()) { - return isolate; + // The entry was cut short — by terminate(), or by the near-heap-limit + // callback, which has already reported to the parent and asked V8 to + // terminate this isolate. Everything below would run JS on it, and a + // terminated worker reports no error. + if (worker->IsTerminating()) { + return; } // WHATWG parity: enable the implicit port's message queue once the @@ -590,8 +604,6 @@ throw NativeScriptException( worker->PassUncaughtExceptionFromWorkerToMain(context, tc, true); worker->Terminate(); } - - return isolate; }); // The registry entry has to exist before the worker can run: the worker diff --git a/NativeScript/runtime/WorkerWrapper.mm b/NativeScript/runtime/WorkerWrapper.mm index 5b3ba448..3306f891 100644 --- a/NativeScript/runtime/WorkerWrapper.mm +++ b/NativeScript/runtime/WorkerWrapper.mm @@ -69,16 +69,23 @@ static void PostToLoop(const std::shared_ptr& loop, std::functionisClosing_; } +const bool WorkerWrapper::IsTerminating() { return this->isTerminating_; } + const int WorkerWrapper::WorkerId() { return this->workerId_; } +void WorkerWrapper::PublishIsolate(Isolate* isolate) { + std::lock_guard lock(this->workerIsolateMutex_); + this->workerIsolate_ = isolate; +} + void WorkerWrapper::PostMessage(std::shared_ptr message) { if (!this->isTerminating_ && !this->isClosing_) { this->queue_.Push(message); } } -void WorkerWrapper::Start(std::shared_ptr> poWorker, - std::function func, std::optional qualityOfService) { +void WorkerWrapper::Start(std::shared_ptr> poWorker, std::function func, + std::optional qualityOfService) { this->poWorker_ = poWorker; // Set before the operation is queued: a worker that terminates inside its // entry script clears this flag from its own thread, and a store made after @@ -232,7 +239,7 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptr func) { +void WorkerWrapper::BackgroundLooper(std::function func) { if (!this->isTerminating_) { CFRunLoopRef runLoop = CFRunLoopGetCurrent(); this->queue_.Initialize( @@ -243,15 +250,13 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptr lock(this->workerIsolateMutex_); - this->workerIsolate_ = workerIsolate; - } + // Publishes the isolate itself, before it runs the entry script. + func(); this->DrainPendingTasks(); - // check again as it could terminate before this + // A terminate() that interrupted the entry, or the entry's own close(), + // ends the worker here without a loop turn. if (!this->isTerminating_) { CFRunLoopRun(); } @@ -350,12 +355,6 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptrworkerIsolate_)) { workerRuntime->RequestTermination(); } @@ -409,10 +408,11 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptrPassUncaughtExceptionFromWorkerToMain(worker->heapLimitMessage_, worker->heapLimitSource_, "", 0, true); - // Terminate() only reaches an isolate BackgroundLooper has already published, - // which happens after the entry script finished evaluating — and a worker - // that exhausts its heap usually does so inside that entry. Ask the isolate - // this callback belongs to directly. + // Terminate() only reaches a published isolate, and this callback is armed + // before publication: a cap small enough can be hit while Runtime::Init is + // still evaluating builtins. Ask the isolate this callback belongs to + // directly, so the GC that is running finishes and whatever JS is on the + // stack unwinds either way. if (Runtime* runtime = Runtime::GetRuntime(worker->heapLimitIsolate_)) { runtime->RequestTermination(); } @@ -470,7 +470,9 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptrisTerminating_) { + // A termination is not an error: a dying worker reports nothing, and a + // TryCatch that caught one holds no exception value to hand a handler. + if (this->isTerminating_ || tc.HasTerminated()) { return; } Local context = Caches::Get(isolate)->GetContext(); @@ -552,6 +554,12 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptr context, TryCatch& tc, bool async) { + // Same rule as CallOnErrorHandlers. The string overload stays open: the + // paths that report a failure and then terminate the worker themselves (a + // missing entry, the heap cap) go through it before they set the flag. + if (this->isTerminating_ || tc.HasTerminated()) { + return; + } Isolate* workerIsolate = v8::Isolate::GetCurrent(); int lineNumber = 0; std::string message = ""; diff --git a/TestRunner/app/tests/WorkerTerminateTests.js b/TestRunner/app/tests/WorkerTerminateTests.js new file mode 100644 index 00000000..a2d7ea8f --- /dev/null +++ b/TestRunner/app/tests/WorkerTerminateTests.js @@ -0,0 +1,112 @@ +// terminate() reaches a worker that is still inside its entry script, the way +// it does in Node and on the web: the running entry is interrupted, the thread +// winds down, and nothing is reported as an error. +describe("Worker terminate during entry evaluation", function () { + var busyEntry = "./workerTerminate/busyEntryWorker.js"; + var parkedEntry = "./workerTerminate/parkedEntryWorker.mjs"; + + var originalTimeout; + beforeEach(function () { + originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL; + jasmine.DEFAULT_TIMEOUT_INTERVAL = 60000; + }); + afterEach(function () { + jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout; + }); + + // Resolves once the worker's thread has ended; rejects when it has not + // within `limitMs`, which is the old behaviour for an entry that never + // returns. + function waitForEnd(worker, limitMs) { + return new Promise(function (resolve, reject) { + var timer = setTimeout(function () { + reject(new Error("worker did not end within " + limitMs + "ms")); + }, limitMs); + worker.addEventListener("nsworkerended", function () { + clearTimeout(timer); + resolve(); + }); + }); + } + + // A terminated worker reports no error, from either hop. + function failOnError(worker) { + worker.onerror = function (event) { + fail("unexpected worker error: " + event.message); + return true; + }; + } + + function settle(done) { + return function (err) { + if (err) { + fail(err.message); + } + done(); + }; + } + + it("interrupts an entry script spinning in a synchronous loop", function (done) { + var worker = new Worker(busyEntry); + failOnError(worker); + worker.onmessage = function (event) { + expect(event.data).toBe("spinning"); + var ended = waitForEnd(worker, 5000); + worker.terminate(); + ended.then(settle(done), settle(done)); + }; + }); + + it("ends an ES module entry parked in a top-level await", function (done) { + var worker = new Worker(parkedEntry); + failOnError(worker); + worker.onmessage = function (event) { + expect(event.data).toBe("parked"); + var ended = waitForEnd(worker, 5000); + worker.terminate(); + ended.then(settle(done), settle(done)); + }; + }); + + // Round i terminates i milliseconds after construction, which lands + // anywhere from before the thread has started, through runtime setup, to + // inside the entry's settle pump. + it("ends a worker terminated at any point of its startup", function (done) { + var ROUNDS = 16; + (function round(i) { + if (i === ROUNDS) { + done(); + return; + } + var worker = new Worker(parkedEntry); + failOnError(worker); + var ended = waitForEnd(worker, 5000); + if (i === 0) { + worker.terminate(); + } else { + setTimeout(function () { worker.terminate(); }, i); + } + ended.then(function () { round(i + 1); }, settle(done)); + })(0); + }); + + it("resolves a node:worker_threads terminate() for a worker stuck in its entry", function (done) { + var wt = require("node:worker_threads"); + var worker = new wt.Worker("~/tests/workerTerminate/busyEntryWorker.js"); + var exitCode = null; + worker.on("error", function (err) { + fail("unexpected worker error: " + err); + }); + worker.on("exit", function (code) { + exitCode = code; + }); + worker.on("message", function (data) { + expect(data).toBe("spinning"); + worker.terminate().then(function (code) { + expect(code).toBe(0); + expect(exitCode).toBe(0); + done(); + }, settle(done)); + }); + }); +}); diff --git a/TestRunner/app/tests/index.js b/TestRunner/app/tests/index.js index 31bfe10a..f2dee09c 100644 --- a/TestRunner/app/tests/index.js +++ b/TestRunner/app/tests/index.js @@ -199,6 +199,9 @@ require("./ExtendedClassNamingTests"); // Worker wrapper reachability across GC (strong while running, collectable after) require("./WorkerLifetimeTests"); +// terminate() landing inside a worker's entry script +require("./WorkerTerminateTests"); + // Tests common for all runtimes (git submodule of NativeScript/common-runtime-tests-app). require("../shared/index").runAllTests(); diff --git a/TestRunner/app/tests/workerTerminate/busyEntryWorker.js b/TestRunner/app/tests/workerTerminate/busyEntryWorker.js new file mode 100644 index 00000000..0395bd41 --- /dev/null +++ b/TestRunner/app/tests/workerTerminate/busyEntryWorker.js @@ -0,0 +1,4 @@ +// Tells the parent the entry is running, then never returns: only a +// termination interrupt can end this worker. +postMessage("spinning"); +for (;;) {} diff --git a/TestRunner/app/tests/workerTerminate/parkedEntryWorker.mjs b/TestRunner/app/tests/workerTerminate/parkedEntryWorker.mjs new file mode 100644 index 00000000..05c2b4d8 --- /dev/null +++ b/TestRunner/app/tests/workerTerminate/parkedEntryWorker.mjs @@ -0,0 +1,5 @@ +// Tells the parent the entry is running, then parks the module graph on a +// promise nothing settles: the entry never finishes on its own, so the worker +// sits in the loader's settle pump and, once that gives up, in its event loop. +postMessage("parked"); +await new Promise(function () {}); diff --git a/docs/worker-threads.md b/docs/worker-threads.md index eda7855a..f1d2ff02 100644 --- a/docs/worker-threads.md +++ b/docs/worker-threads.md @@ -49,12 +49,12 @@ means deliberately unsupported. | `markAsUncloneable(obj)` | real | Brands `obj` so serializing it at all is a `DataCloneError`, in `structuredClone` and every `postMessage` alike. | | `setEnvironmentData(key, value)` | real, deviates | Clones and stores process-wide. No per-thread snapshot — see below. Passing `undefined` (or omitting the value) deletes the key. | | `getEnvironmentData(key)` | real, deviates | Deserializes a fresh copy per read, on any isolate. | -| `resourceLimits` | shim | Always `{}`; the runtime imposes no per-worker limits and reports none. | +| `resourceLimits` | shim | Always `{}`, on the main isolate and inside a worker alike. The constructor *option* of that name is real — see [Worker options](#worker-options) — but a worker cannot read its own caps back through this export. | | `SHARE_ENV` | shim | Exported so the spelling resolves, but inert — see below. | | `threadName` | shim | Always `undefined`. | | `workerData` | shim | Always `null` — see below. | | `parentPort` | shim | `null` on the main isolate. Inside a worker, a `MessagePort`-shaped `EventTarget` over the worker's existing parent channel: `postMessage` forwards to the global `postMessage`, `message`/`messageerror` are re-dispatched from the worker global scope, `start()` and `close()` are no-ops. It is **not** a real port: not transferable, no queue of its own. | -| `Worker` | shim | A class over the runtime's global `Worker` with a small Node-style emitter (`on`/`once`/`off`/`removeListener`) for `message`, `messageerror`, `error`, `online` and `exit`. `postMessage(value, transfer)` and `terminate()` forward. `online` is emitted off a microtask after construction, not from the thread. `exit` (always code `0`) fires exactly once, when the thread has ended, whether the worker was terminated or ended by its own `close()`; `terminate()` resolves at the same point. Unsupported options throw a `TypeError` naming the option: `workerData`, `env`, `eval`, `transferList`, and `stdin`/`stdout`/`stderr` when explicitly truthy. | +| `Worker` | shim | A class over the runtime's global `Worker` with a small Node-style emitter (`on`/`once`/`off`/`removeListener`) for `message`, `messageerror`, `error`, `online` and `exit`. `postMessage(value, transfer)` and `terminate()` forward. `online` is emitted off a microtask after construction, not from the thread. `exit` (always code `0`) fires exactly once, when the thread has ended, whether the worker was terminated or ended by its own `close()`; `terminate()` resolves at the same point. Unsupported options throw a `TypeError` naming the option: `workerData`, `env`, `eval`, `transferList`, and `stdin`/`stdout`/`stderr` when explicitly truthy. The runtime's own options, `ios` and `resourceLimits`, pass through unchanged — see [Worker options](#worker-options). | | `postMessageToThread` | throws | `Error: postMessageToThread is not supported in this runtime`. | | `moveMessagePortToContext` | throws | `Error: moveMessagePortToContext is not supported in this runtime`. | | `locks` | absent | Web Locks are not implemented; the property does not exist. | @@ -269,6 +269,54 @@ code relies on. Transfer is not part of that leniency — a port in a worker transfer list is validated exactly as it is everywhere else, since degrading a transfer would strand the port's sibling. +## Worker options + +The runtime's `Worker` constructor takes two options of its own, and the +`node:worker_threads` shim passes both through unchanged. Unknown keys inside +either object are ignored, so a later runtime can add more without breaking an +older one. + +### `ios.priority` + +The quality of service of the worker's thread: `"userInteractive"`, +`"userInitiated"`, `"default"`, `"utility"` or `"background"`. Omitting it +leaves the operation queue's own default. A non-object `ios`, a non-string +priority or an unrecognized name throws a `TypeError`; `ios: null` is the same +as no `ios` at all. The older top-level `iosPriority` is still accepted, with a +one-time deprecation warning, and `ios.priority` wins when both are given. + +### `resourceLimits` + +Node's option, at the top level of the options object: + +```js +new Worker("./w.js", { + resourceLimits: { + maxOldGenerationSizeMb: 64, + maxYoungGenerationSizeMb: 8, + jsDispatchTableSizeMb: 64, + }, +}); +``` + +| key | effect | +|---|---| +| `maxOldGenerationSizeMb` | Caps the worker isolate's old generation. | +| `maxYoungGenerationSizeMb` | Caps its young generation. | +| `jsDispatchTableSizeMb` | A NativeScript extension: the isolate's JS dispatch table reservation, a whole number of megabytes from 1 to 256. Worker isolates reserve 64 MB by default instead of V8's 256 MB; the main isolate keeps V8's default. | + +Node's `stackSizeMb` and `codeRangeSizeMb` are ignored like any other unknown +key. A `resourceLimits` that is not an object, or a key that is not a number, +throws a `TypeError`; a non-finite, non-positive or oversized value throws a +`RangeError`, as does a fractional or out-of-range `jsDispatchTableSizeMb`. +`resourceLimits: null` is the same as omitting it. + +Exhausting a worker's heap does not take the process down. Every worker +isolate, capped or not, watches its heap limit: a worker that reaches it is +terminated, and the parent's `Worker` receives an `error` event whose message +is `Worker JS heap out of memory`, followed by the cap in parentheses when +`maxOldGenerationSizeMb` was set — `(maxOldGenerationSizeMb: 32)`. + ## Worker lifetime **A `Worker` is held strongly by the runtime from the moment its thread starts @@ -294,6 +342,21 @@ The root is released when the worker ends — `terminate()`, or the worker's own runtime drops the native side with it. Nothing about a *finished* worker is kept alive. +### `terminate()` reaches the entry script + +`terminate()` stops the worker wherever it is, as in Node and on the web. A +worker still evaluating its entry script — spinning in a synchronous loop, +parked in a top-level `await`, or waiting on a remote module fetch — is +interrupted there, exactly like one idling in its event loop: the thread winds +down, `nsworkerended` fires, and a `node:worker_threads` `terminate()` +resolves. A termination is not an error, so a worker cut short in its entry +produces neither a worker-scope `onerror` call nor an `error` event on the +parent's `Worker`. A `terminate()` that lands before the worker's runtime even +exists is honored at the first opportunity, before any app code runs. + +The worker's own `close()` is different: it lets the script that called it run +to completion and ends the worker once that returns, as on the web. + ### `nsworkerended` When the worker's thread has finished, the runtime dispatches a plain `Event` From 6f54a44592a288b7b6501d53ab012c86bd7bbf30 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 7 Oct 2026 16:42:56 -0300 Subject: [PATCH 2/3] fix(worker): keep a termination from being re-armed as an error on its way out A termination that interrupts a CommonJS entry unwinds through the native require callback, whose catch re-threw the message-only failure onto the isolate as an ordinary Error: V8 then reported an exception rather than a termination to RunModule's TryCatch, which built a full error report before the worker boundary dropped it. ReThrowToV8 now leaves a terminating isolate alone, since the pending termination is the failure, and RunModule consults the same three termination signals as the settle pump. The pumped graph walk ahead of an entry's compile bails on termination but reports nothing, so LoadESModule went on to the synchronous HTTP loader, which can block on the network. Both walks are now followed by a termination check. Also refreshes the drain comment that described the pre-publication isolate window, widens the startup-sweep spec's spread so its rounds reach the settle pump, and corrects the resourceLimits RangeError wording. --- NativeScript/runtime/ModuleInternal.mm | 30 +++++++++++++++++-- NativeScript/runtime/NativeScriptException.mm | 9 ++++++ NativeScript/runtime/WorkerWrapper.mm | 14 +++++---- TestRunner/app/tests/WorkerTerminateTests.js | 13 ++++---- docs/worker-threads.md | 5 ++-- 5 files changed, 54 insertions(+), 17 deletions(-) diff --git a/NativeScript/runtime/ModuleInternal.mm b/NativeScript/runtime/ModuleInternal.mm index 4dbad283..5488993a 100644 --- a/NativeScript/runtime/ModuleInternal.mm +++ b/NativeScript/runtime/ModuleInternal.mm @@ -258,7 +258,10 @@ static bool IsHttpModulePath(const std::string& path) { moduleNamespace = ModuleInternal::LoadESModule(isolate, path, BootEntryEvaluationOptions(isHttpModule)); } catch (const NativeScriptException& ex) { - if (RuntimeConfig.IsDebug) { + Runtime* runtime = Runtime::GetRuntime(isolate); + bool terminating = isolate->IsExecutionTerminating() || + (runtime != nullptr && runtime->IsTerminationRequested()); + if (RuntimeConfig.IsDebug && !terminating) { Log(@"***** JavaScript exception occurred *****"); Log(@"Error loading ES module: %s", path.c_str()); Log(@"Exception: %s", ex.getMessage().c_str()); @@ -294,8 +297,12 @@ throw NativeScriptException( if (!success || tc.HasCaught()) { // A termination is caught like an exception but carries no error value; - // naming it as the failure keeps it from being reported as one. - if (tc.HasTerminated()) { + // naming it as the failure keeps it from being reported as one. All three + // signals, as in the settle pump: a native frame between here and the + // interrupted JS may have swallowed the sentinel on its way out. + Runtime* runtime = Runtime::GetRuntime(isolate); + if (tc.HasTerminated() || isolate->IsExecutionTerminating() || + (runtime != nullptr && runtime->IsTerminationRequested())) { throw NativeScriptException("Module evaluation interrupted by isolate termination: " + path); } if (RuntimeConfig.IsDebug) { @@ -1461,6 +1468,21 @@ throw NativeScriptException("Module evaluation interrupted by isolate terminatio return MaybeLocal(); } +// A pumped graph walk bails on a termination request but reports nothing; +// what follows it would compile, fetch synchronously or evaluate on an +// isolate that must not run anything more. +static void ThrowIfLoadInterruptedByTermination(Isolate* isolate, + const std::string& canonicalPath) { + Runtime* runtime = Runtime::GetRuntime(isolate); + if (!isolate->IsExecutionTerminating() && + (runtime == nullptr || !runtime->IsTerminationRequested())) { + return; + } + LogEsmPhase(canonicalPath, "load", "terminated"); + throw NativeScriptException("Module evaluation interrupted by isolate termination: " + + canonicalPath); +} + Local ModuleInternal::LoadESModule(Isolate* isolate, const std::string& path, const ModuleEvaluationOptions& options) { bool isHttpModule = IsHttpModulePath(path); @@ -1540,6 +1562,7 @@ throw NativeScriptException("Module evaluation interrupted by isolate terminatio // registry hit and instantiation resolves as pure lookup. On timeout or // partial coverage the legacy synchronous path still owns correctness. RunModuleGraphLoadPumped(isolate, context, requestPath, kModuleEvaluateDeadlineSeconds); + ThrowIfLoadInterruptedByTermination(isolate, canonicalPath); MaybeLocal maybeMod = LoadHttpModuleForUrl(isolate, context, requestPath); if (!maybeMod.ToLocal(&module)) { logPhase("compile", "fail", "http-loader"); @@ -1563,6 +1586,7 @@ throw NativeScriptException("Module evaluation interrupted by isolate terminatio // async fetch, so it legitimately waits here and then evaluates // synchronously. RunModuleGraphLoadPumped(isolate, context, canonicalPath, kModuleEvaluateDeadlineSeconds); + ThrowIfLoadInterruptedByTermination(isolate, canonicalPath); auto walkedIt = registry.find(canonicalPath); if (walkedIt != registry.end()) { Local walked = walkedIt->second.Get(isolate); diff --git a/NativeScript/runtime/NativeScriptException.mm b/NativeScript/runtime/NativeScriptException.mm index d124ec1e..29efcf57 100644 --- a/NativeScript/runtime/NativeScriptException.mm +++ b/NativeScript/runtime/NativeScriptException.mm @@ -714,6 +714,15 @@ static bool GiveWorkerOnErrorAChance(Isolate* isolate, Local context, L } void NativeScriptException::ReThrowToV8(Isolate* isolate) { + // An isolate that is terminating keeps its termination pending until the JS + // frames below unwind; throwing on it would replace that with an ordinary + // error, and building the error allocates on a heap that may have just hit + // its cap. The failure being re-armed here is the termination itself. + Runtime* runtime = Runtime::GetRuntime(isolate); + if (isolate->IsExecutionTerminating() || + (runtime != nullptr && runtime->IsTerminationRequested())) { + return; + } @try { // The Isolate::Scope here is necessary because the Exception::Error method internally relies on // the Isolate::GetCurrent method which might return null if we do not use the proper scope diff --git a/NativeScript/runtime/WorkerWrapper.mm b/NativeScript/runtime/WorkerWrapper.mm index 3306f891..d4d1796c 100644 --- a/NativeScript/runtime/WorkerWrapper.mm +++ b/NativeScript/runtime/WorkerWrapper.mm @@ -154,12 +154,14 @@ static void PostToLoop(const std::shared_ptr& loop, std::functionworkerIsolate_ == nullptr) { return; } diff --git a/TestRunner/app/tests/WorkerTerminateTests.js b/TestRunner/app/tests/WorkerTerminateTests.js index a2d7ea8f..573cd8d8 100644 --- a/TestRunner/app/tests/WorkerTerminateTests.js +++ b/TestRunner/app/tests/WorkerTerminateTests.js @@ -15,8 +15,7 @@ describe("Worker terminate during entry evaluation", function () { }); // Resolves once the worker's thread has ended; rejects when it has not - // within `limitMs`, which is the old behaviour for an entry that never - // returns. + // within `limitMs`. function waitForEnd(worker, limitMs) { return new Promise(function (resolve, reject) { var timer = setTimeout(function () { @@ -68,9 +67,11 @@ describe("Worker terminate during entry evaluation", function () { }; }); - // Round i terminates i milliseconds after construction, which lands - // anywhere from before the thread has started, through runtime setup, to - // inside the entry's settle pump. + // Round i terminates 25·i milliseconds after construction. Runtime setup + // takes a few hundred milliseconds on a simulator and the local entry's + // settle pump lasts one second after it, so the rounds land anywhere from + // before the thread has started, through runtime setup, to inside the + // pump. it("ends a worker terminated at any point of its startup", function (done) { var ROUNDS = 16; (function round(i) { @@ -84,7 +85,7 @@ describe("Worker terminate during entry evaluation", function () { if (i === 0) { worker.terminate(); } else { - setTimeout(function () { worker.terminate(); }, i); + setTimeout(function () { worker.terminate(); }, i * 25); } ended.then(function () { round(i + 1); }, settle(done)); })(0); diff --git a/docs/worker-threads.md b/docs/worker-threads.md index f1d2ff02..be4f0262 100644 --- a/docs/worker-threads.md +++ b/docs/worker-threads.md @@ -307,8 +307,9 @@ new Worker("./w.js", { Node's `stackSizeMb` and `codeRangeSizeMb` are ignored like any other unknown key. A `resourceLimits` that is not an object, or a key that is not a number, -throws a `TypeError`; a non-finite, non-positive or oversized value throws a -`RangeError`, as does a fractional or out-of-range `jsDispatchTableSizeMb`. +throws a `TypeError`; a non-finite value, one worth less than a byte, or one +too large to hold in bytes throws a `RangeError`, as does a fractional or +out-of-range `jsDispatchTableSizeMb`. `resourceLimits: null` is the same as omitting it. Exhausting a worker's heap does not take the process down. Every worker From a6e5daace4ca1487ebb902f749bcc189d5284055 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 7 Oct 2026 17:23:01 -0300 Subject: [PATCH 3/3] fix(worker): consult the termination-requested flag at the last two failure sites The CommonJS module-function call and the ES module Evaluate failure only recognized a termination V8 had already materialized. A request that landed during the call, before any JS ran to materialize it, let an ordinary failure be formatted as an error on a terminating isolate. Both sites now read the same three signals as the settle pump and the require path. --- NativeScript/runtime/ModuleInternal.mm | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/NativeScript/runtime/ModuleInternal.mm b/NativeScript/runtime/ModuleInternal.mm index 5488993a..ae4db889 100644 --- a/NativeScript/runtime/ModuleInternal.mm +++ b/NativeScript/runtime/ModuleInternal.mm @@ -894,7 +894,9 @@ throw NativeScriptException(isolate, moduleFunc->Call(context, thiz, sizeof(requireArgs) / sizeof(Local), requireArgs) .ToLocal(&result); if (!success || tc.HasCaught()) { - if (tc.HasTerminated()) { + Runtime* runtime = Runtime::GetRuntime(isolate); + if (tc.HasTerminated() || isolate->IsExecutionTerminating() || + (runtime != nullptr && runtime->IsTerminationRequested())) { throw NativeScriptException("Module evaluation interrupted by isolate termination: " + modulePath); } @@ -1295,7 +1297,9 @@ throw NativeScriptException( RemoveModuleFromRegistry(isolate, canonicalPath); // Same rule as the pump below: a termination outranks any failure detail, // and reading the TryCatch as an error would run JS on the dying isolate. - if (tcEval.HasTerminated() || isolate->IsExecutionTerminating()) { + Runtime* runtime = Runtime::GetRuntime(isolate); + if (tcEval.HasTerminated() || isolate->IsExecutionTerminating() || + (runtime != nullptr && runtime->IsTerminationRequested())) { LogEsmPhase(canonicalPath, "evaluate", "terminated"); throw NativeScriptException("Module evaluation interrupted by isolate termination: " + canonicalPath);