From 6d8e58bd2ececf6e3830339f051b6ad2fb7fb08a Mon Sep 17 00:00:00 2001
From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com>
Date: Thu, 17 Sep 2026 13:55:40 +0300
Subject: [PATCH 1/2] fix(worker): isolate and wrapper lifetime fixes around
 startup and teardown

A throwing onclose run from the entry script read the worker isolate before it was published and crashed. Terminate() used the worker isolate with no synchronization against the worker thread deleting its runtime. EndWrapperLifetime kept using the wrapper after dispatching nsworkerended, whose listeners can shut the runtime down and delete it.
---
 NativeScript/runtime/DataWrapper.h            | 12 ++++-
 NativeScript/runtime/Worker.mm                |  4 +-
 NativeScript/runtime/WorkerWrapper.mm         | 48 ++++++++++++++-----
 TestRunner/app/tests/MessagingTests.js        | 10 ++++
 .../tests/messaging/throwingOncloseWorker.js  |  6 +++
 5 files changed, 64 insertions(+), 16 deletions(-)
 create mode 100644 TestRunner/app/tests/messaging/throwingOncloseWorker.js

diff --git a/NativeScript/runtime/DataWrapper.h b/NativeScript/runtime/DataWrapper.h
index b0c04abf..84f6e7b4 100644
--- a/NativeScript/runtime/DataWrapper.h
+++ b/NativeScript/runtime/DataWrapper.h
@@ -553,12 +553,16 @@ class WorkerWrapper : public BaseDataWrapper {
   void Start(std::shared_ptr<v8::Persistent<v8::Value>> poWorker,
              std::function<v8::Isolate*()> func,
              std::optional<int> qualityOfService = std::nullopt);
-  void CallOnErrorHandlers(v8::TryCatch& tc);
+  // 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.
+  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
   // follows the same web order: the worker scope's `onerror` first, then — only
   // if that did not handle it — the parent's Worker error event.
-  void ReportEntryEvaluationRejection(v8::Local<v8::Context> context,
+  void ReportEntryEvaluationRejection(v8::Isolate* isolate,
+                                      v8::Local<v8::Context> context,
                                       v8::Local<v8::Value> reason);
   void PassUncaughtExceptionFromWorkerToMain(v8::Local<v8::Context> context,
                                              v8::TryCatch& tc,
@@ -629,7 +633,11 @@ class WorkerWrapper : public BaseDataWrapper {
 
  private:
   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_.
   v8::Isolate* workerIsolate_;
+  std::mutex workerIsolateMutex_;
   std::atomic<bool> isRunning_;
   std::atomic<bool> isClosing_;
   std::atomic<bool> isTerminating_;
diff --git a/NativeScript/runtime/Worker.mm b/NativeScript/runtime/Worker.mm
index 053ca7ba..14e5dc70 100644
--- a/NativeScript/runtime/Worker.mm
+++ b/NativeScript/runtime/Worker.mm
@@ -564,7 +564,7 @@ throw NativeScriptException(
                                       ? info[0]
                                       : Local<Value>(v8::Exception::Error(tns::ToV8String(
                                             iso, "Worker entry module evaluation rejected")));
-            w->ReportEntryEvaluationRejection(ctx, reason);
+            w->ReportEntryEvaluationRejection(iso, ctx, reason);
           };
           Local<v8::Function> onFulfilled;
           Local<v8::Function> onRejected;
@@ -798,7 +798,7 @@ throw NativeScriptException(
     TryCatch tc(isolate);
     success = onCloseFunc->Call(context, v8::Undefined(isolate), 0, args).ToLocal(&result);
     if (!success && tc.HasCaught()) {
-      worker->CallOnErrorHandlers(tc);
+      worker->CallOnErrorHandlers(isolate, tc);
     }
   }
 }
diff --git a/NativeScript/runtime/WorkerWrapper.mm b/NativeScript/runtime/WorkerWrapper.mm
index 90ddcb57..209ec1f5 100644
--- a/NativeScript/runtime/WorkerWrapper.mm
+++ b/NativeScript/runtime/WorkerWrapper.mm
@@ -120,17 +120,26 @@ static void PostToLoop(const std::shared_ptr<EventLoop>& loop, std::function<voi
 }
 
 void WorkerWrapper::EndWrapperLifetime() {
-  Local<Value> worker =
-      this->poWorker_ != nullptr ? this->poWorker_->Get(this->mainIsolate_) : Local<Value>();
+  // The dispatch below runs listeners, and a listener may shut the runtime
+  // down, whose teardown deletes this wrapper. Everything the dispatch needs is
+  // read first, and the liveness token says afterwards whether `this` is still
+  // there to unroot.
+  Isolate* isolate = this->mainIsolate_;
+  std::shared_ptr<std::atomic<WorkerWrapper*>> selfRef = this->selfRef_;
+  Local<Value> worker = this->poWorker_ != nullptr ? this->poWorker_->Get(isolate) : Local<Value>();
   if (!worker.IsEmpty() && worker->IsObject()) {
-    TryCatch tc(this->mainIsolate_);
-    Worker::EmitEnded(this->mainIsolate_, worker.As<Object>());
+    TryCatch tc(isolate);
+    Worker::EmitEnded(isolate, worker.As<Object>());
     if (tc.HasCaught()) {
       Local<Value> error = tc.Exception();
-      Log(@"%s", tns::ToString(this->mainIsolate_, error).c_str());
-      this->mainIsolate_->ThrowException(error);
+      Log(@"%s", tns::ToString(isolate, error).c_str());
+      isolate->ThrowException(error);
     }
   }
+  if (selfRef->load(std::memory_order_acquire) == nullptr) {
+    // Deleted during the dispatch; that teardown released the Worker object.
+    return;
+  }
   this->UnrootWorkerObject();
 }
 
@@ -181,7 +190,7 @@ static void PostToLoop(const std::shared_ptr<EventLoop>& loop, std::function<voi
     this->onMessage_(this->workerIsolate_, globalTarget, message);
 
     if (tc.HasCaught()) {
-      this->CallOnErrorHandlers(tc);
+      this->CallOnErrorHandlers(this->workerIsolate_, tc);
     }
   }
 
@@ -232,7 +241,11 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptr<Even
         },
         this);
 
-    this->workerIsolate_ = func();
+    Isolate* workerIsolate = func();
+    {
+      std::lock_guard<std::mutex> lock(this->workerIsolateMutex_);
+      this->workerIsolate_ = workerIsolate;
+    }
 
     this->DrainPendingTasks();
 
@@ -242,6 +255,14 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptr<Even
     }
   }
 
+  // Withdrawn before the runtime and its isolate go away below. Terminate()
+  // uses the isolate under this mutex, so a terminate that already read it has
+  // finished with it by the time this returns, and a later one finds null.
+  {
+    std::lock_guard<std::mutex> lock(this->workerIsolateMutex_);
+    this->workerIsolate_ = nullptr;
+  }
+
   // The inspector must be gone before the Runtime (and with it the isolate)
   // is deleted below.
   this->DestroyInspector();
@@ -292,6 +313,9 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptr<Even
   // set terminating to true atomically
   bool wasTerminating = this->isTerminating_.exchange(true);
   if (!wasTerminating) {
+    // Held across the use, not just the read: the worker thread withdraws the
+    // isolate under the same mutex before deleting its runtime.
+    std::unique_lock<std::mutex> isolateLock(this->workerIsolateMutex_);
     if (this->workerIsolate_ != nullptr) {
       // Flagged before the request so a pump that is between iterations sees
       // it on its next check, rather than only once V8 has some JS to
@@ -307,6 +331,7 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptr<Even
       }
       this->workerIsolate_->TerminateExecution();
     }
+    isolateLock.unlock();
     {
       // A worker paused at a breakpoint sits in the inspector's nested pause
       // loop, not in the CFRunLoop — kick it loose so TerminateExecution and
@@ -414,11 +439,10 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptr<Even
   delete client;
 }
 
-void WorkerWrapper::CallOnErrorHandlers(TryCatch& tc) {
+void WorkerWrapper::CallOnErrorHandlers(Isolate* isolate, TryCatch& tc) {
   if (this->isTerminating_) {
     return;
   }
-  Isolate* isolate = this->workerIsolate_;
   Local<Context> context = Caches::Get(isolate)->GetContext();
   Local<Object> global = context->Global();
 
@@ -447,11 +471,11 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptr<Even
   this->PassUncaughtExceptionFromWorkerToMain(context, tc);
 }
 
-void WorkerWrapper::ReportEntryEvaluationRejection(Local<Context> context, Local<Value> reason) {
+void WorkerWrapper::ReportEntryEvaluationRejection(Isolate* isolate, Local<Context> context,
+                                                   Local<Value> reason) {
   if (this->isTerminating_) {
     return;
   }
-  Isolate* isolate = this->workerIsolate_;
   Local<Object> global = context->Global();
 
   Local<Value> onErrorVal;
diff --git a/TestRunner/app/tests/MessagingTests.js b/TestRunner/app/tests/MessagingTests.js
index 59441b26..ceaf2fe8 100644
--- a/TestRunner/app/tests/MessagingTests.js
+++ b/TestRunner/app/tests/MessagingTests.js
@@ -252,6 +252,16 @@ describe("Messaging runtime edges", function () {
                 }
             };
         });
+
+        it("reports an error onclose threw while the entry script was still running", function (done) {
+            var worker = new Worker("./messaging/throwingOncloseWorker.js");
+            worker.onerror = function (event) {
+                event.preventDefault();
+                expect(event.message).toContain("boom from onclose");
+                worker.terminate();
+                done();
+            };
+        });
     });
 
     describe("AbortSignal handler attribute accounting", function () {
diff --git a/TestRunner/app/tests/messaging/throwingOncloseWorker.js b/TestRunner/app/tests/messaging/throwingOncloseWorker.js
new file mode 100644
index 00000000..fceedf0a
--- /dev/null
+++ b/TestRunner/app/tests/messaging/throwingOncloseWorker.js
@@ -0,0 +1,6 @@
+// Closes from inside the entry script, so onclose runs before the entry has
+// finished evaluating.
+onclose = function () {
+    throw new Error("boom from onclose");
+};
+close();

From 1965a835bbd9e45ec1b6ade5766f8d210499503c Mon Sep 17 00:00:00 2001
From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com>
Date: Thu, 17 Sep 2026 14:27:33 +0300
Subject: [PATCH 2/2] fix(worker): serialize the message queue's run loop
 source with its teardown

Push() and Signal() read the run loop source and run loop with no lock while Terminate(), on another thread, invalidates and releases the source. They now go through the mutex Terminate() holds, and the worker thread terminates its queue before it leaves, since the queue borrows that thread's run loop. Also removes the unused GetWorkerIsolate() getter, which handed out the isolate without the mutex that now guards it.
---
 NativeScript/runtime/ConcurrentQueue.cpp | 26 ++++++++----------------
 NativeScript/runtime/ConcurrentQueue.h   |  1 -
 NativeScript/runtime/DataWrapper.h       |  1 -
 NativeScript/runtime/WorkerWrapper.mm    |  6 ++++++
 4 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/NativeScript/runtime/ConcurrentQueue.cpp b/NativeScript/runtime/ConcurrentQueue.cpp
index 6e34073f..7f331fc7 100644
--- a/NativeScript/runtime/ConcurrentQueue.cpp
+++ b/NativeScript/runtime/ConcurrentQueue.cpp
@@ -15,10 +15,6 @@ void ConcurrentQueue::Initialize(CFRunLoopRef runLoop, void (*performWork)(void*
 }
 
 void ConcurrentQueue::Push(std::shared_ptr<worker::Message> message) {
-    if (this->runLoopTasksSource_ != nullptr && !CFRunLoopSourceIsValid(this->runLoopTasksSource_)) {
-        return;
-    }
-
     {
       // Checked under the queue mutex, where Terminate() also flips it while
       // emptying the queue: a push that loses the race is dropped rather than
@@ -30,7 +26,7 @@ void ConcurrentQueue::Push(std::shared_ptr<worker::Message> message) {
         this->messagesQueue_.push(message);
     }
 
-    this->SignalAndWakeUp();
+    this->Signal();
 }
 
 std::vector<std::shared_ptr<worker::Message>> ConcurrentQueue::PopAll() {
@@ -52,20 +48,14 @@ bool ConcurrentQueue::IsEmpty() {
 }
 
 void ConcurrentQueue::Signal() {
-  // Mirrors Push()'s validity handling instead of SignalAndWakeUp()'s
-  // assert: a retry racing Terminate() must be a silent no-op.
-  if (this->runLoopTasksSource_ == nullptr ||
-      !CFRunLoopSourceIsValid(this->runLoopTasksSource_)) {
-    return;
+  // Serializes signaling and waking with Initialize() and Terminate().
+  // Terminate() clears both pointers and invalidates and releases the source;
+  // the run loop is borrowed from the worker thread, which terminates the
+  // queue before it leaves.
+  std::unique_lock<std::mutex> lock(initializationMutex_);
+  if (this->runLoopTasksSource_ != nullptr) {
+    CFRunLoopSourceSignal(this->runLoopTasksSource_);
   }
-  this->SignalAndWakeUp();
-}
-
-void ConcurrentQueue::SignalAndWakeUp() {
-    if (this->runLoopTasksSource_ != nullptr) {
-        tns::Assert(CFRunLoopSourceIsValid(this->runLoopTasksSource_));
-        CFRunLoopSourceSignal(this->runLoopTasksSource_);
-    }
 
     if (this->runLoop_ != nullptr) {
         CFRunLoopWakeUp(this->runLoop_);
diff --git a/NativeScript/runtime/ConcurrentQueue.h b/NativeScript/runtime/ConcurrentQueue.h
index b8fe1694..33c78220 100644
--- a/NativeScript/runtime/ConcurrentQueue.h
+++ b/NativeScript/runtime/ConcurrentQueue.h
@@ -32,7 +32,6 @@ struct ConcurrentQueue {
     std::atomic<bool> terminated{false};
     std::mutex mutex_;
     std::mutex initializationMutex_;
-    void SignalAndWakeUp();
 };
 
 }
diff --git a/NativeScript/runtime/DataWrapper.h b/NativeScript/runtime/DataWrapper.h
index 84f6e7b4..66bc91dc 100644
--- a/NativeScript/runtime/DataWrapper.h
+++ b/NativeScript/runtime/DataWrapper.h
@@ -627,7 +627,6 @@ class WorkerWrapper : public BaseDataWrapper {
   const inline v8::Isolate* GetMainIsolate() { return mainIsolate_; }
   // The only route from the worker thread to the parent: see mainLoop_.
   std::weak_ptr<EventLoop> MainLoop() const { return mainLoop_; }
-  const inline v8::Isolate* GetWorkerIsolate() { return workerIsolate_; }
   const inline void MakeWeak() { isWeak_ = true; }
   const inline bool IsWeak() { return isWeak_; }
 
diff --git a/NativeScript/runtime/WorkerWrapper.mm b/NativeScript/runtime/WorkerWrapper.mm
index 209ec1f5..2cc9213d 100644
--- a/NativeScript/runtime/WorkerWrapper.mm
+++ b/NativeScript/runtime/WorkerWrapper.mm
@@ -255,6 +255,12 @@ static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptr<Even
     }
   }
 
+  // The queue borrows this thread's run loop, so it lets go of it before the
+  // thread leaves: a terminate() that made this thread skip the loop above
+  // only reaches its own queue_.Terminate() later, possibly after the thread
+  // and its run loop are gone. A second Terminate() finds nothing set.
+  this->queue_.Terminate();
+
   // Withdrawn before the runtime and its isolate go away below. Terminate()
   // uses the isolate under this mutex, so a terminate that already read it has
   // finished with it by the time this returns, and a later one finds null.