From b9f50630c85b2efeaa872a9b331b8ebc3841d882 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Thu, 12 Nov 2026 19:15:45 -0300 Subject: [PATCH 1/3] fix(runtime): release native objects on the runtime's thread JS can run on any thread that holds the isolate's Locker, for example a JS function dispatched to a background queue. A garbage collection there runs the wrappers' finalizers on that thread, and they released the native objects inline, so a view, a CA transaction or a block capturing one could be deallocated off the main thread and crash. Off the runtime's thread, ObjectManager now hands these releases ([object release] and Block_release, from finalizers and from __releaseNativeCounterpart) to the runtime's run loop. JS-side disposal is unchanged and still immediate. The hand-over is a CFRunLoop block holding a per-isolate queue, not an event loop task, so it still runs after the event loop stops taking work, and teardown drains inline. A new wrapper for an object whose release is still waiting takes that release over as its own retain instead of retaining again, so the retain-count thresholds that protect JS-extended instances stay right; if native code retained the object meanwhile, the new wrapper is protected on the spot. Collection adapters still release inline: their -dealloc retires the wrapper. A worker's runtime thread is the worker's own thread, so objects released by a worker isolate are still released there. --- NativeScript/runtime/ArgConverter.mm | 18 +- NativeScript/runtime/Caches.h | 16 ++ NativeScript/runtime/ObjectManager.h | 12 ++ NativeScript/runtime/ObjectManager.mm | 175 ++++++++++++++++++- TestRunner/app/tests/GCFinalizerTests.js | 34 ++++ docs/knowledge/v8-resurrecting-finalizers.md | 13 ++ 6 files changed, 258 insertions(+), 10 deletions(-) diff --git a/NativeScript/runtime/ArgConverter.mm b/NativeScript/runtime/ArgConverter.mm index d5464db8..11317385 100644 --- a/NativeScript/runtime/ArgConverter.mm +++ b/NativeScript/runtime/ArgConverter.mm @@ -638,9 +638,17 @@ tns::SetValue(isolate, thiz, wrapper); std::shared_ptr> poThiz = ObjectManager::Register(context, thiz); cache->Instances.emplace(result, poThiz); - if (!resultIsOwned) { + // An initializer can answer with an existing object whose release is still + // waiting (see CreateJsWrapper). + bool tookOver = ObjectManager::TakeDeferredRelease(isolate, result); + if (tookOver && resultIsOwned) { + [result release]; + } else if (!tookOver && !resultIsOwned) { [result retain]; } + if (tookOver) { + ObjectManager::ProtectIfNativelyRetained(result, wrapper); + } ExternalMemory::ChargeEstimatedSize(isolate, thiz); } } @@ -908,7 +916,13 @@ receiver = poValue->Get(isolate).As(); tns::SetValue(isolate, receiver, wrapper); cache->Instances.emplace(target, poValue); - [target retain]; + // An extended class reads its retain count to decide whether native + // code owns the object, so a pending release is taken over, not stacked. + if (!ObjectManager::TakeDeferredRelease(isolate, target)) { + [target retain]; + } else if (wrapper->Type() == WrapperType::ObjCObject) { + ObjectManager::ProtectIfNativelyRetained(target, static_cast(wrapper)); + } } } else { tns::SetValue(isolate, receiver, wrapper); diff --git a/NativeScript/runtime/Caches.h b/NativeScript/runtime/Caches.h index 666d7e63..2b5b3586 100644 --- a/NativeScript/runtime/Caches.h +++ b/NativeScript/runtime/Caches.h @@ -110,6 +110,22 @@ class Caches { return cache->GetOrCreateState(); } + // Teardown only: the existing state, even after InvalidateIsolate, for a + // subsystem that must finish work its state still holds. Never creates. + template + static T* ExistingStateFor(v8::Isolate* isolate) { + auto raw = isolate->GetData(0); + if (raw == nullptr) { + return nullptr; + } + Caches* cache = reinterpret_cast*>(raw)->get(); + size_t index = StateSlotIndexFor(); + if (index >= cache->stateSlots_.size()) { + return nullptr; + } + return static_cast(cache->stateSlots_[index].get()); + } + inline int getIsolateId() { return isolateId_; } // The id assigned at Init; unlike getIsolateId() it survives diff --git a/NativeScript/runtime/ObjectManager.h b/NativeScript/runtime/ObjectManager.h index 67691ff3..5e46627e 100644 --- a/NativeScript/runtime/ObjectManager.h +++ b/NativeScript/runtime/ObjectManager.h @@ -6,6 +6,7 @@ namespace tns { class ObjectManager; +class ObjCDataWrapper; // Parameter of the kFinalizer weak callback armed on target_. // @@ -54,6 +55,17 @@ class ObjectManager { // old Isolate::VisitHandlesWithClassIds walk, which V8 removed. static void DisposeAllRegistered(v8::Isolate* isolate); + // A collection can run on any thread that holds the isolate's Locker, but + // the native releases it causes are handed to the runtime's thread. A new + // Instances entry for an object whose release is still waiting takes that + // release over as its own retain instead of retaining again; returns whether + // it did. Once the entry's retain is settled, call ProtectIfNativelyRetained. + static bool TakeDeferredRelease(v8::Isolate* isolate, id obj); + // An extended class protects its wrapper when native code first retains the + // object past the runtime's own retain. A native retain made while a release + // waited found no wrapper to protect; this catches up. + static void ProtectIfNativelyRetained(id obj, ObjCDataWrapper* wrapper); + private: static void ReleaseNativeCounterpartCallback( const v8::FunctionCallbackInfo& info); diff --git a/NativeScript/runtime/ObjectManager.mm b/NativeScript/runtime/ObjectManager.mm index 904ab233..47e0bedd 100644 --- a/NativeScript/runtime/ObjectManager.mm +++ b/NativeScript/runtime/ObjectManager.mm @@ -1,12 +1,18 @@ #include "ObjectManager.h" #include #include +#include +#include #include +#include +#include #include "Caches.h" #include "Constants.h" #include "DataWrapper.h" #include "FFICall.h" #include "Helpers.h" +#include "Runtime.h" +#include "TNSDerivedClass.h" using namespace v8; using namespace std; @@ -15,11 +21,6 @@ static Class NSTimerClass = objc_getClass("NSTimer"); -void ObjectManager::Init(Isolate* isolate, Local globalTemplate) { - globalTemplate->Set(tns::ToV8String(isolate, "__releaseNativeCounterpart"), - FunctionTemplate::New(isolate, ReleaseNativeCounterpartCallback)); -} - namespace { void LinkRegistered(v8::Isolate* isolate, ObjectWeakCallbackState* state) { @@ -52,8 +53,124 @@ void UnlinkRegistered(ObjectWeakCallbackState* state) { state->next_ = nullptr; } +// Native releases belong on the runtime's thread. JS, and with it a collection +// and its finalizers, can run on any thread that holds the isolate's Locker (a +// JS callback dispatched to a background queue), and a -dealloc that tears down +// a view, a CA transaction or a block capturing one crashes off that thread. +// +// Shared with the run loop block that drains it, which may run after the +// isolate's Caches is gone; the releases themselves need no Locker. +struct DeferredReleases { + std::mutex mutex; + // Object -> releases it is owed. An object stays alive while it is owed one, + // so its address cannot be reused by another object meanwhile. + std::unordered_map objects; + std::vector blocks; + bool scheduled = false; +}; + +struct DeferredReleasesState { + std::shared_ptr pending = std::make_shared(); +}; + +// Object releases waiting in any isolate: lets wrapper creation skip the lookup. +// Process-wide, so while one isolate has some, the others pay a lookup too. +std::atomic deferredObjectReleases_{0}; + +void DrainDeferredReleases(const std::shared_ptr& pending) { + std::unordered_map objects; + std::vector blocks; + { + std::lock_guard lock(pending->mutex); + objects.swap(pending->objects); + blocks.swap(pending->blocks); + pending->scheduled = false; + int count = 0; + for (const auto& entry : objects) { + count += entry.second; + } + deferredObjectReleases_.fetch_sub(count, std::memory_order_relaxed); + } + for (const auto& entry : objects) { + id obj = (__bridge id)entry.first; + for (unsigned i = 0; i < entry.second; i++) { + [obj release]; + } + } + for (const void* block : blocks) { + Block_release(block); + } +} + +// Null when the caller is on the runtime's thread (or there is no runtime to +// hand over to) and should release inline. +std::shared_ptr DeferralTarget(Isolate* isolate, CFRunLoopRef* loop) { + Runtime* runtime = Runtime::GetRuntime(isolate); + if (runtime == nullptr || runtime->RuntimeLoop() == nullptr || + CFRunLoopGetCurrent() == runtime->RuntimeLoop()) { + return nullptr; + } + DeferredReleasesState* state = Caches::StateFor(isolate); + if (state == nullptr) { + return nullptr; + } + *loop = runtime->RuntimeLoop(); + return state->pending; +} + +// Called with pending->mutex held. +void ScheduleDrain(const std::shared_ptr& pending, CFRunLoopRef loop) { + if (pending->scheduled) { + return; + } + pending->scheduled = true; + std::shared_ptr shared = pending; + // A plain run loop block rather than an event loop task: it must run even + // once the event loop stops taking work, and needs no isolate. The loop is + // retained because a worker's thread can exit, freeing it, meanwhile. + CFRetain(loop); + CFRunLoopPerformBlock(loop, kCFRunLoopCommonModes, ^{ + DrainDeferredReleases(shared); + }); + CFRunLoopWakeUp(loop); + CFRelease(loop); +} + +void ReleaseOnRuntimeThread(Isolate* isolate, id obj) { + CFRunLoopRef loop = nullptr; + std::shared_ptr pending = DeferralTarget(isolate, &loop); + if (pending == nullptr) { + [obj release]; + return; + } + std::lock_guard lock(pending->mutex); + pending->objects[(__bridge void*)obj]++; + deferredObjectReleases_.fetch_add(1, std::memory_order_relaxed); + ScheduleDrain(pending, loop); +} + +void BlockReleaseOnRuntimeThread(Isolate* isolate, const void* block) { + CFRunLoopRef loop = nullptr; + std::shared_ptr pending = DeferralTarget(isolate, &loop); + if (pending == nullptr) { + Block_release(block); + return; + } + std::lock_guard lock(pending->mutex); + pending->blocks.push_back(block); + ScheduleDrain(pending, loop); +} + } // namespace +void ObjectManager::Init(Isolate* isolate, Local globalTemplate) { + globalTemplate->Set(tns::ToV8String(isolate, "__releaseNativeCounterpart"), + FunctionTemplate::New(isolate, ReleaseNativeCounterpartCallback)); + // Created here, on the runtime's thread: a collection on another thread must + // only find it. + Caches::StateFor(isolate); +} + std::shared_ptr> ObjectManager::Register(Local context, const Local obj) { Isolate* isolate = v8::Isolate::GetCurrent(); @@ -104,6 +221,11 @@ void DisposeHandle(v8::Isolate* isolate, Isolate::Scope isolateScope(isolate); HandleScope scope(isolate); + // A worker's run loop stops with its runtime, so nothing else would drain. + if (DeferredReleasesState* deferred = Caches::ExistingStateFor(isolate)) { + DrainDeferredReleases(deferred->pending); + } + // Detach the whole list first so disposal can't walk into freed entries, and // claim every state up front: disposing one entry can re-enter JS from // -dealloc and reach __releaseNativeCounterpart for any other entry this @@ -175,6 +297,34 @@ void DisposeHandle(v8::Isolate* isolate, state->target_->SetWeak(state, FinalizerCallback, WeakCallbackType::kFinalizer); } +bool ObjectManager::TakeDeferredRelease(Isolate* isolate, id obj) { + if (deferredObjectReleases_.load(std::memory_order_relaxed) == 0) { + return false; + } + DeferredReleasesState* state = Caches::StateFor(isolate); + if (state == nullptr) { + return false; + } + DeferredReleases* pending = state->pending.get(); + std::lock_guard lock(pending->mutex); + auto it = pending->objects.find((__bridge void*)obj); + if (it == pending->objects.end()) { + return false; + } + if (--it->second == 0) { + pending->objects.erase(it); + } + deferredObjectReleases_.fetch_sub(1, std::memory_order_relaxed); + return true; +} + +void ObjectManager::ProtectIfNativelyRetained(id obj, ObjCDataWrapper* wrapper) { + if (class_conformsToProtocol(object_getClass(obj), @protocol(TNSDerivedClass)) && + [obj retainCount] > 1) { + wrapper->GcProtect(); + } +} + bool ObjectManager::DisposeValue(Isolate* isolate, Local value, bool isFinalDisposal) { if (value.IsEmpty() || value->IsNullOrUndefined() || !value->IsObject()) { return true; @@ -219,7 +369,12 @@ void DisposeHandle(v8::Isolate* isolate, cache->Instances.erase(it); } } - [target release]; + if (objCObjectWrapper->IsAdapterClaim()) { + // The adapter's -dealloc retires this wrapper; it must run first. + [target release]; + } else { + ReleaseOnRuntimeThread(isolate, target); + } } break; } @@ -237,7 +392,7 @@ void DisposeHandle(v8::Isolate* isolate, // Block_copy and runs the block's dispose helper once we drop the last // reference. (Using CFRelease here over-released stack blocks that were // never promoted to the heap, crashing in objc_release during GC.) - Block_release(blockWrapper->Block()); + BlockReleaseOnRuntimeThread(isolate, blockWrapper->Block()); break; } case WrapperType::Reference: { @@ -386,7 +541,11 @@ void DisposeHandle(v8::Isolate* isolate, // wrapper, so it must not be touched afterwards. bool adapterClaim = objcWrapper->IsAdapterClaim(); - [data release]; + if (adapterClaim) { + [data release]; + } else { + ReleaseOnRuntimeThread(isolate, data); + } // The release above can run a -dealloc that detaches this wrapper; the // internal field owns it, so free only what is still attached. An diff --git a/TestRunner/app/tests/GCFinalizerTests.js b/TestRunner/app/tests/GCFinalizerTests.js index 4d77e7a5..192a49aa 100644 --- a/TestRunner/app/tests/GCFinalizerTests.js +++ b/TestRunner/app/tests/GCFinalizerTests.js @@ -35,6 +35,40 @@ describe("GC finalizer callbacks", function () { expect(calls).toBe(1); }); + // A JS callback dispatched to a background queue holds the isolate's + // Locker there, so a collection and its finalizers can run off the main + // thread. The native releases they make must still happen on it. + it("releases natively on the runtime's thread after a background collection", function (done) { + var threads = []; + var DeallocThread = TNSApi.extend({ + methodCalledInDealloc: function () { + threads.push(NSThread.isMainThread); + } + }, { name: "TNSApiDeallocThread" }); + + // Owned only through its wrapper, so its finalizer releases it. + (function () { + DeallocThread.alloc().init(); + })(); + + var queue = dispatch_get_global_queue(qos_class_t.QOS_CLASS_DEFAULT, 0); + dispatch_async(queue, function () { + scrubStack(); + __collect(); + __collect(); + }); + + var attempts = 200; + (function poll() { + if (threads.length > 0 || --attempts === 0) { + expect(threads).toEqual([true]); + done(); + return; + } + setTimeout(poll, 10); + })(); + }); + // A GcProtected (natively retained) instance is resurrected by its // finalizer, but anything reachable only through it is queued in the same // GC and disposed — the resurrected parent then holds "husks" whose diff --git a/docs/knowledge/v8-resurrecting-finalizers.md b/docs/knowledge/v8-resurrecting-finalizers.md index ece736d6..58f33742 100644 --- a/docs/knowledge/v8-resurrecting-finalizers.md +++ b/docs/knowledge/v8-resurrecting-finalizers.md @@ -129,6 +129,19 @@ global handles via `IterateStrongRoots` (`SkipRoot::kWeak`), so pending nodes ar during a nested GC; root *updating* uses `IterateAllRoots`, a single pass over `NORMAL||WEAK`, so a pending node's slot is updated exactly once after evacuation. +## Which thread releases the native object + +Finalizers run on whichever thread triggered the collection, and that can be any thread holding +the isolate's Locker — a JS function dispatched to a background queue included. Off the +runtime's thread, `ObjectManager` hands the native release (`[object release]`, +`Block_release`) to the runtime's run loop instead of making it inline; the JS side is disposed +immediately either way. The same applies to `__releaseNativeCounterpart` called from such a +thread: the wrapper is retired at once, the object a run loop turn later. A new wrapper built +for an object whose release is still waiting takes that release over (keeping the retain counts +the extended classes' swizzle reads exact). Collection adapters are exempt, since their +`-dealloc` retires the wrapper, and teardown drains the queue inline. For a worker, the +runtime's thread is the worker's own thread, not the main thread. + ## Deliberate deviations from upstream's original - **No `PENDING` state, no `NEAR_DEATH` for finalizers.** 14.9's `NodeState` is 2 bits with all From b0d95ef90c4716fa79bfbd8de37d041e09aeae3e Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Thu, 12 Nov 2026 21:22:25 -0300 Subject: [PATCH 2/3] fix(runtime): defer only releases of objects born on the runtime's thread Deferring every off-thread release moved all of a background queue's deallocations onto the main thread, including objects that never belonged there. Wrappers now record whether the object first reached JS on its runtime's thread (a thread-local marker the runtime sets where it binds its event loop, compared with the current isolate when the wrapper is built), and only those releases are handed over. Objects first wrapped on another thread are released where the collection runs, as before. The queue is now plain per-isolate containers: finalizers, __releaseNativeCounterpart, wrapper creation, the drain and teardown all hold the isolate's Locker, so it needs no mutex or atomics. It drains in one event loop task per batch instead of a CFRunLoop block; teardown, which runs after the loop stops, drains it inline before and after the disposal walk. --- NativeScript/runtime/ArgConverter.mm | 7 +- NativeScript/runtime/DataWrapper.h | 27 +++- NativeScript/runtime/ObjectManager.mm | 139 ++++++++----------- NativeScript/runtime/Runtime.mm | 8 ++ TestRunner/app/tests/GCFinalizerTests.js | 31 +++++ docs/knowledge/v8-resurrecting-finalizers.md | 23 +-- 6 files changed, 140 insertions(+), 95 deletions(-) diff --git a/NativeScript/runtime/ArgConverter.mm b/NativeScript/runtime/ArgConverter.mm index 11317385..1875c941 100644 --- a/NativeScript/runtime/ArgConverter.mm +++ b/NativeScript/runtime/ArgConverter.mm @@ -647,6 +647,9 @@ [result retain]; } if (tookOver) { + // Only releases of objects that first reached JS on the runtime's + // thread are ever queued. + wrapper->SetBornOnRuntimeThread(); ObjectManager::ProtectIfNativelyRetained(result, wrapper); } ExternalMemory::ChargeEstimatedSize(isolate, thiz); @@ -921,7 +924,9 @@ if (!ObjectManager::TakeDeferredRelease(isolate, target)) { [target retain]; } else if (wrapper->Type() == WrapperType::ObjCObject) { - ObjectManager::ProtectIfNativelyRetained(target, static_cast(wrapper)); + ObjCDataWrapper* objcWrapper = static_cast(wrapper); + objcWrapper->SetBornOnRuntimeThread(); + ObjectManager::ProtectIfNativelyRetained(target, objcWrapper); } } } else { diff --git a/NativeScript/runtime/DataWrapper.h b/NativeScript/runtime/DataWrapper.h index a98655a5..d6be21a0 100644 --- a/NativeScript/runtime/DataWrapper.h +++ b/NativeScript/runtime/DataWrapper.h @@ -20,6 +20,15 @@ class WorkerInspectorClient; namespace tns { +// The isolate whose runtime runs on this thread (its event loop's thread), +// while the runtime is bound here. JS can also run on other threads, any that +// take the isolate's Locker. +inline thread_local v8::Isolate* runtimeThreadIsolate = nullptr; + +inline bool OnRuntimeThreadOf(v8::Isolate* isolate) { + return isolate != nullptr && runtimeThreadIsolate == isolate; +} + class PrimitiveDataWrapper; struct ObjectWeakCallbackState; class EventLoop; @@ -396,7 +405,8 @@ class ObjCDataWrapper : public BaseDataWrapper { ObjCDataWrapper(id data, const TypeEncoding* typeEncoding = nullptr) : data_(data), typeEncoding_(typeEncoding), - klass_(object_getClass(data)) {} + klass_(object_getClass(data)), + bornOnRuntimeThread_(OnRuntimeThreadOf(v8::Isolate::TryGetCurrent())) {} const WrapperType Type() { return WrapperType::ObjCObject; } @@ -409,6 +419,11 @@ class ObjCDataWrapper : public BaseDataWrapper { bool IsAdapterClaim() { return this->adapterClaim_; } void MarkAdapterClaim() { this->adapterClaim_ = true; } + // Whether the object first reached JS on its runtime's thread, the stand-in + // for "belongs to that thread": its native release must happen there too. + bool BornOnRuntimeThread() { return this->bornOnRuntimeThread_; } + void SetBornOnRuntimeThread() { this->bornOnRuntimeThread_ = true; } + const TypeEncoding* TypeEncoding() { return this->typeEncoding_; } // The class Data() had when this wrapper was built. Data() alone cannot tell @@ -421,6 +436,7 @@ class ObjCDataWrapper : public BaseDataWrapper { id data_; const tns::TypeEncoding* typeEncoding_; Class klass_; + bool bornOnRuntimeThread_; }; class ObjCClassWrapper : public BaseDataWrapper { @@ -488,7 +504,10 @@ class AnonymousFunctionWrapper : public BaseDataWrapper { class BlockWrapper : public BaseDataWrapper { public: BlockWrapper(void* block, const TypeEncoding* typeEncoding, bool ownsBlock) - : block_(block), typeEncoding_(typeEncoding), ownsBlock_(ownsBlock) {} + : block_(block), + typeEncoding_(typeEncoding), + ownsBlock_(ownsBlock), + bornOnRuntimeThread_(OnRuntimeThreadOf(v8::Isolate::TryGetCurrent())) {} const WrapperType Type() { return WrapperType::Block; } @@ -498,10 +517,14 @@ class BlockWrapper : public BaseDataWrapper { bool OwnsBlock() { return this->ownsBlock_; } + // See ObjCDataWrapper::BornOnRuntimeThread. + bool BornOnRuntimeThread() { return this->bornOnRuntimeThread_; } + private: void* block_; const TypeEncoding* typeEncoding_; bool ownsBlock_; + bool bornOnRuntimeThread_; }; class FunctionReferenceTypeWrapper : public BaseDataWrapper { diff --git a/NativeScript/runtime/ObjectManager.mm b/NativeScript/runtime/ObjectManager.mm index 47e0bedd..6d98e6cd 100644 --- a/NativeScript/runtime/ObjectManager.mm +++ b/NativeScript/runtime/ObjectManager.mm @@ -1,8 +1,6 @@ #include "ObjectManager.h" #include #include -#include -#include #include #include #include @@ -10,6 +8,7 @@ #include "Constants.h" #include "DataWrapper.h" #include "FFICall.h" +#include "EventLoop.h" #include "Helpers.h" #include "Runtime.h" #include "TNSDerivedClass.h" @@ -53,44 +52,31 @@ void UnlinkRegistered(ObjectWeakCallbackState* state) { state->next_ = nullptr; } -// Native releases belong on the runtime's thread. JS, and with it a collection -// and its finalizers, can run on any thread that holds the isolate's Locker (a -// JS callback dispatched to a background queue), and a -dealloc that tears down -// a view, a CA transaction or a block capturing one crashes off that thread. +// The native release of an object that first reached JS on its runtime's +// thread belongs on that thread. JS, and with it a collection and its +// finalizers, can run on any thread that holds the isolate's Locker (a JS +// callback dispatched to a background queue), and a -dealloc that tears down a +// view, a CA transaction or a block capturing one crashes off the main thread. +// Objects first wrapped on another thread are still released where the +// collection runs. // -// Shared with the run loop block that drains it, which may run after the -// isolate's Caches is gone; the releases themselves need no Locker. +// Every access holds the isolate's Locker: finalizers and +// __releaseNativeCounterpart on any thread, and on the runtime's thread the +// drain task, wrapper creation and teardown. So no other synchronization. struct DeferredReleases { - std::mutex mutex; // Object -> releases it is owed. An object stays alive while it is owed one, // so its address cannot be reused by another object meanwhile. std::unordered_map objects; std::vector blocks; - bool scheduled = false; + bool drainPosted = false; }; -struct DeferredReleasesState { - std::shared_ptr pending = std::make_shared(); -}; - -// Object releases waiting in any isolate: lets wrapper creation skip the lookup. -// Process-wide, so while one isolate has some, the others pay a lookup too. -std::atomic deferredObjectReleases_{0}; - -void DrainDeferredReleases(const std::shared_ptr& pending) { +void DrainDeferredReleases(DeferredReleases* pending) { std::unordered_map objects; std::vector blocks; - { - std::lock_guard lock(pending->mutex); - objects.swap(pending->objects); - blocks.swap(pending->blocks); - pending->scheduled = false; - int count = 0; - for (const auto& entry : objects) { - count += entry.second; - } - deferredObjectReleases_.fetch_sub(count, std::memory_order_relaxed); - } + objects.swap(pending->objects); + blocks.swap(pending->blocks); + pending->drainPosted = false; for (const auto& entry : objects) { id obj = (__bridge id)entry.first; for (unsigned i = 0; i < entry.second; i++) { @@ -102,63 +88,50 @@ void DrainDeferredReleases(const std::shared_ptr& pending) { } } -// Null when the caller is on the runtime's thread (or there is no runtime to -// hand over to) and should release inline. -std::shared_ptr DeferralTarget(Isolate* isolate, CFRunLoopRef* loop) { - Runtime* runtime = Runtime::GetRuntime(isolate); - if (runtime == nullptr || runtime->RuntimeLoop() == nullptr || - CFRunLoopGetCurrent() == runtime->RuntimeLoop()) { - return nullptr; - } - DeferredReleasesState* state = Caches::StateFor(isolate); - if (state == nullptr) { +// The queue to hand a release to, or null when the caller releases inline. +// Created by Init, so a collection on another thread never creates it; read +// without the validity check because a collection can still run on another +// thread between InvalidateIsolate and teardown, which drains it. +DeferredReleases* DeferralQueue(Isolate* isolate, bool bornOnRuntimeThread) { + if (!bornOnRuntimeThread || OnRuntimeThreadOf(isolate)) { return nullptr; } - *loop = runtime->RuntimeLoop(); - return state->pending; + return Caches::ExistingStateFor(isolate); } -// Called with pending->mutex held. -void ScheduleDrain(const std::shared_ptr& pending, CFRunLoopRef loop) { - if (pending->scheduled) { +// One event loop task per batch. If the loop no longer takes work, teardown is +// under way and drains the queue itself. +void PostDrain(Isolate* isolate, DeferredReleases* pending) { + if (pending->drainPosted) { return; } - pending->scheduled = true; - std::shared_ptr shared = pending; - // A plain run loop block rather than an event loop task: it must run even - // once the event loop stops taking work, and needs no isolate. The loop is - // retained because a worker's thread can exit, freeing it, meanwhile. - CFRetain(loop); - CFRunLoopPerformBlock(loop, kCFRunLoopCommonModes, ^{ - DrainDeferredReleases(shared); + Runtime* runtime = Runtime::GetRuntime(isolate); + std::shared_ptr loop = runtime != nullptr ? runtime->GetEventLoop() : nullptr; + pending->drainPosted = loop != nullptr && loop->PostInternal([isolate]() { + if (DeferredReleases* queued = Caches::ExistingStateFor(isolate)) { + DrainDeferredReleases(queued); + } }); - CFRunLoopWakeUp(loop); - CFRelease(loop); } -void ReleaseOnRuntimeThread(Isolate* isolate, id obj) { - CFRunLoopRef loop = nullptr; - std::shared_ptr pending = DeferralTarget(isolate, &loop); +void ReleaseOnRuntimeThread(Isolate* isolate, id obj, bool bornOnRuntimeThread) { + DeferredReleases* pending = DeferralQueue(isolate, bornOnRuntimeThread); if (pending == nullptr) { [obj release]; return; } - std::lock_guard lock(pending->mutex); pending->objects[(__bridge void*)obj]++; - deferredObjectReleases_.fetch_add(1, std::memory_order_relaxed); - ScheduleDrain(pending, loop); + PostDrain(isolate, pending); } -void BlockReleaseOnRuntimeThread(Isolate* isolate, const void* block) { - CFRunLoopRef loop = nullptr; - std::shared_ptr pending = DeferralTarget(isolate, &loop); +void BlockReleaseOnRuntimeThread(Isolate* isolate, const void* block, bool bornOnRuntimeThread) { + DeferredReleases* pending = DeferralQueue(isolate, bornOnRuntimeThread); if (pending == nullptr) { Block_release(block); return; } - std::lock_guard lock(pending->mutex); pending->blocks.push_back(block); - ScheduleDrain(pending, loop); + PostDrain(isolate, pending); } } // namespace @@ -166,9 +139,7 @@ void BlockReleaseOnRuntimeThread(Isolate* isolate, const void* block) { void ObjectManager::Init(Isolate* isolate, Local globalTemplate) { globalTemplate->Set(tns::ToV8String(isolate, "__releaseNativeCounterpart"), FunctionTemplate::New(isolate, ReleaseNativeCounterpartCallback)); - // Created here, on the runtime's thread: a collection on another thread must - // only find it. - Caches::StateFor(isolate); + Caches::StateFor(isolate); } std::shared_ptr> ObjectManager::Register(Local context, @@ -221,9 +192,10 @@ void DisposeHandle(v8::Isolate* isolate, Isolate::Scope isolateScope(isolate); HandleScope scope(isolate); - // A worker's run loop stops with its runtime, so nothing else would drain. - if (DeferredReleasesState* deferred = Caches::ExistingStateFor(isolate)) { - DrainDeferredReleases(deferred->pending); + // The event loop has stopped taking work, so nothing else would drain. + DeferredReleases* deferred = Caches::ExistingStateFor(isolate); + if (deferred != nullptr) { + DrainDeferredReleases(deferred); } // Detach the whole list first so disposal can't walk into freed entries, and @@ -252,6 +224,12 @@ void DisposeHandle(v8::Isolate* isolate, state = next; } + // The walk's own releases are inline on the runtime's thread; this catches + // any made from another. + if (deferred != nullptr) { + DrainDeferredReleases(deferred); + } + DisposeHandleMap(isolate, cache->CtorFuncs); DisposeHandleMap(isolate, cache->ProtocolCtorFuncs); DisposeHandleMap(isolate, cache->CFunctions); @@ -298,15 +276,10 @@ void DisposeHandle(v8::Isolate* isolate, } bool ObjectManager::TakeDeferredRelease(Isolate* isolate, id obj) { - if (deferredObjectReleases_.load(std::memory_order_relaxed) == 0) { - return false; - } - DeferredReleasesState* state = Caches::StateFor(isolate); - if (state == nullptr) { + DeferredReleases* pending = Caches::ExistingStateFor(isolate); + if (pending == nullptr || pending->objects.empty()) { return false; } - DeferredReleases* pending = state->pending.get(); - std::lock_guard lock(pending->mutex); auto it = pending->objects.find((__bridge void*)obj); if (it == pending->objects.end()) { return false; @@ -314,7 +287,6 @@ void DisposeHandle(v8::Isolate* isolate, if (--it->second == 0) { pending->objects.erase(it); } - deferredObjectReleases_.fetch_sub(1, std::memory_order_relaxed); return true; } @@ -373,7 +345,7 @@ void DisposeHandle(v8::Isolate* isolate, // The adapter's -dealloc retires this wrapper; it must run first. [target release]; } else { - ReleaseOnRuntimeThread(isolate, target); + ReleaseOnRuntimeThread(isolate, target, objCObjectWrapper->BornOnRuntimeThread()); } } break; @@ -392,7 +364,8 @@ void DisposeHandle(v8::Isolate* isolate, // Block_copy and runs the block's dispose helper once we drop the last // reference. (Using CFRelease here over-released stack blocks that were // never promoted to the heap, crashing in objc_release during GC.) - BlockReleaseOnRuntimeThread(isolate, blockWrapper->Block()); + BlockReleaseOnRuntimeThread(isolate, blockWrapper->Block(), + blockWrapper->BornOnRuntimeThread()); break; } case WrapperType::Reference: { @@ -544,7 +517,7 @@ void DisposeHandle(v8::Isolate* isolate, if (adapterClaim) { [data release]; } else { - ReleaseOnRuntimeThread(isolate, data); + ReleaseOnRuntimeThread(isolate, data, objcWrapper->BornOnRuntimeThread()); } // The release above can run a -dealloc that detaches this wrapper; the diff --git a/NativeScript/runtime/Runtime.mm b/NativeScript/runtime/Runtime.mm index ebe46f48..2559d2bf 100644 --- a/NativeScript/runtime/Runtime.mm +++ b/NativeScript/runtime/Runtime.mm @@ -1,4 +1,5 @@ #include "Runtime.h" +#include "DataWrapper.h" #include #include #include "ArgConverter.h" @@ -277,12 +278,16 @@ void DisposeIsolateWhenPossible(Isolate* isolate, int gateId) { eventLoop_->BindToCurrentThread(); isolate->SetData(Constants::RUNTIME_SLOT, this); isolate_ = isolate; + tns::runtimeThreadIsolate = isolate; } Runtime::~Runtime() { if (!initialized_) { // Never published: no other thread can know this isolate, and it holds no // caches, gate or wrappers. + if (tns::runtimeThreadIsolate == isolate_) { + tns::runtimeThreadIsolate = nullptr; + } isolate_->Dispose(); NativeScriptPlatform::Instance()->IsolateDisposed(isolate_, eventLoop_); currentRuntime_ = nullptr; @@ -368,6 +373,9 @@ void DisposeIsolateWhenPossible(Isolate* isolate, int gateId) { Caches::Remove(this->isolate_); this->isolate_->SetData(Constants::RUNTIME_SLOT, nullptr); + if (tns::runtimeThreadIsolate == this->isolate_) { + tns::runtimeThreadIsolate = nullptr; + } // Last, and under the Locker: from here on nothing reads a wrapper through // a JS object, so threads that never pinned may free theirs without the diff --git a/TestRunner/app/tests/GCFinalizerTests.js b/TestRunner/app/tests/GCFinalizerTests.js index 192a49aa..a40a61ad 100644 --- a/TestRunner/app/tests/GCFinalizerTests.js +++ b/TestRunner/app/tests/GCFinalizerTests.js @@ -69,6 +69,37 @@ describe("GC finalizer callbacks", function () { })(); }); + // An object that first reached JS on a background thread has no tie to the + // runtime's thread, so its release stays with the collection that ran there. + it("releases a background-born object where the background collection runs", function (done) { + var threads = []; + var DeallocThread = TNSApi.extend({ + methodCalledInDealloc: function () { + threads.push(NSThread.isMainThread); + } + }, { name: "TNSApiDeallocBackgroundThread" }); + + var queue = dispatch_get_global_queue(qos_class_t.QOS_CLASS_DEFAULT, 0); + dispatch_async(queue, function () { + (function () { + DeallocThread.alloc().init(); + })(); + scrubStack(); + __collect(); + __collect(); + }); + + var attempts = 200; + (function poll() { + if (threads.length > 0 || --attempts === 0) { + expect(threads).toEqual([false]); + done(); + return; + } + setTimeout(poll, 10); + })(); + }); + // A GcProtected (natively retained) instance is resurrected by its // finalizer, but anything reachable only through it is queued in the same // GC and disposed — the resurrected parent then holds "husks" whose diff --git a/docs/knowledge/v8-resurrecting-finalizers.md b/docs/knowledge/v8-resurrecting-finalizers.md index 58f33742..233749e9 100644 --- a/docs/knowledge/v8-resurrecting-finalizers.md +++ b/docs/knowledge/v8-resurrecting-finalizers.md @@ -132,15 +132,20 @@ so a pending node's slot is updated exactly once after evacuation. ## Which thread releases the native object Finalizers run on whichever thread triggered the collection, and that can be any thread holding -the isolate's Locker — a JS function dispatched to a background queue included. Off the -runtime's thread, `ObjectManager` hands the native release (`[object release]`, -`Block_release`) to the runtime's run loop instead of making it inline; the JS side is disposed -immediately either way. The same applies to `__releaseNativeCounterpart` called from such a -thread: the wrapper is retired at once, the object a run loop turn later. A new wrapper built -for an object whose release is still waiting takes that release over (keeping the retain counts -the extended classes' swizzle reads exact). Collection adapters are exempt, since their -`-dealloc` retires the wrapper, and teardown drains the queue inline. For a worker, the -runtime's thread is the worker's own thread, not the main thread. +the isolate's Locker — a JS function dispatched to a background queue included. An object that +first reached JS on the runtime's thread (`ObjCDataWrapper`/`BlockWrapper::BornOnRuntimeThread`, +recorded when the wrapper is built) is released on that thread: off it, `ObjectManager` queues +the native release (`[object release]`, `Block_release`) and posts one event loop task to drain +the queue; the JS side is disposed immediately either way. The same applies to +`__releaseNativeCounterpart` called from such a thread: the wrapper is retired at once, the +object a loop turn later. Objects that first reached JS on another thread are released where the +collection runs, as are collection adapters, whose `-dealloc` retires the wrapper. + +The queue needs no synchronization of its own: finalizers, `__releaseNativeCounterpart`, the +drain task, wrapper creation and teardown all hold the Locker. A new wrapper built for an object +whose release is still queued takes that release over (keeping the retain counts the extended +classes' swizzle reads exact). Teardown drains the queue inline, since the event loop has stopped +by then. For a worker, the runtime's thread is the worker's own thread, not the main thread. ## Deliberate deviations from upstream's original From 7a4469ef4b16c55cbdf046ea0a0f98045f7bdbf7 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Thu, 12 Nov 2026 21:43:07 -0300 Subject: [PATCH 3/3] fix(runtime): tell a runtime's thread by its own thread id A single thread-local marker named one isolate per thread, so a second runtime started on the same thread replaced it, and either one's teardown cleared it for both. Each runtime now records its thread where it binds its event loop, and the check compares it with the calling thread through the isolate's runtime. --- NativeScript/runtime/DataWrapper.h | 12 ++++-------- NativeScript/runtime/Runtime.h | 4 ++++ NativeScript/runtime/Runtime.mm | 16 +++++++++------- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/NativeScript/runtime/DataWrapper.h b/NativeScript/runtime/DataWrapper.h index d6be21a0..452f1120 100644 --- a/NativeScript/runtime/DataWrapper.h +++ b/NativeScript/runtime/DataWrapper.h @@ -20,14 +20,10 @@ class WorkerInspectorClient; namespace tns { -// The isolate whose runtime runs on this thread (its event loop's thread), -// while the runtime is bound here. JS can also run on other threads, any that -// take the isolate's Locker. -inline thread_local v8::Isolate* runtimeThreadIsolate = nullptr; - -inline bool OnRuntimeThreadOf(v8::Isolate* isolate) { - return isolate != nullptr && runtimeThreadIsolate == isolate; -} +// Whether the calling thread is the one `isolate`'s runtime runs on (its event +// loop's thread). JS can also run on other threads, any that take the +// isolate's Locker. Several runtimes may share a thread. Defined in Runtime.mm. +bool OnRuntimeThreadOf(v8::Isolate* isolate); class PrimitiveDataWrapper; struct ObjectWeakCallbackState; diff --git a/NativeScript/runtime/Runtime.h b/NativeScript/runtime/Runtime.h index f4faf180..714e44ae 100644 --- a/NativeScript/runtime/Runtime.h +++ b/NativeScript/runtime/Runtime.h @@ -1,6 +1,7 @@ #ifndef Runtime_h #define Runtime_h +#include #include #include @@ -51,6 +52,8 @@ class Runtime { inline CFRunLoopRef RuntimeLoop() { return runtimeLoop_; } + inline bool IsOnRuntimeThread() const { return pthread_equal(runtimeThread_, pthread_self()) != 0; } + /* * Scheduler bound to this runtime's runloop. Producers on any thread post; * entries run only on the runtime's home thread. @@ -192,6 +195,7 @@ class Runtime { std::unique_ptr moduleInternal_; int workerId_; CFRunLoopRef runtimeLoop_; + pthread_t runtimeThread_; std::shared_ptr eventLoop_; // Drains unhandled promise rejections once per runloop turn // (kCFRunLoopBeforeWaiting). Torn down before isolate disposal in ~Runtime. diff --git a/NativeScript/runtime/Runtime.mm b/NativeScript/runtime/Runtime.mm index 2559d2bf..26b0236a 100644 --- a/NativeScript/runtime/Runtime.mm +++ b/NativeScript/runtime/Runtime.mm @@ -266,6 +266,7 @@ void DisposeIsolateWhenPossible(Isolate* isolate, int gateId) { #endif Isolate* isolate = Isolate::New(create_params); runtimeLoop_ = CFRunLoopGetCurrent(); + runtimeThread_ = pthread_self(); // v8 already asked for this isolate's task runner during Isolate::New, so // the registry may hold an unbound loop - or a stopped one, when a worker // isolate reuses a disposed isolate's address. Refresh, then attach to this @@ -278,16 +279,12 @@ void DisposeIsolateWhenPossible(Isolate* isolate, int gateId) { eventLoop_->BindToCurrentThread(); isolate->SetData(Constants::RUNTIME_SLOT, this); isolate_ = isolate; - tns::runtimeThreadIsolate = isolate; } Runtime::~Runtime() { if (!initialized_) { // Never published: no other thread can know this isolate, and it holds no // caches, gate or wrappers. - if (tns::runtimeThreadIsolate == isolate_) { - tns::runtimeThreadIsolate = nullptr; - } isolate_->Dispose(); NativeScriptPlatform::Instance()->IsolateDisposed(isolate_, eventLoop_); currentRuntime_ = nullptr; @@ -373,9 +370,6 @@ void DisposeIsolateWhenPossible(Isolate* isolate, int gateId) { Caches::Remove(this->isolate_); this->isolate_->SetData(Constants::RUNTIME_SLOT, nullptr); - if (tns::runtimeThreadIsolate == this->isolate_) { - tns::runtimeThreadIsolate = nullptr; - } // Last, and under the Locker: from here on nothing reads a wrapper through // a JS object, so threads that never pinned may free theirs without the @@ -395,6 +389,14 @@ void DisposeIsolateWhenPossible(Isolate* isolate, int gateId) { currentRuntime_ = nullptr; } +bool OnRuntimeThreadOf(v8::Isolate* isolate) { + if (isolate == nullptr) { + return false; + } + Runtime* runtime = Runtime::GetRuntime(isolate); + return runtime != nullptr && runtime->IsOnRuntimeThread(); +} + Runtime* Runtime::GetRuntime(v8::Isolate* isolate) { return static_cast(isolate->GetData(Constants::RUNTIME_SLOT)); }