From 168ff7cf100dccc823b171babe51aa83bf86a33a Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:16:47 +0300 Subject: [PATCH 1/2] fix(worker): keep a worker isolate alive while a __runOnMainThread callback locks it RunMainThreadEntry read the target isolate from its entry, released the cache lock and only then took the isolate's Locker. A worker that ended in between removed the entry and disposed the isolate, and the main thread then locked freed memory. The main thread now holds the isolate from the moment it takes the entry until it releases the Locker, and a worker waits for those holds before disposing its isolate. --- .../runtime/src/main/cpp/CallbackHandlers.cpp | 28 ++++++++++++++++++- .../runtime/src/main/cpp/CallbackHandlers.h | 13 +++++++++ test-app/runtime/src/main/cpp/Runtime.cpp | 7 +++-- .../runtime/src/main/cpp/WorkerWrapper.cpp | 1 + 4 files changed, 45 insertions(+), 4 deletions(-) diff --git a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp index 8d2b18135..d29f8c7ff 100644 --- a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp +++ b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp @@ -730,7 +730,24 @@ void CallbackHandlers::RunMainThreadEntry(uint64_t key) { return; } isolate = it->second.isolate_; - } + // Taken with the entry, under the same lock RemoveIsolateEntries + // takes, so a worker either removes the entry first or waits for this + // hold before disposing the isolate. + ++heldIsolates_[isolate]; + } + // Declared before the Locker so that it lets go only after the Locker is + // released. + struct IsolateHold { + Isolate *isolate; + ~IsolateHold() { + std::lock_guard lock(cacheMutex_); + auto held = heldIsolates_.find(isolate); + if (--held->second == 0) { + heldIsolates_.erase(held); + isolateReleased_.notify_all(); + } + } + } hold{isolate}; v8::Locker locker(isolate); Isolate::Scope isolate_scope(isolate); @@ -1932,8 +1949,17 @@ void CallbackHandlers::RemoveIsolateEntries(v8::Isolate *isolate) { } } } + +void CallbackHandlers::WaitForMainThreadCallbacks(v8::Isolate *isolate) { + std::unique_lock lock(cacheMutex_); + isolateReleased_.wait(lock, [isolate]() { + return heldIsolates_.find(isolate) == heldIsolates_.end(); + }); +} robin_hood::unordered_map CallbackHandlers::cache_; std::mutex CallbackHandlers::cacheMutex_; +robin_hood::unordered_map CallbackHandlers::heldIsolates_; +std::condition_variable CallbackHandlers::isolateReleased_; std::atomic_int64_t CallbackHandlers::count_ = {0}; diff --git a/test-app/runtime/src/main/cpp/CallbackHandlers.h b/test-app/runtime/src/main/cpp/CallbackHandlers.h index 86dba64a5..56c1a437a 100644 --- a/test-app/runtime/src/main/cpp/CallbackHandlers.h +++ b/test-app/runtime/src/main/cpp/CallbackHandlers.h @@ -3,6 +3,7 @@ #include #include +#include #include #include #include "JEnv.h" @@ -163,6 +164,14 @@ namespace tns { static void RemoveIsolateEntries(v8::Isolate *isolate); + /* + * Blocks until no __runOnMainThread callback still holds `isolate`. + * A worker calls it after releasing its Locker and before disposing + * the isolate: a callback that took the isolate before + * RemoveIsolateEntries ran may be waiting on that Locker. + */ + static void WaitForMainThreadCallbacks(v8::Isolate *isolate); + private: CallbackHandlers() { @@ -248,6 +257,10 @@ namespace tns { // thread (multithreaded JS, workers), each under a different // isolate's Locker, so the Lockers provide no mutual exclusion static std::mutex cacheMutex_; + // How many RunMainThreadEntry calls hold each isolate, from reading + // its entry until they release its Locker; guarded by cacheMutex_ + static robin_hood::unordered_map heldIsolates_; + static std::condition_variable isolateReleased_; }; diff --git a/test-app/runtime/src/main/cpp/Runtime.cpp b/test-app/runtime/src/main/cpp/Runtime.cpp index d7ab015e8..c1e08e211 100644 --- a/test-app/runtime/src/main/cpp/Runtime.cpp +++ b/test-app/runtime/src/main/cpp/Runtime.cpp @@ -1175,9 +1175,10 @@ void Runtime::DestroyRuntime() { m_dispatchNativeUncaughtErrorFunc.Reset(); // Both hold v8::Global handles to JS callbacks, so their entries must be // dropped here rather than in ~Runtime, which runs after Isolate::Dispose -- - // resetting a Global then writes into a freed handle table. Doing it here - // also closes a window in which the main thread could take a Locker on this - // isolate (RunMainThreadEntry) after it had already been disposed. + // resetting a Global then writes into a freed handle table. A + // RunMainThreadEntry that has not taken its entry yet finds it gone; one that + // already has is waited for before the isolate is disposed + // (CallbackHandlers::WaitForMainThreadCallbacks). CallbackHandlers::RemoveIsolateEntries(m_isolate); FrameCallbacks::RemoveIsolateEntries(m_isolate); diff --git a/test-app/runtime/src/main/cpp/WorkerWrapper.cpp b/test-app/runtime/src/main/cpp/WorkerWrapper.cpp index 04abc68fd..b513e084d 100644 --- a/test-app/runtime/src/main/cpp/WorkerWrapper.cpp +++ b/test-app/runtime/src/main/cpp/WorkerWrapper.cpp @@ -667,6 +667,7 @@ void WorkerWrapper::BackgroundLooper(std::shared_ptr self) { isolate->RemoveNearHeapLimitCallback(WorkerWrapper::OnNearHeapLimit, 0); runtime_->DestroyRuntime(); } + CallbackHandlers::WaitForMainThreadCallbacks(isolate); isolate->Dispose(); // Dispose freed the isolate's memory, so its address can be reused by // a concurrent Isolate::New - drop the platform's loop entry now, not From a340e10143105ed90a4a19afbd4ea202f829c513 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:28:44 +0300 Subject: [PATCH 2/2] fix(worker): report a worker's __runOnMainThread exception without its isolate handle The exception a worker's callback threw kept a handle into the worker isolate after RunMainThreadEntry let go of it, so the isolate could be disposed before the event loop reported it, and the report resolved the handle against the main thread's isolate. For a worker's callback the exception now drops the handle while the isolate is still held, and only its message and stack reach Java. --- test-app/runtime/src/main/cpp/CallbackHandlers.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp index d29f8c7ff..1e3da91ce 100644 --- a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp +++ b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp @@ -772,8 +772,15 @@ void CallbackHandlers::RunMainThreadEntry(uint64_t key) { if (tc.HasCaught() && !NativeScriptException::ContainUncaughtCallbackException(isolate, tc)) { + NativeScriptException ex(tc); + if (!runtime->IsMainThread()) { + // Reported only after this function lets go of the isolate, which + // its worker may dispose by then, and from the main thread, which + // never enters it: only the message and stack can travel. + ex.ReleaseJsHandle(); + } // surfaces via the event loop's guard as a pending Java exception - throw NativeScriptException(tc); + throw ex; } }