diff --git a/NativeScript/runtime/DataWrapper.h b/NativeScript/runtime/DataWrapper.h index 9a4e0029..b0c04abf 100644 --- a/NativeScript/runtime/DataWrapper.h +++ b/NativeScript/runtime/DataWrapper.h @@ -20,6 +20,8 @@ class WorkerInspectorClient; namespace tns { class PrimitiveDataWrapper; +struct ObjectWeakCallbackState; +class EventLoop; enum class WrapperType { Base = 1 << 0, @@ -598,6 +600,19 @@ class WorkerWrapper : public BaseDataWrapper { 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 + // isolate's thread only -- they re-arm that isolate's global handle -- and + // the unroot is idempotent, so an end reached by more than one path re-arms + // the finalizer once. + void RootWorkerObject(); + void UnrootWorkerObject(); + // Dispatches the end-of-worker event and unroots. Main isolate's thread, + // with the isolate entered and locked by the caller. + void EndWrapperLifetime(); + + ~WorkerWrapper(); const WrapperType Type(); const int Id(); @@ -606,6 +621,8 @@ class WorkerWrapper : public BaseDataWrapper { const bool IsClosing(); const int WorkerId(); const inline v8::Isolate* GetMainIsolate() { return mainIsolate_; } + // The only route from the worker thread to the parent: see mainLoop_. + std::weak_ptr MainLoop() const { return mainLoop_; } const inline v8::Isolate* GetWorkerIsolate() { return workerIsolate_; } const inline void MakeWeak() { isWeak_ = true; } const inline bool IsWeak() { return isWeak_; } @@ -625,6 +642,12 @@ class WorkerWrapper : public BaseDataWrapper { std::shared_ptr)> onMessage_; std::shared_ptr> poWorker_; + // The parent's event loop, taken on the parent's thread at construction. + // Every worker-thread post to the parent goes through it and never through + // the parent isolate: the parent runtime may be mid-teardown or its isolate + // already disposed when the post runs, whereas a loop that has shut down + // drops the post, and an expired pointer means the parent is gone entirely. + std::weak_ptr mainLoop_; ConcurrentQueue queue_; static std::atomic nextId_; int workerId_; @@ -645,6 +668,15 @@ class WorkerWrapper : public BaseDataWrapper { // handle may be created. static size_t OnNearHeapLimit(void* data, size_t current_heap_limit, size_t initial_heap_limit); + // Parked while the Worker object is rooted, so the unroot can re-arm the very + // finalizer ObjectManager::Register installed. Main isolate's thread only. + ObjectWeakCallbackState* weakCallbackState_ = nullptr; + bool workerObjectRooted_ = false; + // Cleared by the destructor, so a task posted from the worker thread can tell + // whether this wrapper still exists once it reaches the main isolate. The + // wrapper is only ever destroyed with that isolate locked, which is what the + // task takes before reading this. + std::shared_ptr> selfRef_; void BackgroundLooper(std::function func); void DrainPendingTasks(); diff --git a/NativeScript/runtime/ObjectManager.mm b/NativeScript/runtime/ObjectManager.mm index 2c12bc33..5787fe66 100644 --- a/NativeScript/runtime/ObjectManager.mm +++ b/NativeScript/runtime/ObjectManager.mm @@ -304,7 +304,16 @@ void DisposeHandle(v8::Isolate* isolate, case WrapperType::Worker: { WorkerWrapper* worker = static_cast(wrapper); if (!worker->isDisposed()) { - // during final disposal, inform the worker it should delete itself + // A running worker's Worker object is rooted (WorkerWrapper:: + // RootWorkerObject), so a weak callback should not reach a live worker + // at all. This refusal stays as the floor under that: re-arming keeps + // the wrapper alive for another cycle, which is safe, whereas freeing + // it while the thread still posts through it is not. Reaching it is not + // free either -- a re-armed handle that is also a weak-collection key + // can corrupt the collector's ephemeron bookkeeping -- so it is a + // fallback, not a mechanism to rely on. + // + // During final disposal, inform the worker it should delete itself. if (isFinalDisposal) { worker->MakeWeak(); } diff --git a/NativeScript/runtime/Worker.h b/NativeScript/runtime/Worker.h index e4ab1b6e..70c81467 100644 --- a/NativeScript/runtime/Worker.h +++ b/NativeScript/runtime/Worker.h @@ -30,6 +30,13 @@ class Worker { const std::string& message, const std::string& source, const std::string& stackTrace, int lineNumber); + // Dispatches `nsworkerended` on `receiver` (the Worker object, on the parent + // isolate) once the worker's thread has finished. Internal and non-standard: + // the web has no end-of-worker event, and the node:worker_threads shim is + // what turns this into an 'exit'. A listener that throws leaves the exception + // pending for the caller's TryCatch. No-op before InitEvents has run. + static void EmitEnded(v8::Isolate* isolate, v8::Local receiver); + static std::vector GlobalFunctions; private: diff --git a/NativeScript/runtime/Worker.mm b/NativeScript/runtime/Worker.mm index fa84367b..053ca7ba 100644 --- a/NativeScript/runtime/Worker.mm +++ b/NativeScript/runtime/Worker.mm @@ -22,11 +22,12 @@ namespace { // The worker-events builtin's delivery callouts for this isolate. Both message -// directions share emitMessage; only the receiver differs. emitError is -// parent-side only. +// directions share emitMessage; only the receiver differs. emitError and +// emitEnded are parent-side only. struct WorkerEventsState { Global emitMessage; Global emitError; + Global emitEnded; }; } // namespace @@ -294,10 +295,16 @@ bool ParseResourceLimits(Isolate* isolate, Local context, Local emitError->IsFunction(); tns::Assert(success, isolate); + Local emitEnded; + success = exports->Get(context, tns::ToV8String(isolate, "emitEnded")).ToLocal(&emitEnded) && + emitEnded->IsFunction(); + tns::Assert(success, isolate); + WorkerEventsState* state = Caches::StateFor(isolate); tns::Assert(state != nullptr, isolate); state->emitMessage.Reset(isolate, emitMessage.As()); state->emitError.Reset(isolate, emitError.As()); + state->emitEnded.Reset(isolate, emitEnded.As()); } void Worker::ConstructorCallback(const FunctionCallbackInfo& info) { @@ -596,6 +603,10 @@ throw NativeScriptException( Caches::Workers->Insert(worker->Id(), state); worker->Start(poWorker, func, qos); + // The thread is away, so from here the Worker object is a GC root. The + // parent's loop cannot run before this returns, so the thread-exit + // notification can never overtake this root. + worker->RootWorkerObject(); } catch (NativeScriptException& ex) { ex.ReThrowToV8(isolate); } @@ -625,8 +636,8 @@ throw NativeScriptException( // Resolved before anything is serialized: serializing a transfer list // detaches the caller's buffers, so bailing out afterwards would destroy // their contents without ever delivering the message. - auto runtime = static_cast(state->GetIsolate()->GetData(Constants::RUNTIME_SLOT)); - if (runtime == nullptr) { + std::shared_ptr mainLoop = worker->MainLoop().lock(); + if (mainLoop == nullptr) { return; } @@ -642,7 +653,7 @@ throw NativeScriptException( return; } - runtime->GetEventLoop()->PostInternal([state, message]() { + mainLoop->PostInternal([state, message]() { Isolate* isolate = state->GetIsolate(); v8::Locker locker(isolate); Isolate::Scope isolate_scope(isolate); @@ -752,6 +763,16 @@ throw NativeScriptException( return result->BooleanValue(isolate); } +void Worker::EmitEnded(Isolate* isolate, Local receiver) { + WorkerEventsState* state = Caches::StateFor(isolate); + if (state == nullptr || state->emitEnded.IsEmpty()) { + return; + } + Local context = Caches::Get(isolate)->GetContext(); + Local result; + (void)state->emitEnded.Get(isolate)->Call(context, receiver, 0, nullptr).ToLocal(&result); +} + void Worker::CloseWorkerCallback(const FunctionCallbackInfo& info) { Isolate* isolate = info.GetIsolate(); int workerId = Worker::GetWorkerId(isolate, info.This()); @@ -789,6 +810,10 @@ throw NativeScriptException( WorkerWrapper* worker = static_cast(wrapper); worker->Terminate(); + // The root is NOT released here: the wrapper stays strong until the thread + // has actually wound down and the thread-exit notification releases it, so + // no GC can condemn a wrapper whose thread is still draining — the + // ObjectManager resurrection fallback stays unreachable for workers. } void Worker::SetWorkerId(Isolate* isolate, int workerId) { diff --git a/NativeScript/runtime/WorkerWrapper.mm b/NativeScript/runtime/WorkerWrapper.mm index 61011d65..90ddcb57 100644 --- a/NativeScript/runtime/WorkerWrapper.mm +++ b/NativeScript/runtime/WorkerWrapper.mm @@ -4,6 +4,7 @@ #include "DataWrapper.h" #include "ErrorEvents.h" #include "Helpers.h" +#include "ObjectManager.h" #include "Runtime.h" #include "RuntimeConfig.h" #include "Worker.h" @@ -24,11 +25,8 @@ // Posts to the target runtime's internal lane from the worker thread. When // async is false, blocks until the entry ran - or until it is destroyed // unrun by a shutdown that raced the post, which must release the waiter too. -static void PostToRuntimeLoop(Runtime* runtime, std::function fn, bool async) { - auto loop = runtime->GetEventLoop(); - if (loop == nullptr) { - return; - } +static void PostToLoop(const std::shared_ptr& loop, std::function fn, + bool async) { if (async) { loop->PostInternal(std::move(fn)); return; @@ -57,7 +55,11 @@ static void PostToRuntimeLoop(Runtime* runtime, std::function fn, bool a isWeak_(false), messagesEnabled_(false), onMessage_(onMessage), - workerId_(nextId_.fetch_add(1, std::memory_order_relaxed) + 1) {} + mainLoop_(Runtime::GetRuntime(mainIsolate)->GetEventLoop()), + workerId_(nextId_.fetch_add(1, std::memory_order_relaxed) + 1), + selfRef_(std::make_shared>(this)) {} + +WorkerWrapper::~WorkerWrapper() { this->selfRef_->store(nullptr, std::memory_order_release); } const WrapperType WorkerWrapper::Type() { return WrapperType::Worker; } @@ -94,6 +96,44 @@ static void PostToRuntimeLoop(Runtime* runtime, std::function fn, bool a [workers_ addOperation:op]; } +void WorkerWrapper::RootWorkerObject() { + if (this->workerObjectRooted_ || this->poWorker_ == nullptr || this->poWorker_->IsEmpty() || + !this->poWorker_->IsWeak()) { + return; + } + this->weakCallbackState_ = this->poWorker_->ClearWeak(); + this->workerObjectRooted_ = true; +} + +void WorkerWrapper::UnrootWorkerObject() { + if (!this->workerObjectRooted_) { + return; + } + this->workerObjectRooted_ = false; + ObjectWeakCallbackState* state = this->weakCallbackState_; + this->weakCallbackState_ = nullptr; + if (state == nullptr || this->poWorker_ == nullptr || this->poWorker_->IsEmpty()) { + return; + } + this->poWorker_->SetWeak(state, ObjectManager::FinalizerCallback, + v8::WeakCallbackType::kFinalizer); +} + +void WorkerWrapper::EndWrapperLifetime() { + Local worker = + this->poWorker_ != nullptr ? this->poWorker_->Get(this->mainIsolate_) : Local(); + if (!worker.IsEmpty() && worker->IsObject()) { + TryCatch tc(this->mainIsolate_); + Worker::EmitEnded(this->mainIsolate_, worker.As()); + if (tc.HasCaught()) { + Local error = tc.Exception(); + Log(@"%s", tns::ToString(this->mainIsolate_, error).c_str()); + this->mainIsolate_->ThrowException(error); + } + } + this->UnrootWorkerObject(); +} + void WorkerWrapper::DrainPendingTasks() { // The drain source is armed (and can be signaled by a main-thread // PostMessage) BEFORE `workerIsolate_` is assigned in BackgroundLooper, and @@ -154,6 +194,33 @@ static void PostToRuntimeLoop(Runtime* runtime, std::function fn, bool a } } +// Hands the parent isolate the end-of-worker notification: the `nsworkerended` +// dispatch and the unroot that makes the Worker object collectable again. +// Takes only primitives plus the liveness token, because the wrapper it acts on +// may already be gone by the time the parent's loop gets here -- and, when the +// parent is shutting down, the post is dropped and the parent's teardown +// cascade owns disposal instead. +static void PostThreadEndedNotification(Isolate* mainIsolate, std::weak_ptr mainLoop, + std::shared_ptr> selfRef) { + std::shared_ptr loop = mainLoop.lock(); + if (loop == nullptr) { + return; + } + PostToLoop( + loop, + [mainIsolate, selfRef]() { + v8::Locker locker(mainIsolate); + Isolate::Scope isolate_scope(mainIsolate); + HandleScope handle_scope(mainIsolate); + WorkerWrapper* self = selfRef->load(std::memory_order_acquire); + if (self == nullptr) { + return; + } + self->EndWrapperLifetime(); + }, + true); +} + void WorkerWrapper::BackgroundLooper(std::function func) { if (!this->isTerminating_) { CFRunLoopRef runLoop = CFRunLoopGetCurrent(); @@ -188,20 +255,30 @@ static void PostToRuntimeLoop(Runtime* runtime, std::function fn, bool a this->heapLimitIsolate_ = nullptr; } + // Everything needed below is read first: publishing isDisposed_ is the last + // permitted touch of `this`. From that store on, a parent that is tearing + // down may delete this wrapper concurrently, and ~Runtime deletes it on this + // thread when the parent already handed ownership over. + Isolate* mainIsolate = this->mainIsolate_; + std::weak_ptr mainLoop = this->mainLoop_; + std::shared_ptr> selfRef = this->selfRef_; + int workerId = this->workerId_; this->isDisposed_ = true; + Runtime* runtime = Runtime::GetCurrentRuntime(); if (runtime != nullptr) { delete runtime; } else { // Runtime was never created (worker terminated before initialization). // The runtime destructor normally handles this cleanup, so do it here. - int workerId = this->workerId_; bool found; auto state = Caches::Workers->Get(workerId, found); if (found) { Caches::Workers->Remove(workerId); } } + + PostThreadEndedNotification(mainIsolate, mainLoop, selfRef); } void WorkerWrapper::EnableMessageQueue() { @@ -480,8 +557,8 @@ static void PostToRuntimeLoop(Runtime* runtime, std::function fn, bool a void WorkerWrapper::ForwardErrorPayloadToMain(const std::string& message, const std::string& source, const std::string& stackTrace, int lineNumber, bool async) { - auto runtime = static_cast(mainIsolate_->GetData(Constants::RUNTIME_SLOT)); - if (runtime == nullptr) { + std::shared_ptr loop = mainLoop_.lock(); + if (loop == nullptr) { return; } // The task runs later, on the parent's loop, and this wrapper may be gone by @@ -492,8 +569,8 @@ static void PostToRuntimeLoop(Runtime* runtime, std::function fn, bool a // the Worker object is gone and there is nothing left to report to. Isolate* mainIsolate = mainIsolate_; std::shared_ptr> poWorker = poWorker_; - PostToRuntimeLoop( - runtime, + PostToLoop( + loop, [mainIsolate, poWorker, message, source, stackTrace, lineNumber]() { v8::Locker locker(mainIsolate); Isolate::Scope isolate_scope(mainIsolate); diff --git a/NativeScript/runtime/js/README.md b/NativeScript/runtime/js/README.md index c1bdcf73..90f96478 100644 --- a/NativeScript/runtime/js/README.md +++ b/NativeScript/runtime/js/README.md @@ -117,10 +117,12 @@ The two extra rules a lazy builtin lives by: are whatever user code left behind, so it should not reach for them at all. - The per-instance wrappers `defineEventHandler` creates live on the target's **own listener bag**, under a private symbol — never in a WeakMap keyed by - the target. An ObjectManager-registered object (a `Worker`) can be - resurrected by its finalizer while its thread is alive, and a resurrected - object's weak-collection entries are already gone, so a WeakMap would hand - the revived object a fresh, empty handler map. + the target. Own-instance state is Node's own design for handler attributes, + and it keeps the builtins independent of the patched collector's handling of + resurrected ephemeron keys (`kFinalizer` resurrection interacting with + WeakMaps has been a source of collector bugs, and the patch is re-ported on + every V8 upgrade — builtins not leaning on it means a re-port mistake breaks + app-level tests, not the event system itself). - No `import`/`export` — these are classic function bodies, not modules. - ESLint (`eslint.config.mjs` at the repo root, run by lint-staged) declares `exports`, `require`, `module`, `binding`, `primordials` and the reachable diff --git a/NativeScript/runtime/js/events.js b/NativeScript/runtime/js/events.js index 22ba21cc..d2a706f5 100644 --- a/NativeScript/runtime/js/events.js +++ b/NativeScript/runtime/js/events.js @@ -44,12 +44,12 @@ function setListenerErrorReporter(fn) { // Event name -> handler-attribute wrapper (see defineEventHandler), stored on // the target's own listener bag under a symbol so it cannot collide with an -// event type. Deliberately NOT a WeakMap keyed by the target: a Worker is an -// ObjectManager-registered object whose finalizer resurrects it while its -// thread is alive, and a resurrected object's weak-collection entries are -// already gone. Each wrapper carries a `delta` that the listener count is -// corrected by: the wrapper occupies one slot in the listener list from its -// first assignment onwards, but a cleared handler is not a listener. +// event type. Deliberately NOT a WeakMap keyed by the target: the wrappers +// live with the target, as Node keeps them, and stay independent of how the +// collector treats weak-collection entries of objects that native code keeps +// alive. Each wrapper carries a `delta` that the listener count is corrected +// by: the wrapper occupies one slot in the listener list from its first +// assignment onwards, but a cleared handler is not a listener. var kHandlers = Symbol("handlers"); function handlersOf(target) { diff --git a/NativeScript/runtime/js/node-worker-threads.js b/NativeScript/runtime/js/node-worker-threads.js index 7f26e573..77e02756 100644 --- a/NativeScript/runtime/js/node-worker-threads.js +++ b/NativeScript/runtime/js/node-worker-threads.js @@ -28,6 +28,7 @@ const { ObjectCreate, ObjectDefineProperty, ObjectFreeze, + Promise, PromisePrototypeThen, PromiseResolve, SymbolFor, @@ -145,6 +146,8 @@ class WorkerEmitter { class Worker extends WorkerEmitter { #worker; #exited = false; + // Every terminate() promise settles when the thread's end is reported. + #exitWaiters = []; constructor(filename, options) { super(); @@ -182,24 +185,53 @@ class Worker extends WorkerEmitter { worker.onerror = function (error) { self.emit("error", error); }; + // The runtime's end-of-worker event: the one place 'exit' comes from, for + // a worker's own close() and for terminate() alike, so nothing the worker + // sent before it ended can follow 'exit'. + FunctionPrototypeCall( + addEventListener, + worker, + "nsworkerended", + function () { + self.#reportExit(); + } + ); soon(function () { self.emit("online", undefined); }); } + // Node emits 'exit' once and settles terminate() after it. The code is + // always 0: this runtime has no thread exit status to report, and the + // cross-runtime suite pins that for every end a worker can take. + #reportExit() { + if (this.#exited) { + return; + } + this.#exited = true; + this.emit("exit", 0); + const waiters = this.#exitWaiters; + this.#exitWaiters = []; + for (let i = 0; i < waiters.length; i++) { + waiters[i](0); + } + } + postMessage(value, transfer) { this.#worker.postMessage(value, transfer); } + // Resolves with the exit code once the thread has actually ended. A parent + // that is itself tearing down never delivers that signal, so the promise + // stays pending there, as it does in Node when the parent dies. terminate() { + if (this.#exited) { + return PromiseResolve(0); + } this.#worker.terminate(); const self = this; - return PromisePrototypeThen(PromiseResolve(), function () { - if (!self.#exited) { - self.#exited = true; - self.emit("exit", 0); - } - return 0; + return new Promise(function (resolve) { + ArrayPrototypePush(self.#exitWaiters, resolve); }); } } diff --git a/NativeScript/runtime/js/worker-events.js b/NativeScript/runtime/js/worker-events.js index ecc3f089..c64b1cfc 100644 --- a/NativeScript/runtime/js/worker-events.js +++ b/NativeScript/runtime/js/worker-events.js @@ -12,6 +12,7 @@ const { ObjectDefineProperty, ObjectSetPrototypeOf } = primordials; const { + Event, EventTarget, defineEventHandler, dispatchEventRethrowing, @@ -73,6 +74,15 @@ function emitError(message, filename, lineno, stackTrace) { return event.defaultPrevented; } +// The parent-side end-of-worker callout, invoked by native with the Worker +// object as `this` once the worker's thread has finished — its own close() as +// much as a terminate(). `nsworkerended` is internal and non-standard: the web +// has no end-of-worker event, and the node:worker_threads shim is what turns +// this into an 'exit'. +function emitEnded() { + dispatchEventRethrowing(this, new Event("nsworkerended")); +} + ObjectSetPrototypeOf(g.Worker.prototype, EventTarget.prototype); defineEventHandler(g.Worker.prototype, "message"); defineEventHandler(g.Worker.prototype, "messageerror"); @@ -99,4 +109,4 @@ for (const name of ["onmessage", "onmessageerror"]) { }); } -module.exports = { emitMessage, emitError }; +module.exports = { emitMessage, emitError, emitEnded }; diff --git a/TestRunner/app/tests/WorkerLifetimeTests.js b/TestRunner/app/tests/WorkerLifetimeTests.js new file mode 100644 index 00000000..f1f73d80 --- /dev/null +++ b/TestRunner/app/tests/WorkerLifetimeTests.js @@ -0,0 +1,258 @@ +// Worker lifetime under GC. A running worker's JS wrapper is a GC root, so it +// behaves like any other strongly held object: weak collections keyed on it +// keep their entries, and it keeps answering messages nobody holds a reference +// to it for. Once the worker ends — terminate() or its own close() — the root +// is dropped and the wrapper becomes collectable. + +describe("Worker lifetime", function () { + const WORKER_COUNT = 4; + const PAYLOAD_SIZE = 64; + + // A collection per runloop turn: weak-collection clearing needs turns after + // the collect, so nothing here asserts synchronously after __collect(). + function pollGC(predicate, cb) { + let turns = 0; + (function poll() { + __collect(); + if (predicate() || turns >= 100) { + cb(); + return; + } + turns++; + setTimeout(poll, 20); + })(); + } + + // Reached through a call rather than a closure, so the worker it derefs + // cannot end up in a scope the caller's later callbacks keep alive. + function terminateWorker(ref) { + const worker = ref.deref(); + if (worker !== undefined) { + worker.terminate(); + } + } + + function postToWorker(ref, message) { + const worker = ref.deref(); + if (worker !== undefined) { + worker.postMessage(message); + } + } + + // Enough allocation to put V8 part-way through an incremental/concurrent + // mark, so the collection that follows finishes a mark that was already + // running rather than starting an atomic one. + function churn() { + let sink = null; + for (let i = 0; i < 24; i++) { + const block = new Array(8192); + for (let j = 0; j < 8192; j++) { + block[j] = { j: j, s: "churn-" + j }; + } + sink = block; + } + return sink !== null; + } + + function makePayload(id) { + const payload = new Array(PAYLOAD_SIZE); + for (let i = 0; i < PAYLOAD_SIZE; i++) { + payload[i] = "payload-" + id + "-" + i; + } + return payload; + } + + it("a live Worker survives GC as a WeakMap key", function (done) { + // Nothing outside this map holds the values: an entry whose key stays + // alive while its value is not marked is what leaves a dangling value + // slot behind. + const sideTable = new WeakMap(); + const refs = []; + let replies = 0; + + for (let i = 0; i < WORKER_COUNT; i++) { + refs.push((function () { + const worker = new Worker("./eventLoopEchoWorker.js"); + // A second entry reachable only through the first one's value, + // so resolving these takes more than one ephemeron pass. + const link = { id: i }; + sideTable.set(link, { deep: i, payload: makePayload("deep" + i) }); + sideTable.set(worker, { id: i, link: link, payload: makePayload(i) }); + worker.onmessage = function () { replies++; }; + worker.postMessage("ping"); + return new WeakRef(worker); + })()); + } + + let round = 0; + function spin() { + churn(); + // async execution runs the collection from a task, so V8 treats the + // stack as pointer-free and the workers are genuinely unreachable + // for it — a conservative scan of this frame would not let them be. + __collect({ execution: "async" }).then(function () { + __collect(); + + // Only some turns touch the workers: a turn that does not leaves + // them dead for a whole mark cycle. + if (round % 3 === 0) { + for (let i = 0; i < refs.length; i++) { + postToWorker(refs[i], "ping-" + round); + } + } + + round++; + if (round < 15) { + setTimeout(spin, 20); + return; + } + + for (let i = 0; i < refs.length; i++) { + const survivor = refs[i].deref(); + expect(survivor).not.toBeUndefined(); + if (survivor === undefined) { + continue; + } + const entry = sideTable.get(survivor); + expect(entry).not.toBeUndefined(); + if (entry !== undefined) { + expect(entry.id).toBe(i); + expect(entry.payload.length).toBe(PAYLOAD_SIZE); + expect(entry.payload[PAYLOAD_SIZE - 1]).toBe("payload-" + i + "-" + (PAYLOAD_SIZE - 1)); + const deep = sideTable.get(entry.link); + expect(deep).not.toBeUndefined(); + if (deep !== undefined) { + expect(deep.deep).toBe(i); + expect(deep.payload.length).toBe(PAYLOAD_SIZE); + } + } + } + expect(replies).toBeGreaterThan(0); + + for (let i = 0; i < refs.length; i++) { + terminateWorker(refs[i]); + } + done(); + }); + } + spin(); + }); + + it("an unreferenced live Worker still answers messages", function (done) { + let reply = null; + const ref = (function () { + const worker = new Worker("./eventLoopEchoWorker.js"); + worker.onmessage = function (event) { reply = event.data; }; + worker.postMessage("hello"); + return new WeakRef(worker); + })(); + + pollGC(function () { return reply !== null; }, function () { + expect(reply).toBe("hello"); + expect(ref.deref()).not.toBeUndefined(); + terminateWorker(ref); + done(); + }); + }); + + it("a terminated Worker becomes collectable", function (done) { + const ref = (function () { + const worker = new Worker("./eventLoopEchoWorker.js"); + worker.postMessage("ping"); + return new WeakRef(worker); + })(); + + setTimeout(function () { + terminateWorker(ref); + setTimeout(function () { + pollGC(function () { return ref.deref() === undefined; }, function () { + expect(ref.deref()).toBeUndefined(); + done(); + }); + }, 100); + }, 150); + }); + + it("a Worker that closed itself becomes collectable", function (done) { + const ref = (function () { + const worker = new Worker("./workerLifetimeCloseWorker.js"); + worker.postMessage("close"); + return new WeakRef(worker); + })(); + + setTimeout(function () { + pollGC(function () { return ref.deref() === undefined; }, function () { + expect(ref.deref()).toBeUndefined(); + done(); + }); + }, 300); + }); +}); + +describe("node:worker_threads Worker exit", function () { + const wt = require("node:worker_threads"); + + it("emits 'exit' once when the worker closes itself", function (done) { + const worker = new wt.Worker("~/tests/workerLifetimeCloseWorker.js"); + const codes = []; + worker.on("exit", function (code) { codes.push(code); }); + worker.postMessage("go"); + + setTimeout(function () { + expect(codes).toEqual([0]); + done(); + }, 800); + }); + + it("emits 'exit' once on terminate(), after the thread ended, and resolves then", function (done) { + const worker = new wt.Worker("~/tests/eventLoopEchoWorker.js"); + const codes = []; + worker.on("exit", function (code) { codes.push(code); }); + + setTimeout(function () { + let resolved = null; + worker.terminate().then(function (code) { + resolved = code; + // 'exit' precedes the promise settling. + expect(codes).toEqual([0]); + }); + setTimeout(function () { + expect(resolved).toBe(0); + expect(codes).toEqual([0]); + worker.terminate().then(function (code) { + expect(code).toBe(0); + expect(codes).toEqual([0]); + done(); + }); + }, 800); + }, 150); + }); +}); + +describe("Worker teardown with a transferred port in flight", function () { + // The parent worker's loop still holds a message carrying a port whose + // sibling that worker owns; dropping it during shutdown posts the sibling's + // close sentinel back into the loop being shut down. + it("ends a terminated worker whose dropped message sentinels a port it owns", function (done) { + var worker = new Worker("./messaging/deadlockParent.js"); + var ended = false; + worker.addEventListener("nsworkerended", function () { ended = true; }); + worker.onerror = function (event) { + fail("worker error: " + event.message); + return true; + }; + worker.onmessage = function (event) { + expect(event.data).toBe("ready"); + worker.terminate(); + var deadline = Date.now() + 5000; + (function poll() { + if (ended || Date.now() > deadline) { + expect(ended).toBe(true); + done(); + return; + } + setTimeout(poll, 50); + })(); + }; + }); +}); diff --git a/TestRunner/app/tests/index.js b/TestRunner/app/tests/index.js index 7409edb2..31bfe10a 100644 --- a/TestRunner/app/tests/index.js +++ b/TestRunner/app/tests/index.js @@ -196,6 +196,9 @@ require("./NapiCoverageTests"); // Worker-isolate scoping of extended objc class names require("./ExtendedClassNamingTests"); +// Worker wrapper reachability across GC (strong while running, collectable after) +require("./WorkerLifetimeTests"); + // Tests common for all runtimes (git submodule of NativeScript/common-runtime-tests-app). require("../shared/index").runAllTests(); diff --git a/TestRunner/app/tests/messaging/deadlockChild.js b/TestRunner/app/tests/messaging/deadlockChild.js new file mode 100644 index 00000000..9f309884 --- /dev/null +++ b/TestRunner/app/tests/messaging/deadlockChild.js @@ -0,0 +1,5 @@ +onmessage = function (event) { + var port = event.data.port; + postMessage(port, [port]); + Atomics.store(event.data.flag, 0, 1); +}; diff --git a/TestRunner/app/tests/messaging/deadlockParent.js b/TestRunner/app/tests/messaging/deadlockParent.js new file mode 100644 index 00000000..c60c1d59 --- /dev/null +++ b/TestRunner/app/tests/messaging/deadlockParent.js @@ -0,0 +1,13 @@ +// Leaves a message that carries a port on this worker's own loop, undrained, +// at the moment the parent terminates it: the port's sibling is port1, owned +// by this worker. Spinning inside a timer callback keeps the loop from +// draining while still letting terminate() interrupt the JS. +var channel = new MessageChannel(); +var child = new Worker("./deadlockChild.js"); +var flag = new Int32Array(new SharedArrayBuffer(4)); +child.postMessage({ port: channel.port2, flag: flag }, [channel.port2]); +setTimeout(function () { + while (Atomics.load(flag, 0) === 0) {} + postMessage("ready"); + for (;;) {} +}, 0); diff --git a/TestRunner/app/tests/workerLifetimeCloseWorker.js b/TestRunner/app/tests/workerLifetimeCloseWorker.js new file mode 100644 index 00000000..ed90b183 --- /dev/null +++ b/TestRunner/app/tests/workerLifetimeCloseWorker.js @@ -0,0 +1,6 @@ +// Ends itself on request, so the parent can observe the end-of-worker path +// that does not go through terminate(). +onmessage = function () { + postMessage("closing"); + close(); +}; diff --git a/docs/knowledge/v8-resurrecting-finalizers.md b/docs/knowledge/v8-resurrecting-finalizers.md index 4d739471..c7326287 100644 --- a/docs/knowledge/v8-resurrecting-finalizers.md +++ b/docs/knowledge/v8-resurrecting-finalizers.md @@ -240,9 +240,12 @@ default configuration reaches none of it. 5. **Nested GC inside a finalizer callback.** Allocate heavily in the callback; confirm no double-invocation and no collection of the object under inspection. -The runtime's existing GC tests are the acceptance gate for the patch as the runtime uses it, -and they pass — in particular *"Worker instance should not be garbage collected if the worker -thread is alive"*, which exercises the `WorkerWrapper` resurrection site directly. +`TestRunner/app/tests/GCFinalizerTests.js` is the acceptance gate for the patch as the runtime +uses it. The Worker wrapper no longer depends on resurrection: a running worker's JS object is a +strong root until its thread ends (`WorkerWrapper::RootWorkerObject`), so the shared test +*"Worker instance should not be garbage collected if the worker thread is alive"* passes through +rooting and never reaches the resurrection branch — it must not be read as evidence that a +re-ported patch works. ObjectManager's refuse-and-re-weaken branch remains only as a fallback. ## Upgrade cost diff --git a/docs/worker-threads.md b/docs/worker-threads.md index 1d409c2d..eda7855a 100644 --- a/docs/worker-threads.md +++ b/docs/worker-threads.md @@ -54,7 +54,7 @@ means deliberately unsupported. | `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. 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. | | `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. | @@ -72,12 +72,16 @@ Values are cloned on the way in and deserialized fresh on each read, so mutating the object you passed does not reach a reader, and two readers never share one object. -### `exit` comes only from `terminate()` +### `exit` fires when the thread has ended, always with code `0` -The runtime has no thread-exit signal — nothing reports that a worker's isolate -finished. `terminate()` therefore resolves with `0` and emits `exit` with code -`0` on the way, and that is the only path that emits it. A worker that ends by -its own `close()` produces no `exit`. +`exit` is emitted once, from the runtime's end-of-worker notification, so every +`message` and `error` the worker produced before it ended has been delivered +first. Node reports the thread's exit code; this runtime has none to report, so +the code is `0` whichever way the worker ended — `terminate()`, its own +`close()`, an uncaught error, a missing entry or its heap limit. `terminate()` +resolves with `0` at the same moment `exit` fires. A parent that is itself +tearing down never delivers the notification, so a `terminate()` awaited from a +dying isolate stays pending, as it does in Node when the parent process exits. ### A worker error carries no `error` object, and the worker scope's `onerror` is not an event @@ -264,3 +268,39 @@ rather than raising a `DataCloneError`, which is long-standing behaviour app 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 lifetime + +**A `Worker` is held strongly by the runtime from the moment its thread starts +until that thread ends**, the way a browser keeps a running worker's handle +alive. Dropping every reference to one does not stop it: it keeps running, and +it keeps dispatching `message` and `error` events at the handlers installed on +it. + +```js +(function () { + const worker = new Worker("./worker.js"); + worker.onmessage = handle; // still fires; nothing here holds `worker` + worker.postMessage("go"); +})(); +``` + +Being a GC root also means a `Worker` is a well-behaved key: put one in a +`WeakMap`, `WeakSet` or `WeakRef` and the entry survives for as long as the +worker runs. + +The root is released when the worker ends — `terminate()`, or the worker's own +`close()`. From then on the object is collectable like any other, and the +runtime drops the native side with it. Nothing about a *finished* worker is +kept alive. + +### `nsworkerended` + +When the worker's thread has finished, the runtime dispatches a plain `Event` +named `nsworkerended` on the `Worker` object. It is **internal and +non-standard** — the web has no end-of-worker event, and the name is deliberately +outside the standard namespace. It exists so that `node:worker_threads` can +report `'exit'` for a worker that ended by its own `close()`; app code should +not rely on it. The event is best effort: a worker whose parent is already +tearing down never delivers it, because the parent's own teardown disposes the +worker anyway.