diff --git a/NativeScript/runtime/ArgConverter.mm b/NativeScript/runtime/ArgConverter.mm index d5464db8..1875c941 100644 --- a/NativeScript/runtime/ArgConverter.mm +++ b/NativeScript/runtime/ArgConverter.mm @@ -638,9 +638,20 @@ 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) { + // 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); } } @@ -908,7 +919,15 @@ 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) { + ObjCDataWrapper* objcWrapper = static_cast(wrapper); + objcWrapper->SetBornOnRuntimeThread(); + ObjectManager::ProtectIfNativelyRetained(target, objcWrapper); + } } } 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/DataWrapper.h b/NativeScript/runtime/DataWrapper.h index a98655a5..452f1120 100644 --- a/NativeScript/runtime/DataWrapper.h +++ b/NativeScript/runtime/DataWrapper.h @@ -20,6 +20,11 @@ class WorkerInspectorClient; namespace tns { +// 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; class EventLoop; @@ -396,7 +401,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 +415,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 +432,7 @@ class ObjCDataWrapper : public BaseDataWrapper { id data_; const tns::TypeEncoding* typeEncoding_; Class klass_; + bool bornOnRuntimeThread_; }; class ObjCClassWrapper : public BaseDataWrapper { @@ -488,7 +500,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 +513,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.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..6d98e6cd 100644 --- a/NativeScript/runtime/ObjectManager.mm +++ b/NativeScript/runtime/ObjectManager.mm @@ -2,11 +2,16 @@ #include #include #include +#include +#include #include "Caches.h" #include "Constants.h" #include "DataWrapper.h" #include "FFICall.h" +#include "EventLoop.h" #include "Helpers.h" +#include "Runtime.h" +#include "TNSDerivedClass.h" using namespace v8; using namespace std; @@ -15,11 +20,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 +52,96 @@ void UnlinkRegistered(ObjectWeakCallbackState* state) { state->next_ = nullptr; } +// 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. +// +// 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 { + // 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 drainPosted = false; +}; + +void DrainDeferredReleases(DeferredReleases* pending) { + std::unordered_map objects; + std::vector blocks; + 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++) { + [obj release]; + } + } + for (const void* block : blocks) { + Block_release(block); + } +} + +// 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; + } + return Caches::ExistingStateFor(isolate); +} + +// 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; + } + 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); + } + }); +} + +void ReleaseOnRuntimeThread(Isolate* isolate, id obj, bool bornOnRuntimeThread) { + DeferredReleases* pending = DeferralQueue(isolate, bornOnRuntimeThread); + if (pending == nullptr) { + [obj release]; + return; + } + pending->objects[(__bridge void*)obj]++; + PostDrain(isolate, pending); +} + +void BlockReleaseOnRuntimeThread(Isolate* isolate, const void* block, bool bornOnRuntimeThread) { + DeferredReleases* pending = DeferralQueue(isolate, bornOnRuntimeThread); + if (pending == nullptr) { + Block_release(block); + return; + } + pending->blocks.push_back(block); + PostDrain(isolate, pending); +} + } // namespace +void ObjectManager::Init(Isolate* isolate, Local globalTemplate) { + globalTemplate->Set(tns::ToV8String(isolate, "__releaseNativeCounterpart"), + FunctionTemplate::New(isolate, ReleaseNativeCounterpartCallback)); + Caches::StateFor(isolate); +} + std::shared_ptr> ObjectManager::Register(Local context, const Local obj) { Isolate* isolate = v8::Isolate::GetCurrent(); @@ -104,6 +192,12 @@ void DisposeHandle(v8::Isolate* isolate, Isolate::Scope isolateScope(isolate); HandleScope scope(isolate); + // 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 // claim every state up front: disposing one entry can re-enter JS from // -dealloc and reach __releaseNativeCounterpart for any other entry this @@ -130,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); @@ -175,6 +275,28 @@ void DisposeHandle(v8::Isolate* isolate, state->target_->SetWeak(state, FinalizerCallback, WeakCallbackType::kFinalizer); } +bool ObjectManager::TakeDeferredRelease(Isolate* isolate, id obj) { + DeferredReleases* pending = Caches::ExistingStateFor(isolate); + if (pending == nullptr || pending->objects.empty()) { + return false; + } + auto it = pending->objects.find((__bridge void*)obj); + if (it == pending->objects.end()) { + return false; + } + if (--it->second == 0) { + pending->objects.erase(it); + } + 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 +341,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, objCObjectWrapper->BornOnRuntimeThread()); + } } break; } @@ -237,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.) - Block_release(blockWrapper->Block()); + BlockReleaseOnRuntimeThread(isolate, blockWrapper->Block(), + blockWrapper->BornOnRuntimeThread()); break; } case WrapperType::Reference: { @@ -386,7 +514,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, objcWrapper->BornOnRuntimeThread()); + } // 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/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 ebe46f48..26b0236a 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" @@ -265,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 @@ -387,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)); } diff --git a/TestRunner/app/tests/GCFinalizerTests.js b/TestRunner/app/tests/GCFinalizerTests.js index 4d77e7a5..a40a61ad 100644 --- a/TestRunner/app/tests/GCFinalizerTests.js +++ b/TestRunner/app/tests/GCFinalizerTests.js @@ -35,6 +35,71 @@ 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); + })(); + }); + + // 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 ece736d6..233749e9 100644 --- a/docs/knowledge/v8-resurrecting-finalizers.md +++ b/docs/knowledge/v8-resurrecting-finalizers.md @@ -129,6 +129,24 @@ 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. 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 - **No `PENDING` state, no `NEAR_DEATH` for finalizers.** 14.9's `NodeState` is 2 bits with all