Visitar URL original
fix(worker): keep the worker isolate alive while terminate() uses it, and let it stop a closing worker by adrian-niculescu · Pull Request #2067 · NativeScript/android · GitHub
Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
fix(worker): keep the worker isolate alive while terminate() uses it
Terminate() read the worker isolate and then interrupted it with no lock, while the worker thread could clear the pointer and dispose that isolate in the same window: a worker that ends through its own close() while its parent calls terminate(), or a parent that terminates its children during its own shutdown. Terminate() now holds a mutex across the read and the use, and the worker thread takes it when it withdraws the isolate, before disposing it.
  • Loading branch information
adrian-niculescu committed Oct 6, 2026
commit a9a5ea44006dd3021fbeb7ea46aea30f50c5a087
33 changes: 22 additions & 11 deletions test-app/runtime/src/main/cpp/WorkerWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,23 @@ void WorkerWrapper::Terminate() {
return;
}

Isolate* isolate = workerIsolate_.load();
if (isolate != nullptr) {
// The only v8 call that is legal from another thread - interrupts any
// JS currently running on the worker (e.g. a busy loop).
isolate->TerminateExecution();
// A pump parked with nothing queued runs no JS, so the interrupt
// above never materializes for it - the loop's own flag ends it.
auto loop = NativeScriptPlatform::Instance()->LookupEventLoop(isolate);
if (loop != nullptr) {
loop->NoteTerminationRequested();
{
// Held across the use, not just the read: the worker thread withdraws
// the isolate under the same mutex before disposing it, so a terminate
// that already read it finishes with it first, and a later one finds
// null.
std::lock_guard<std::mutex> lock(workerIsolateMutex_);
Isolate* isolate = workerIsolate_.load();
if (isolate != nullptr) {
// The only v8 call that is legal from another thread - interrupts any
// JS currently running on the worker (e.g. a busy loop).
isolate->TerminateExecution();
// A pump parked with nothing queued runs no JS, so the interrupt
// above never materializes for it - the loop's own flag ends it.
auto loop = NativeScriptPlatform::Instance()->LookupEventLoop(isolate);
if (loop != nullptr) {
loop->NoteTerminationRequested();
}
}
}

Expand Down Expand Up @@ -656,7 +663,11 @@ void WorkerWrapper::BackgroundLooper(std::shared_ptr<WorkerWrapper> self) {
// bootstrap failed between initWorkerRuntime and the workerIsolate_
// publish (e.g. a JNI error while resolving the looper), the atomic is
// still null while the isolate very much needs disposing.
workerIsolate_.store(nullptr);
{
// Waits out a Terminate() on another thread that is still using it.
std::lock_guard<std::mutex> lock(workerIsolateMutex_);
workerIsolate_.store(nullptr);
}
Isolate* isolate = runtime_->GetIsolate();
{
v8::Locker locker(isolate);
Expand Down
4 changes: 4 additions & 0 deletions test-app/runtime/src/main/cpp/WorkerWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,11 @@ class WorkerWrapper : public std::enable_shared_from_this<WorkerWrapper> {
// The parent runtime's task queue; weak so a child outliving its parent
// just drops its posts instead of touching a dead runtime.
std::weak_ptr<EventLoop> parentTasks_;
// Written by the worker thread only: published once the runtime is up,
// withdrawn before the isolate is disposed. Any other thread reads and uses
// it under workerIsolateMutex_.
std::atomic<v8::Isolate*> workerIsolate_;
std::mutex workerIsolateMutex_;
Runtime* runtime_;

const int workerId_;
Expand Down
Loading