fix: keep the Worker object rooted until its thread ends, and report the end as nsworkerended

[edusperoni]

Stacked on #2043 (feat/worker-threads) — merge that first.

What this fixes

terminate() reset the Worker object's Persistent and dropped the registry entry synchronously, while the worker thread was still winding down. From that moment the wrapper was unreachable from native, the end of the worker was unobservable from JS, and any message the worker had already queued on the parent's loop was discarded when it arrived.

Android vs iOS lifetime — what was verified before porting

The iOS change is largely about replacing finalizer resurrection with reachability. Android never had that problem, so most of it does not apply:

• The Worker object is already a strong root, from construction. WorkerWrapper::poWorker_ is new Persistent<Object>(parentIsolate, workerObject) (WorkerWrapper.cpp) and is never made weak. There is no RootWorkerObject/UnrootWorkerObject to add.
• The Worker object is not an ObjectManager object. CallbackHandlers::NewThreadCallback takes args.This() of the plain FunctionTemplate installed in Runtime::PrepareV8Runtime (Runtime.cpp, the Worker constructor block) and hands it to the wrapper; nothing registers it with ObjectManager or marks it for GC. So none of iOS's DataWrapper/ObjectManager refuse-and-re-weaken work has an Android counterpart.
• The wrapper itself is shared_ptr-owned by the registry and by the detached thread (Start() captures shared_from_this() and BackgroundLooper holds it for the thread's whole life), so this cannot die under the thread. iOS's "read everything before publishing isDisposed_" hardening exists because a tearing-down iOS parent can delete the wrapper concurrently; that cannot happen here, and it is left alone.
• Worker-thread posts already reach the parent only through its event loop. parentTasks_ is a std::weak_ptr<EventLoop> captured on the parent's thread at construction, and PostMessageToParent, PassUncaughtExceptionFromWorkerToParent and the thread-exit post all go through it. The only reads of the parent isolate's runtime slot (Runtime::GetRuntime(parentIsolate)) happen inside lambdas that run on the parent's thread. Nothing was wrong; nothing changed.

The change

• CallbackHandlers::WorkerObjectTerminateCallback no longer calls WorkerWrapper::ClearWorkerOnParent(id). terminate() only starts the wind-down.
• WorkerWrapper::BackgroundLooper's final post to the parent's loop now runs a new WorkerWrapper::NotifyThreadEndedOnParent(workerId): on the parent's thread, with the parent isolate locked and entered, it resolves the wrapper by id, takes the Worker object out of poWorker_, calls WorkerEvents::EmitEnded under a TryCatch (a throwing listener is reported exactly as FireErrorOnParentWorkerObject/FireMessageOnParentWorkerObject report one — ContainUncaughtCallbackException then ReportFromEventLoopEntry), and only then calls ClearWorkerOnParent.
• WorkerEvents::EmitEnded mirrors EmitError's shape: a third emitEnded callout cached in WorkerEventsState at WorkerEvents::Init.
• js/worker-events.js gains emitEnded(), which dispatches a plain Event("nsworkerended") on the Worker object.
• js/node-worker-threads.js: the shim's Worker listens for nsworkerended and reports the exit from there. exit (code 0) is emitted exactly once, for a self-close() as much as for terminate(), and terminate() now resolves from the same notification instead of off a microtask, so nothing the worker sent can arrive after exit.
• A parent that is itself tearing down (TerminateChildren → ClearWorkerOnParent on the parent's thread) or whose loop is gone (expired parentTasks_) never delivers the notification. That is deliberate, matches iOS, and is documented as best effort.

Consumer audit for the removed clear. Nothing relied on the persistent being empty after terminate() for correctness: PostMessageToParent already returns early on isTerminating_; every error source is gated at the point of forwarding (CallWorkerScopeOnErrorHandle returns early on IsTerminating(), the unhandled-rejection path in NativeScriptException.cpp checks IsTerminating()/IsDisposed(), and BackgroundLooper's catch checks !isTerminating_), so no error from a terminated worker reaches the parent; the isTerminated private only guards a double terminate(); FireMessageOnParentWorkerObject/FireErrorOnParentWorkerObject keep their empty-persistent guards for the teardown paths that still clear early. The one observable change is the intended one: a message the worker queued before terminate() is now delivered instead of being dropped on arrival, and it is delivered before nsworkerended.

Tests

New Android-only specs in test-app/app/src/main/assets/app/tests/testWorkerLifetime.js (wired in mainpage.js), with workerLifetimeCloseWorker.js and messaging/deadlockChild.js / messaging/deadlockParent.js:

• a live Worker survives GC as a WeakMap key (the ephemeron repro, kept as a regression guard; collections are driven from a task via __collect({ execution: "async" }) so conservative stack scanning does not keep the workers alive)
• an unreferenced live Worker still answers messages
• a terminated Worker becomes collectable — observed after nsworkerended, since the root outlives terminate() by design
• a Worker that closed itself becomes collectable
• node:worker_threads: exit exactly once on self-close(); exit exactly once on terminate(), after the thread ended, with terminate() resolving after it and a second terminate() resolving immediately
• a terminated worker whose dropped message sentinels a port it owns still ends

Full device suite on a Pixel_3a_API_36 arm64 emulator: 1398 specs, 0 failures, 4 skipped (baseline on feat/worker-threads was 1391/0/4; the 7 new specs all ran and passed). npm run lint clean.

Deviations from iOS (ab72efc2)

1. Not ported: DataWrapper.h, ObjectManager.mm, RootWorkerObject/UnrootWorkerObject, EndWrapperLifetime, selfRef_. Android's persistent is strong from construction and the Worker object is not an ObjectManager object, so there is no weak handle to clear and no resurrection branch to take it off. The wrapper is kept alive by shared_ptrs, so iOS's atomic liveness token is unnecessary — the notification resolves the wrapper through the id-keyed registry instead.
2. Not ported: PostToRuntimeLoop → PostToLoop / mainLoop_. Android already captures parentTasks_ as a weak_ptr<EventLoop> on the parent's thread; the isolate-slot read iOS was removing does not exist here.
3. Not ported: the isDisposed_ publication reordering. The thread owns a shared_ptr to the wrapper, so nothing can delete it mid-teardown.
4. EmitEnded lives in WorkerEvents, not Worker. Android splits the worker-events callouts into WorkerEvents.{h,cpp}; that is where EmitMessage/EmitError already are.
5. docs/knowledge/v8-resurrecting-finalizers.md has no Android counterpart (docs/knowledge/ holds only v8-14-migration.md), so that hunk is skipped. The js/README.md listener-bag rule is reworded to the same effect without referring to a patch document this repo does not carry.
6. Docs say "from the moment it is constructed" where iOS says "from the moment its thread starts", because Android's persistent is strong from construction rather than from Start().
7. Tests are in Android style (var/function, jasmine done) and bump jasmine.DEFAULT_TIMEOUT_INTERVAL to 30s the way the other worker suites do; the fixed waits in the node:worker_threads specs are 2000 ms rather than iOS's 800 ms for emulator slowness. The two collectability specs additionally gate on nsworkerended before observing the WeakRef, which iOS approximates with a fixed delay.

Mirrors NativeScript/ios#456.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}