Skip to content

fix(worker): terminate() reaches a worker still inside its entry script #493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
edusperoni wants to merge 3 commits into from
Open

fix(worker): terminate() reaches a worker still inside its entry script #493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1f141da
fix(worker): terminate() reaches a worker still inside its entry script
edusperoni Oct 7, 2026
6f54a44
fix(worker): keep a termination from being re-armed as an error on it…
edusperoni Oct 7, 2026
a6e5daa
fix(worker): consult the termination-requested flag at the last two f…
edusperoni Oct 7, 2026
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
43 changes: 27 additions & 16 deletions NativeScript/runtime/DataWrapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<v8::Persistent<v8::Value>> poWorker,
std::function<v8::Isolate*()> func,
std::function<void()> func,
std::optional<int> 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
Expand Down Expand Up @@ -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<size_t> 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
Expand All @@ -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_.
Expand Down Expand Up @@ -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<bool> isRunning_;
Expand Down Expand Up @@ -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_;
Expand All @@ -707,7 +718,7 @@ class WorkerWrapper : public BaseDataWrapper {
// task would run on.
std::shared_ptr<std::atomic<WorkerWrapper*>> selfRef_;

void BackgroundLooper(std::function<v8::Isolate*()> func);
void BackgroundLooper(std::function<void()> func);
void DrainPendingTasks();
void ForwardErrorPayloadToMain(const std::string& message,
const std::string& source,
Expand Down
101 changes: 78 additions & 23 deletions NativeScript/runtime/ModuleInternal.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -293,6 +296,15 @@ 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. 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) {
Log(@"***** JavaScript exception occurred *****");
Log(@"Error in require() call:");
Expand Down Expand Up @@ -882,6 +894,12 @@ throw NativeScriptException(isolate,
moduleFunc->Call(context, thiz, sizeof(requireArgs) / sizeof(Local<Value>), requireArgs)
.ToLocal(&result);
if (!success || tc.HasCaught()) {
Runtime* runtime = Runtime::GetRuntime(isolate);
if (tc.HasTerminated() || isolate->IsExecutionTerminating() ||
(runtime != nullptr && runtime->IsTerminationRequested())) {
throw NativeScriptException("Module evaluation interrupted by isolate termination: " +
modulePath);
}
throw NativeScriptException(isolate, tc, "Error calling module function");
}
}
Expand Down Expand Up @@ -1277,6 +1295,15 @@ throw NativeScriptException(
Local<Value> 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.
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);
}
const char* classification = "unknown";
if (tcEval.HasCaught()) {
Local<Message> msg = tcEval.Message();
Expand Down Expand Up @@ -1364,32 +1391,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) {
Expand All @@ -1411,6 +1444,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) {
Expand All @@ -1434,6 +1472,21 @@ throw NativeScriptException("Module evaluation interrupted by isolate terminatio
return MaybeLocal<Promise>();
}

// 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<Value> ModuleInternal::LoadESModule(Isolate* isolate, const std::string& path,
const ModuleEvaluationOptions& options) {
bool isHttpModule = IsHttpModulePath(path);
Expand Down Expand Up @@ -1513,6 +1566,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<Module> maybeMod = LoadHttpModuleForUrl(isolate, context, requestPath);
if (!maybeMod.ToLocal(&module)) {
logPhase("compile", "fail", "http-loader");
Expand All @@ -1536,6 +1590,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<Module> walked = walkedIt->second.Get(isolate);
Expand Down
22 changes: 20 additions & 2 deletions NativeScript/runtime/NativeScriptException.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Value> error = tc.Exception();
this->javascriptException_ = new Persistent<Value>(isolate, tc.Exception());
this->message_ = GetErrorMessage(isolate, error, message);
Expand Down Expand Up @@ -333,7 +342,7 @@ static void ScheduleDeferredThrow(Isolate* isolate, NSException* e) {
if (error->IsObject()) {
auto errObject = error.As<Object>();
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<v8::Value> fullMessage_;
Expand Down Expand Up @@ -705,6 +714,15 @@ static bool GiveWorkerOnErrorAChance(Isolate* isolate, Local<Context> 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
Expand Down Expand Up @@ -765,7 +783,7 @@ static bool GiveWorkerOnErrorAChance(Isolate* isolate, Local<Context> context, L
std::string errMessage;
bool hasFullErrorMessage = false;
auto v8FullMessage = tns::ToV8String(isolate, "fullMessage");
if (error->IsObject() && error.As<Object>()->Has(context, v8FullMessage).ToChecked()) {
if (error->IsObject() && error.As<Object>()->Has(context, v8FullMessage).FromMaybe(false)) {
hasFullErrorMessage = true;
Local<Value> errMsgVal;
bool success = error.As<Object>()->Get(context, v8FullMessage).ToLocal(&errMsgVal);
Expand Down
36 changes: 24 additions & 12 deletions NativeScript/runtime/Worker.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ throw NativeScriptException(
// vocabulary updates).
tns::LoaderVocabulary inheritedVocabulary = tns::CaptureLoaderVocabulary(isolate);

std::function<Isolate*()> func([worker, workerPath, inheritedVocabulary, resourceLimits]() {
std::function<void()> 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.
Expand Down Expand Up @@ -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
Expand All @@ -491,7 +499,7 @@ throw NativeScriptException(
worker->PassUncaughtExceptionFromWorkerToMain(
"Worker script does not exist: " + resolvedPath, resolvedPath, "", 1, true);
worker->Terminate();
return isolate;
return;
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading