From da0b7c8a3dce234d9ff5fbb167e62c25ac08ae34 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Fri, 9 Oct 2026 16:29:29 -0300 Subject: [PATCH 1/2] test(runtime): reproduce collection adapters read across worker teardown A worker hands a JS array or object to native code that keeps reading it from four background threads while the worker is terminated. A read that passed IsolateWrapper::IsValid() and then queued on the isolate's Locker behind ~Runtime runs after the teardown removed the isolate's Caches: Caches::Get() returns an empty stand-in, and GetContext() dereferences its null context (SEGV in -[ArrayAdapter objectAtIndex:] under ASan). The same wait can also outlast Isolate::Dispose, which ~Runtime does not defer for a thread blocked on the Locker. --- TestFixtures/TNSTestNativeCallbacks.h | 5 ++ TestFixtures/TNSTestNativeCallbacks.m | 24 ++++++++++ .../app/tests/IsolateTeardownCallbackTests.js | 47 +++++++++++++++++++ .../app/tests/collectionAdapterQueryWorker.js | 7 +++ TestRunner/app/tests/index.js | 1 + 5 files changed, 84 insertions(+) create mode 100644 TestRunner/app/tests/IsolateTeardownCallbackTests.js create mode 100644 TestRunner/app/tests/collectionAdapterQueryWorker.js diff --git a/TestFixtures/TNSTestNativeCallbacks.h b/TestFixtures/TNSTestNativeCallbacks.h index 23b83beb..faaf7ce2 100644 --- a/TestFixtures/TNSTestNativeCallbacks.h +++ b/TestFixtures/TNSTestNativeCallbacks.h @@ -90,6 +90,11 @@ // queue. + (void)runOnBackgroundQueue:(void (^)(void))work completion:(void (^)(void))completion; +// Reads `collection` (an NSArray or NSDictionary) in a tight loop from +// `threads` global-queue threads until `ms` have passed. Those loops hold the +// last references to it, so its final release lands on one of them. ++ (void)query:(id)collection fromThreads:(int)threads forMilliseconds:(int)ms; + - (void (^)())getBlock; - (void (^)())getBlockFromNative; diff --git a/TestFixtures/TNSTestNativeCallbacks.m b/TestFixtures/TNSTestNativeCallbacks.m index 7438d9f4..51fe0f38 100644 --- a/TestFixtures/TNSTestNativeCallbacks.m +++ b/TestFixtures/TNSTestNativeCallbacks.m @@ -412,6 +412,30 @@ + (void)repeat:(int)count pausingAfter:(void (^)(int))step { } } ++ (void)query:(id)collection fromThreads:(int)threads forMilliseconds:(int)ms { + uint64_t deadline = clock_gettime_nsec_np(CLOCK_UPTIME_RAW) + (uint64_t)ms * NSEC_PER_MSEC; + for (int i = 0; i < threads; i++) { + dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0), ^{ + while (clock_gettime_nsec_np(CLOCK_UPTIME_RAW) < deadline) { + @autoreleasepool { + if ([collection isKindOfClass:[NSArray class]]) { + NSArray* array = collection; + if (array.count > 0) { + UNUSED([array objectAtIndex:0]); + } + } else if ([collection isKindOfClass:[NSDictionary class]]) { + NSDictionary* dictionary = collection; + UNUSED(dictionary.count); + for (id key in dictionary) { + UNUSED([dictionary objectForKey:key]); + } + } + } + } + }); + } +} + + (void)runOnBackgroundQueue:(void (^)(void))work completion:(void (^)(void))completion { dispatch_async(dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0), ^{ work(); diff --git a/TestRunner/app/tests/IsolateTeardownCallbackTests.js b/TestRunner/app/tests/IsolateTeardownCallbackTests.js new file mode 100644 index 00000000..418d47d1 --- /dev/null +++ b/TestRunner/app/tests/IsolateTeardownCallbackTests.js @@ -0,0 +1,47 @@ +// Native threads keep calling into a worker's isolate while that worker is +// torn down. Calls that were already waiting for the isolate's Locker when the +// teardown took it must not touch the isolate once it is gone. +describe("Collection adapters read from native threads", function () { + var ROUNDS = 3; + + var originalTimeout; + beforeEach(function () { + originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL; + jasmine.DEFAULT_TIMEOUT_INTERVAL = 20000; + }); + afterEach(function () { + jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout; + }); + + function terminateWhileQueried(kind, round, done) { + if (round === ROUNDS) { + done(); + return; + } + var worker = new Worker("./collectionAdapterQueryWorker.js"); + worker.onmessage = function (msg) { + expect(msg.data).toBe("querying"); + setTimeout(function () { + worker.terminate(); + // Past the native loops' deadline, so each round's last + // release has happened before the next one starts. + setTimeout(function () { + terminateWhileQueried(kind, round + 1, done); + }, 600); + }, 20); + }; + worker.onerror = function (e) { + expect(String(e && e.message ? e.message : e)).toBe(""); + done(); + }; + worker.postMessage(kind); + } + + it("survive the teardown of the worker that made an array adapter", function (done) { + terminateWhileQueried("array", 0, done); + }); + + it("survive the teardown of the worker that made a dictionary adapter", function (done) { + terminateWhileQueried("dictionary", 0, done); + }); +}); diff --git a/TestRunner/app/tests/collectionAdapterQueryWorker.js b/TestRunner/app/tests/collectionAdapterQueryWorker.js new file mode 100644 index 00000000..ba071ddd --- /dev/null +++ b/TestRunner/app/tests/collectionAdapterQueryWorker.js @@ -0,0 +1,7 @@ +// Hands a JS collection to native loops that keep reading it from background +// threads past this worker's termination. +onmessage = function (msg) { + var collection = msg.data === "array" ? [1, 2, 3] : { a: 1, b: 2, c: 3 }; + TNSTestNativeCallbacks.queryFromThreadsForMilliseconds(collection, 4, 400); + postMessage("querying"); +}; diff --git a/TestRunner/app/tests/index.js b/TestRunner/app/tests/index.js index f2f9e6fe..101d3396 100644 --- a/TestRunner/app/tests/index.js +++ b/TestRunner/app/tests/index.js @@ -131,6 +131,7 @@ require("./ApiTests"); require("./NsRuntimeTests"); require("./GCFinalizerTests"); require("./BlockCacheRaceTests"); +require("./IsolateTeardownCallbackTests"); require("./WorkerConcurrentStartupTests"); require("./WorkerOptionsTests"); require("./WorkerResourceLimitsTests"); From efb92efc1543601246a9b28f5f5de510eb3e167c Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Fri, 9 Oct 2026 16:35:10 -0300 Subject: [PATCH 2/2] fix(runtime): pin the isolate gate at the remaining foreign-thread entries The collection adapters, the extended classes' synthesized methods and the JS property accessors installed on native classes all checked IsolateWrapper::IsValid() and then waited for the isolate's Locker. A thread that passed the check could get the Locker only after ~Runtime had removed the isolate's Caches, or after Isolate::Dispose, and then read a null context or a freed isolate. These entries now follow ArgConverter::MethodCallback: pin the gate, bail when the pin is refused or the isolate is invalid, take the Locker, and check validity again under it. - ArrayAdapter, DictionaryAdapter and its key enumerators: count, objectAtIndex:, objectForKey:, keyEnumerator, nextObject and allObjects return 0/nil once the isolate is gone. - Adapter -dealloc (Array, Dictionary, NSData): like the JS block dispose, it detaches its claim under the Locker until the gate closes, even after the isolate is invalidated, because the teardown can still reach the claim through the JS object. Once the gate is closed it frees the claim without the Locker. - Extended class +initialize: skipped once the isolate is gone. - Extended class retain/release: pin only on the retain counts that toggle GC protection. The foreign-thread path posts through NativeScriptPlatform::LookupEventLoop and a runloop captured at __extends time instead of reading the Runtime, which the teardown frees. The protect/unprotect closures take the Locker before they read Instances. - PropertyCallbackContext carries an IsolateWrapper, and the property getter/setter callbacks of ClassBuilder and MetadataBuilder pin like MethodCallback: a getter returns a zeroed value and a setter does nothing once the isolate is gone. --- NativeScript/runtime/ArrayAdapter.mm | 75 +++++++------ NativeScript/runtime/ClassBuilder.h | 3 + NativeScript/runtime/ClassBuilder.mm | 130 ++++++++++++---------- NativeScript/runtime/DictionaryAdapter.mm | 116 +++++++++++-------- NativeScript/runtime/MetadataBuilder.mm | 16 +++ NativeScript/runtime/NSDataAdapter.mm | 52 +++++---- 6 files changed, 233 insertions(+), 159 deletions(-) diff --git a/NativeScript/runtime/ArrayAdapter.mm b/NativeScript/runtime/ArrayAdapter.mm index 2dfbdb9e..f882d531 100644 --- a/NativeScript/runtime/ArrayAdapter.mm +++ b/NativeScript/runtime/ArrayAdapter.mm @@ -39,15 +39,21 @@ - (instancetype)initWithJSObject:(Local)jsObject isolate:(Isolate*)isola - (NSUInteger)count { auto isolate = wrapper_->Isolate(); - if (!wrapper_->IsValid()) { - return 0; - } NSUInteger result = 0; // Scopes-before-@throw: a branded escape from the JS boundary is @thrown only // after the inner block's V8 scopes destruct. NSException* __strong pendingThrow = nil; { + // Any thread may read the array, so the isolate can be torn down while + // this one waits for the Locker; validity is checked again once it is held. + IsolatePin pin = wrapper_->Pin(); + if (!pin || !wrapper_->IsValid()) { + return 0; + } v8::Locker locker(isolate); + if (!wrapper_->IsValid()) { + return 0; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); @@ -76,10 +82,6 @@ - (NSUInteger)count { - (id)objectAtIndex:(NSUInteger)index { auto isolate = wrapper_->Isolate(); - if (!wrapper_->IsValid()) { - return nil; - } - if (!(index < [self count])) { // Out of bounds: return the adapter default rather than aborting. return nil; @@ -88,7 +90,14 @@ - (id)objectAtIndex:(NSUInteger)index { id result = nil; NSException* __strong pendingThrow = nil; { + IsolatePin pin = wrapper_->Pin(); + if (!pin || !wrapper_->IsValid()) { + return nil; + } v8::Locker locker(isolate); + if (!wrapper_->IsValid()) { + return nil; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); @@ -112,32 +121,36 @@ - (id)objectAtIndex:(NSUInteger)index { } - (void)dealloc { - if (wrapper_->IsValid()) { - auto isolate = wrapper_->Isolate(); - v8::Locker locker(isolate); - Isolate::Scope isolate_scope(isolate); - HandleScope handle_scope(isolate); - wrapper_->GetCache()->Instances.erase(self); - // Detach and free only a wrapper that is still the one we attached: a - // finalizer or __releaseNativeCounterpart can have retired it already, and - // whatever else sits in the field belongs to another owner. Once the - // isolate is gone the field can no longer be read, so the claim is dropped - // rather than freed blind. - if (dataWrapper_ != nullptr) { - Local value = self->object_->Get(isolate); - if (tns::GetValue(isolate, value) == dataWrapper_) { - tns::DeleteValue(isolate, value); - delete dataWrapper_; + { + IsolatePin pin = wrapper_->Pin(); + if (pin) { + auto isolate = wrapper_->Isolate(); + v8::Locker locker(isolate); + // Validity is not the test: until the teardown closes the gate it can + // still reach the claim through the JS object, so it is detached under + // the Locker even once the isolate is invalidated. + if (!wrapper_->IsTornDown()) { + Isolate::Scope isolate_scope(isolate); + HandleScope handle_scope(isolate); + wrapper_->GetCache()->Instances.erase(self); + // Detach and free only a wrapper that is still the one we attached: a + // finalizer or __releaseNativeCounterpart can have retired it already, + // and whatever else sits in the field belongs to another owner. + if (dataWrapper_ != nullptr) { + Local value = self->object_->Get(isolate); + if (tns::GetValue(isolate, value) == dataWrapper_) { + tns::DeleteValue(isolate, value); + delete dataWrapper_; + } + dataWrapper_ = nullptr; + } + self->object_->Reset(); } - dataWrapper_ = nullptr; } - self->object_->Reset(); - } else if (dataWrapper_ != nullptr) { - // The isolate is gone, and with it the JS object and every reader of the - // claim; no other path deletes one (__releaseNativeCounterpart leaves - // adapter claims attached), so the owner frees it here — adapters - // released after a worker isolate's teardown otherwise leak one wrapper - // each. + } + if (dataWrapper_ != nullptr) { + // The gate is closed: the JS object and every reader of the claim are + // gone, and no other path deletes an adapter claim. delete dataWrapper_; dataWrapper_ = nullptr; } diff --git a/NativeScript/runtime/ClassBuilder.h b/NativeScript/runtime/ClassBuilder.h index 8ab4a9f6..9b5dce83 100644 --- a/NativeScript/runtime/ClassBuilder.h +++ b/NativeScript/runtime/ClassBuilder.h @@ -2,6 +2,7 @@ #define ClassBuilder_h #include "Common.h" +#include "IsolateWrapper.h" #include "Metadata.h" namespace tns { @@ -14,10 +15,12 @@ struct PropertyCallbackContext { std::shared_ptr> implementationObject, const PropertyMeta* meta) : isolate_(isolate), + isolateWrapper_(isolate), callback_(callback), implementationObject_(implementationObject), meta_(meta) {} v8::Isolate* isolate_; + IsolateWrapper isolateWrapper_; std::shared_ptr> callback_; std::shared_ptr> implementationObject_; const PropertyMeta* meta_; diff --git a/NativeScript/runtime/ClassBuilder.mm b/NativeScript/runtime/ClassBuilder.mm index 5b91128b..4b93d75a 100644 --- a/NativeScript/runtime/ClassBuilder.mm +++ b/NativeScript/runtime/ClassBuilder.mm @@ -5,10 +5,12 @@ #include "ArgConverter.h" #include "BuiltinLoader.h" #include "Caches.h" +#include "EventLoop.h" #include "FastEnumerationAdapter.h" #include "Helpers.h" #include "Interop.h" #include "NativeScriptException.h" +#include "NativeScriptPlatform.h" #include "ObjectManager.h" #include "Runtime.h" #include "TNSDerivedClass.h" @@ -267,10 +269,15 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { cache->CtorFuncs.emplace(extendedClassName, poExtendedClassCtorFunc); IMP newInitialize = imp_implementationWithBlock(^(id self) { - if (!isolateWrapper.IsValid()) { + // +initialize runs on whichever thread first messages the class. + IsolatePin pin = isolateWrapper.Pin(); + if (!pin || !isolateWrapper.IsValid()) { return; } v8::Locker locker(isolate); + if (!isolateWrapper.IsValid()) { + return; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); Local context = Caches::Get(isolate)->GetContext(); @@ -313,35 +320,39 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { /// counterpart as well. id (*retain)(id, SEL) = (id (*)(id, SEL))FindNotOverridenMethod(extendedClass, @selector(retain)); + // Read here, on the isolate's thread: the blocks below run on any + // thread and must not reach the Runtime, which a teardown can free. + CFRunLoopRef runtimeLoop = Runtime::GetRuntime(isolate)->RuntimeLoop(); IMP newRetain = imp_implementationWithBlock(^id(id self) { - if (!isolateWrapper.IsValid()) { - return retain(self, @selector(retain)); - } if ([self retainCount] == 1) { - auto runtime = Runtime::GetRuntime(isolate); - auto runtimeLoop = runtime->RuntimeLoop(); - void* weakSelf = (__bridge void*)self; - auto gcProtect = [isolateWrapper, weakSelf, isolate]() { - auto innerCache = isolateWrapper.GetCache(); - auto it = innerCache->Instances.find((id)weakSelf); - if (it != innerCache->Instances.end()) { + IsolatePin pin = isolateWrapper.Pin(); + if (pin && isolateWrapper.IsValid()) { + void* weakSelf = (__bridge void*)self; + auto gcProtect = [isolateWrapper, weakSelf, isolate]() { v8::Locker locker(isolate); - Isolate::Scope isolate_scope(isolate); - HandleScope handle_scope(isolate); - Local value = it->second->Get(isolate); - BaseDataWrapper* wrapper = tns::GetValue(isolate, value); - if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) { - ObjCDataWrapper* objcWrapper = static_cast(wrapper); - objcWrapper->GcProtect(); + auto innerCache = isolateWrapper.GetCache(); + auto it = innerCache->Instances.find((id)weakSelf); + if (it != innerCache->Instances.end()) { + Isolate::Scope isolate_scope(isolate); + HandleScope handle_scope(isolate); + Local value = it->second->Get(isolate); + BaseDataWrapper* wrapper = tns::GetValue(isolate, value); + if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) { + ObjCDataWrapper* objcWrapper = static_cast(wrapper); + objcWrapper->GcProtect(); + } } + }; + if (CFRunLoopGetCurrent() != runtimeLoop) { + // bare entry: the closure does its own Locker ceremony + std::shared_ptr loop = + NativeScriptPlatform::Instance()->LookupEventLoop(isolate); + if (loop != nullptr) { + loop->PostInternalBare(gcProtect); + } + } else { + gcProtect(); } - }; - if (CFRunLoopGetCurrent() != runtimeLoop) { - // bare entry: the closure does its own Locker ceremony, exactly - // like the performed block it replaces - runtime->GetEventLoop()->PostInternalBare(gcProtect); - } else { - gcProtect(); } } @@ -352,21 +363,17 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { void (*release)(id, SEL) = (void (*)(id, SEL))FindNotOverridenMethod(extendedClass, @selector(release)); IMP newRelease = imp_implementationWithBlock(^(id self) { - if (!isolateWrapper.IsValid()) { - release(self, @selector(release)); - return; - } - if ([self retainCount] == 2) { - void* weakSelf = (__bridge void*)self; - auto gcUnprotect = [isolateWrapper, weakSelf, isolate]() { - auto innerCache = isolateWrapper.GetCache(); - auto it = innerCache->Instances.find((id)weakSelf); - if (it != innerCache->Instances.end()) { + IsolatePin pin = isolateWrapper.Pin(); + if (pin && isolateWrapper.IsValid()) { + void* weakSelf = (__bridge void*)self; + auto gcUnprotect = [isolateWrapper, weakSelf, isolate]() { v8::Locker locker(isolate); - Isolate::Scope isolate_scope(isolate); - HandleScope handle_scope(isolate); - if (it->second != nullptr) { + auto innerCache = isolateWrapper.GetCache(); + auto it = innerCache->Instances.find((id)weakSelf); + if (it != innerCache->Instances.end() && it->second != nullptr) { + Isolate::Scope isolate_scope(isolate); + HandleScope handle_scope(isolate); Local value = it->second->Get(isolate); BaseDataWrapper* wrapper = tns::GetValue(isolate, value); if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) { @@ -374,29 +381,16 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { objcWrapper->GcUnprotect(); } } - } - }; - auto runtime = Runtime::GetRuntime(isolate); - auto runtimeLoop = runtime->RuntimeLoop(); - if (CFRunLoopGetCurrent() != runtimeLoop) { - // bare entry: the closure does its own Locker ceremony, exactly - // like the performed block it replaces - runtime->GetEventLoop()->PostInternalBare(gcUnprotect); - } else { - auto innerCache = isolateWrapper.GetCache(); - auto it = innerCache->Instances.find(self); - if (it != innerCache->Instances.end()) { - v8::Locker locker(isolate); - Isolate::Scope isolate_scope(isolate); - HandleScope handle_scope(isolate); - if (it->second != nullptr) { - Local value = it->second->Get(isolate); - BaseDataWrapper* wrapper = tns::GetValue(isolate, value); - if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) { - ObjCDataWrapper* objcWrapper = static_cast(wrapper); - objcWrapper->GcUnprotect(); - } + }; + if (CFRunLoopGetCurrent() != runtimeLoop) { + // bare entry: the closure does its own Locker ceremony + std::shared_ptr loop = + NativeScriptPlatform::Instance()->LookupEventLoop(isolate); + if (loop != nullptr) { + loop->PostInternalBare(gcUnprotect); } + } else { + gcUnprotect(); } } } @@ -908,7 +902,16 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { // after every V8 scope in the inner block has destructed. NSException* __strong pendingThrow = nil; { + IsolatePin pin = context->isolateWrapper_.Pin(); + if (!pin || !context->isolateWrapper_.IsValid()) { + memset(retValue, 0, cif->rtype->size); + return; + } v8::Locker locker(isolate); + if (!context->isolateWrapper_.IsValid()) { + memset(retValue, 0, cif->rtype->size); + return; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); Local getterFunc = context->callback_->Get(isolate); @@ -956,7 +959,14 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { Isolate* isolate = context->isolate_; NSException* __strong pendingThrow = nil; { + IsolatePin pin = context->isolateWrapper_.Pin(); + if (!pin || !context->isolateWrapper_.IsValid()) { + return; + } v8::Locker locker(isolate); + if (!context->isolateWrapper_.IsValid()) { + return; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); Local setterFunc = context->callback_->Get(isolate); diff --git a/NativeScript/runtime/DictionaryAdapter.mm b/NativeScript/runtime/DictionaryAdapter.mm index 67a66606..0ac6595b 100644 --- a/NativeScript/runtime/DictionaryAdapter.mm +++ b/NativeScript/runtime/DictionaryAdapter.mm @@ -41,16 +41,20 @@ - (instancetype)initWithMap:(std::shared_ptr>)map } - (id)nextObject { - if (!wrapper_->IsValid()) { - return nil; - } Isolate* isolate = wrapper_->Isolate(); NSString* result = nil; // Scopes-before-@throw: keep V8 scopes in an inner block so a branded escape // is @thrown only after they destruct. NSException* __strong pendingThrow = nil; { + IsolatePin pin = wrapper_->Pin(); + if (!pin || !wrapper_->IsValid()) { + return nil; + } v8::Locker locker(isolate); + if (!wrapper_->IsValid()) { + return nil; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); @@ -140,14 +144,18 @@ - (instancetype)initWithProperties:(std::shared_ptr>)dictionar } - (id)nextObject { - if (!wrapper_->IsValid()) { - return nil; - } Isolate* isolate = wrapper_->Isolate(); NSString* result = nil; NSException* __strong pendingThrow = nil; { + IsolatePin pin = wrapper_->Pin(); + if (!pin || !wrapper_->IsValid()) { + return nil; + } v8::Locker locker(isolate); + if (!wrapper_->IsValid()) { + return nil; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); @@ -174,14 +182,18 @@ - (id)nextObject { } - (NSArray*)allObjects { - if (!wrapper_->IsValid()) { - return nil; - } Isolate* isolate = wrapper_->Isolate(); NSMutableArray* array = [NSMutableArray array]; NSException* __strong pendingThrow = nil; { + IsolatePin pin = wrapper_->Pin(); + if (!pin || !wrapper_->IsValid()) { + return nil; + } v8::Locker locker(isolate); + if (!wrapper_->IsValid()) { + return nil; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); @@ -247,14 +259,18 @@ - (instancetype)initWithJSObject:(Local)jsObject isolate:(Isolate*)isola } - (NSUInteger)count { - if (!wrapper_->IsValid()) { - return 0; - } Isolate* isolate = wrapper_->Isolate(); NSUInteger result = 0; NSException* __strong pendingThrow = nil; { + IsolatePin pin = wrapper_->Pin(); + if (!pin || !wrapper_->IsValid()) { + return 0; + } v8::Locker locker(isolate); + if (!wrapper_->IsValid()) { + return 0; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); @@ -283,14 +299,18 @@ - (NSUInteger)count { } - (id)objectForKey:(id)aKey { - if (!wrapper_->IsValid()) { - return nil; - } Isolate* isolate = wrapper_->Isolate(); id result = nil; NSException* __strong pendingThrow = nil; { + IsolatePin pin = wrapper_->Pin(); + if (!pin || !wrapper_->IsValid()) { + return nil; + } v8::Locker locker(isolate); + if (!wrapper_->IsValid()) { + return nil; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); @@ -334,11 +354,15 @@ - (id)objectForKey:(id)aKey { } - (NSEnumerator*)keyEnumerator { - if (!wrapper_->IsValid()) { + Isolate* isolate = wrapper_->Isolate(); + IsolatePin pin = wrapper_->Pin(); + if (!pin || !wrapper_->IsValid()) { return nil; } - Isolate* isolate = wrapper_->Isolate(); v8::Locker locker(isolate); + if (!wrapper_->IsValid()) { + return nil; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); @@ -356,35 +380,39 @@ - (NSEnumerator*)keyEnumerator { } - (void)dealloc { - if (wrapper_->IsValid()) { - Isolate* isolate = wrapper_->Isolate(); - v8::Locker locker(isolate); - Isolate::Scope isolate_scope(isolate); - HandleScope handle_scope(isolate); - wrapper_->GetCache()->Instances.erase(self); - // Detach and free only a wrapper that is still the one we attached: a - // finalizer or __releaseNativeCounterpart can have retired it already, and - // whatever else sits in the field belongs to another owner. Once the - // isolate is gone the field can no longer be read, so the claim is dropped - // rather than freed blind. - if (dataWrapper_ != nullptr) { - Local value = self->object_->Get(isolate); - if (tns::GetValue(isolate, value) == dataWrapper_) { - tns::DeleteValue(isolate, value); - delete dataWrapper_; + { + IsolatePin pin = wrapper_->Pin(); + if (pin) { + Isolate* isolate = wrapper_->Isolate(); + v8::Locker locker(isolate); + // Validity is not the test: until the teardown closes the gate it can + // still reach the claim through the JS object, so it is detached under + // the Locker even once the isolate is invalidated. + if (!wrapper_->IsTornDown()) { + Isolate::Scope isolate_scope(isolate); + HandleScope handle_scope(isolate); + wrapper_->GetCache()->Instances.erase(self); + // Detach and free only a wrapper that is still the one we attached: a + // finalizer or __releaseNativeCounterpart can have retired it already, + // and whatever else sits in the field belongs to another owner. + if (dataWrapper_ != nullptr) { + Local value = self->object_->Get(isolate); + if (tns::GetValue(isolate, value) == dataWrapper_) { + tns::DeleteValue(isolate, value); + delete dataWrapper_; + } + dataWrapper_ = nullptr; + } + // Persistent does not reset in its destructor; the enumerators + // vended by -keyEnumerator hold this adapter alive, so nothing can be + // reading the handle by the time this runs. + self->object_->Reset(); } - dataWrapper_ = nullptr; } - // Persistent does not reset in its destructor; the enumerators - // vended by -keyEnumerator hold this adapter alive, so nothing can be - // reading the handle by the time this runs. - self->object_->Reset(); - } else if (dataWrapper_ != nullptr) { - // The isolate is gone, and with it the JS object and every reader of the - // claim; no other path deletes one (__releaseNativeCounterpart leaves - // adapter claims attached), so the owner frees it here — adapters - // released after a worker isolate's teardown otherwise leak one wrapper - // each. + } + if (dataWrapper_ != nullptr) { + // The gate is closed: the JS object and every reader of the claim are + // gone, and no other path deletes an adapter claim. delete dataWrapper_; dataWrapper_ = nullptr; } diff --git a/NativeScript/runtime/MetadataBuilder.mm b/NativeScript/runtime/MetadataBuilder.mm index fd472856..6f6e83af 100644 --- a/NativeScript/runtime/MetadataBuilder.mm +++ b/NativeScript/runtime/MetadataBuilder.mm @@ -1196,7 +1196,16 @@ CMethodCall methodCall(context, item->userData_, typeEncoding, args, // the inner block's V8 scopes destruct. NSException* __strong pendingThrow = nil; { + IsolatePin pin = context->isolateWrapper_.Pin(); + if (!pin || !context->isolateWrapper_.IsValid()) { + memset(retValue, 0, cif->rtype->size); + return; + } v8::Locker locker(isolate); + if (!context->isolateWrapper_.IsValid()) { + memset(retValue, 0, cif->rtype->size); + return; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); Local getterFunc = context->callback_->Get(isolate); @@ -1257,7 +1266,14 @@ CMethodCall methodCall(context, item->userData_, typeEncoding, args, Isolate* isolate = context->isolate_; NSException* __strong pendingThrow = nil; { + IsolatePin pin = context->isolateWrapper_.Pin(); + if (!pin || !context->isolateWrapper_.IsValid()) { + return; + } v8::Locker locker(isolate); + if (!context->isolateWrapper_.IsValid()) { + return; + } Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); Local setterFunc = context->callback_->Get(isolate); diff --git a/NativeScript/runtime/NSDataAdapter.mm b/NativeScript/runtime/NSDataAdapter.mm index 2b2c6972..346d7782 100644 --- a/NativeScript/runtime/NSDataAdapter.mm +++ b/NativeScript/runtime/NSDataAdapter.mm @@ -102,32 +102,36 @@ - (NSUInteger)length { } - (void)dealloc { - if (wrapper_->IsValid()) { - auto isolate = wrapper_->Isolate(); - v8::Locker locker(isolate); - Isolate::Scope isolate_scope(isolate); - HandleScope handle_scope(isolate); - wrapper_->GetCache()->Instances.erase(self); - // Detach and free only a wrapper that is still the one we attached: a - // finalizer or __releaseNativeCounterpart can have retired it already, and - // whatever else sits in the field belongs to another owner. Once the - // isolate is gone the field can no longer be read, so the claim is dropped - // rather than freed blind. - if (dataWrapper_ != nullptr) { - Local value = self->object_->Get(isolate); - if (tns::GetValue(isolate, value) == dataWrapper_) { - tns::DeleteValue(isolate, value); - delete dataWrapper_; + { + IsolatePin pin = wrapper_->Pin(); + if (pin) { + auto isolate = wrapper_->Isolate(); + v8::Locker locker(isolate); + // Validity is not the test: until the teardown closes the gate it can + // still reach the claim through the JS object, so it is detached under + // the Locker even once the isolate is invalidated. + if (!wrapper_->IsTornDown()) { + Isolate::Scope isolate_scope(isolate); + HandleScope handle_scope(isolate); + wrapper_->GetCache()->Instances.erase(self); + // Detach and free only a wrapper that is still the one we attached: a + // finalizer or __releaseNativeCounterpart can have retired it already, + // and whatever else sits in the field belongs to another owner. + if (dataWrapper_ != nullptr) { + Local value = self->object_->Get(isolate); + if (tns::GetValue(isolate, value) == dataWrapper_) { + tns::DeleteValue(isolate, value); + delete dataWrapper_; + } + dataWrapper_ = nullptr; + } + self->object_->Reset(); } - dataWrapper_ = nullptr; } - self->object_->Reset(); - } else if (dataWrapper_ != nullptr) { - // The isolate is gone, and with it the JS object and every reader of the - // claim; no other path deletes one (__releaseNativeCounterpart leaves - // adapter claims attached), so the owner frees it here — adapters - // released after a worker isolate's teardown otherwise leak one wrapper - // each. + } + if (dataWrapper_ != nullptr) { + // The gate is closed: the JS object and every reader of the claim are + // gone, and no other path deletes an adapter claim. delete dataWrapper_; dataWrapper_ = nullptr; }