From f35930371960a833214a3a3136261163b28f9cd9 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Fri, 9 Oct 2026 16:29:29 -0300 Subject: [PATCH 1/4] 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 c8d65770..60a2fa12 100644 --- a/TestRunner/app/tests/index.js +++ b/TestRunner/app/tests/index.js @@ -134,6 +134,7 @@ require("./NsWorkerThreadsTests"); require("./NsUrlTests"); require("./GCFinalizerTests"); require("./BlockCacheRaceTests"); +require("./IsolateTeardownCallbackTests"); require("./WorkerConcurrentStartupTests"); require("./WorkerOptionsTests"); require("./WorkerResourceLimitsTests"); From 4d444d904df0c39bb5e29a63d840d4a990d0ef05 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Fri, 9 Oct 2026 16:35:10 -0300 Subject: [PATCH 2/4] 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 | 124 +++++++++++++-------- NativeScript/runtime/MetadataBuilder.mm | 16 +++ NativeScript/runtime/NSDataAdapter.mm | 52 +++++---- 6 files changed, 237 insertions(+), 163 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 ce792f4a..6e2abfee 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 62e29628..42811dad 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); @@ -256,14 +268,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); @@ -292,14 +308,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); @@ -343,11 +363,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); @@ -365,39 +389,43 @@ - (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 = - claimTarget_ != nullptr ? claimTarget_->Get(isolate) : 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 = + claimTarget_ != nullptr ? claimTarget_->Get(isolate) : 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(); + if (claimTarget_ != nullptr) { + claimTarget_->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(); - if (claimTarget_ != nullptr) { - claimTarget_->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 d45d980c..6ed5fc02 100644 --- a/NativeScript/runtime/MetadataBuilder.mm +++ b/NativeScript/runtime/MetadataBuilder.mm @@ -1312,7 +1312,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); @@ -1373,7 +1382,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; } From 3d224ee4384180c218259692ec8af18edd3fd9a6 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Wed, 14 Oct 2026 01:45:28 +0300 Subject: [PATCH 3/4] fix(runtime): avoid cross-isolate adapter and block teardown deadlocks The collection adapters (DictionaryAdapter, ArrayAdapter, NSDataAdapter) and the JS block dispose helper take their isolate's Locker on whatever thread drops the last native reference. When that thread is inside another isolate, it waits for one isolate's Locker while holding the other's, and the two can deadlock: - the main thread, inside a JS task, calls a native API that invokes a callback registered by a worker, and waits for the worker's Locker; - the worker's thread is collecting, and a finalizer releases an object holding an adapter or a block that belongs to the main isolate, whose teardown waits for the main isolate's Locker. A notification posted from the main isolate with a JS object as userInfo, observed by a block registered in a worker, is enough: the worker ends up with the last reference to each notification. IsolateWrapper::HandOffTeardown posts such a teardown to the owning isolate's loop when the calling thread is inside another isolate and does not hold the owner's Locker. If the loop drops the entry while shutting down, the entry runs it; if the loop refuses the post, a dispatch queue runs it with no isolate lock held. Adapters: -dealloc hands itself off and returns without freeing anything; the object stays allocated until it runs again on its own isolate. The claim an adapter attaches to its JS object holds the adapter weakly, so marshalling the object either takes a reference to a live adapter or, once its last release has started, detaches the dead claim and builds a fresh adapter that attaches its own. Only the adapter frees its claim: retiring the JS object, including __releaseNativeCounterpart, no longer releases the adapter. Blocks: libclosure frees a block as soon as its dispose helper returns, so the helper first clears the address cached in the function's BlockWrapper, under a mutex that reuse of the cached block also takes, and only then hands off the cleanup that needs the isolate. The ns:worker_threads specs get the 30 second startup bound the other worker suites use; the utility-priority worker did not always start within the default 5 seconds on CI. --- NativeScript/runtime/ArgConverter.mm | 2 +- NativeScript/runtime/ArrayAdapter.mm | 22 +-- NativeScript/runtime/DataWrapper.h | 39 +++-- NativeScript/runtime/DictionaryAdapter.mm | 22 +-- NativeScript/runtime/Helpers.h | 4 + NativeScript/runtime/Helpers.mm | 13 ++ NativeScript/runtime/Interop.h | 5 +- NativeScript/runtime/Interop.mm | 103 ++++++------- NativeScript/runtime/InteropTypes.mm | 3 +- NativeScript/runtime/IsolateWrapper.cpp | 53 +++++++ NativeScript/runtime/IsolateWrapper.h | 13 ++ NativeScript/runtime/NSDataAdapter.mm | 22 +-- NativeScript/runtime/ObjectManager.mm | 21 +-- TestRunner/app/tests/GCFinalizerTests.js | 137 ++++++++++++++++++ TestRunner/app/tests/NsWorkerThreadsTests.js | 22 ++- .../tests/adapterCrossIsolateReleaseWorker.js | 23 +++ .../app/tests/adapterTeardownReadWorker.js | 5 + 17 files changed, 393 insertions(+), 116 deletions(-) create mode 100644 TestRunner/app/tests/adapterCrossIsolateReleaseWorker.js create mode 100644 TestRunner/app/tests/adapterTeardownReadWorker.js diff --git a/NativeScript/runtime/ArgConverter.mm b/NativeScript/runtime/ArgConverter.mm index d5464db8..3ed1798b 100644 --- a/NativeScript/runtime/ArgConverter.mm +++ b/NativeScript/runtime/ArgConverter.mm @@ -534,7 +534,7 @@ if (type == BinaryTypeEncodingType::InterfaceDeclarationReference || type == BinaryTypeEncodingType::InstanceTypeEncoding || type == BinaryTypeEncodingType::IdEncoding) { - BaseDataWrapper* baseWrapper = tns::GetValue(isolate, value); + BaseDataWrapper* baseWrapper = tns::GetValueForMarshalling(isolate, value); if (baseWrapper != nullptr && baseWrapper->Type() == WrapperType::ObjCObject) { ObjCDataWrapper* wrapper = static_cast(baseWrapper); id data = wrapper->Data(); diff --git a/NativeScript/runtime/ArrayAdapter.mm b/NativeScript/runtime/ArrayAdapter.mm index f882d531..67af9501 100644 --- a/NativeScript/runtime/ArrayAdapter.mm +++ b/NativeScript/runtime/ArrayAdapter.mm @@ -121,6 +121,11 @@ - (id)objectAtIndex:(NSUInteger)index { } - (void)dealloc { + // Reached from a thread inside another isolate, the teardown below runs on + // this adapter's own isolate instead; the object stays allocated until then. + if (wrapper_->HandOffTeardown([self]() { [self dealloc]; })) { + return; + } { IsolatePin pin = wrapper_->Pin(); if (pin) { @@ -133,27 +138,22 @@ - (void)dealloc { 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. + // Detach only a claim that is still the one we attached: marshalling + // the object once this adapter's last release has started detaches + // it, 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(); } } } - 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; - } + // No other path frees an adapter claim, attached or not. + delete dataWrapper_; + dataWrapper_ = nullptr; delete wrapper_; self->object_ = nullptr; [super dealloc]; diff --git a/NativeScript/runtime/DataWrapper.h b/NativeScript/runtime/DataWrapper.h index a98655a5..0f43fccc 100644 --- a/NativeScript/runtime/DataWrapper.h +++ b/NativeScript/runtime/DataWrapper.h @@ -3,6 +3,7 @@ #include +#include #include #include #include @@ -398,16 +399,28 @@ class ObjCDataWrapper : public BaseDataWrapper { typeEncoding_(typeEncoding), klass_(object_getClass(data)) {} - const WrapperType Type() { return WrapperType::ObjCObject; } - - id Data() { return this->data_; } + ~ObjCDataWrapper() override { + if (adapterClaim_) { + objc_storeWeak(&data_, nil); + } + } - // True for the claim a collection adapter attaches to the plain JS object it - // was built from. The adapter owns that claim exclusively -- it must stay - // deletable from the adapter's -dealloc even after isolate teardown -- so no - // other retirement path may free it. - bool IsAdapterClaim() { return this->adapterClaim_; } - void MarkAdapterClaim() { this->adapterClaim_ = true; } + const WrapperType Type() override { return WrapperType::ObjCObject; } + + // A claim borrows its adapter. Weak loading atomically retains a live + // adapter until the autorelease pool drains, or returns nil once its last + // release has started, including while -dealloc waits for another isolate. + id Data() { return adapterClaim_ ? objc_loadWeak(&data_) : data_; } + + // The adapter owns its claim, including after isolate teardown. Retirement + // of the JS object must neither release the adapter nor free the claim. + bool IsAdapterClaim() { return adapterClaim_; } + void MarkAdapterClaim() { + id adapter = data_; + data_ = nil; + objc_storeWeak(&data_, adapter); + adapterClaim_ = true; + } const TypeEncoding* TypeEncoding() { return this->typeEncoding_; } @@ -492,13 +505,21 @@ class BlockWrapper : public BaseDataWrapper { const WrapperType Type() { return WrapperType::Block; } + // Raw access is for wrappers that own a native block. A borrowed JS block + // must be retained while the mutex excludes its dispose helper. void* Block() { return this->block_; } + void* RetainBlock(); + void ClearBlock() { + std::lock_guard lock(blockMutex_); + block_ = nullptr; + } const TypeEncoding* Encodings() { return this->typeEncoding_; } bool OwnsBlock() { return this->ownsBlock_; } private: + std::mutex blockMutex_; void* block_; const TypeEncoding* typeEncoding_; bool ownsBlock_; diff --git a/NativeScript/runtime/DictionaryAdapter.mm b/NativeScript/runtime/DictionaryAdapter.mm index 42811dad..3bf7fc87 100644 --- a/NativeScript/runtime/DictionaryAdapter.mm +++ b/NativeScript/runtime/DictionaryAdapter.mm @@ -389,6 +389,11 @@ - (NSEnumerator*)keyEnumerator { } - (void)dealloc { + // Reached from a thread inside another isolate, the teardown below runs on + // this adapter's own isolate instead; the object stays allocated until then. + if (wrapper_->HandOffTeardown([self]() { [self dealloc]; })) { + return; + } { IsolatePin pin = wrapper_->Pin(); if (pin) { @@ -401,17 +406,15 @@ - (void)dealloc { 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. + // Detach only a claim that is still the one we attached: marshalling + // the object once this adapter's last release has started detaches + // it, and whatever else sits in the field belongs to another owner. if (dataWrapper_ != nullptr) { Local value = claimTarget_ != nullptr ? claimTarget_->Get(isolate) : 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 @@ -423,12 +426,9 @@ - (void)dealloc { } } } - 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; - } + // No other path frees an adapter claim, attached or not. + delete dataWrapper_; + dataWrapper_ = nullptr; self->object_ = nullptr; delete claimTarget_; claimTarget_ = nullptr; diff --git a/NativeScript/runtime/Helpers.h b/NativeScript/runtime/Helpers.h index d4fd42b8..7aa48f1d 100644 --- a/NativeScript/runtime/Helpers.h +++ b/NativeScript/runtime/Helpers.h @@ -292,6 +292,10 @@ v8::Local GetCreationContextOrCurrent(v8::Isolate* isolate, // the Proxy target, so a proxied wrapper yields its target's wrapper. void SetValue(v8::Isolate* isolate, const v8::Local& obj, BaseDataWrapper* value); BaseDataWrapper* GetValue(v8::Isolate* isolate, const v8::Local& val); +// Acquires an autoreleased reference for an adapter claim. A claim whose +// adapter's last release has started is detached and reads as null, so the +// value gets a fresh adapter that attaches its own claim. +BaseDataWrapper* GetValueForMarshalling(v8::Isolate* isolate, const v8::Local& val); // What happens when JS touches a wrapper whose native counterpart has already // been released (its internal field was neutered by DisposeValue) — a state a diff --git a/NativeScript/runtime/Helpers.mm b/NativeScript/runtime/Helpers.mm index 41d5361b..41ad23f6 100644 --- a/NativeScript/runtime/Helpers.mm +++ b/NativeScript/runtime/Helpers.mm @@ -337,6 +337,19 @@ throw NativeScriptException( metadataProp.As()->Value(v8::kExternalPointerTypeTagDefault)); } +tns::BaseDataWrapper* tns::GetValueForMarshalling(Isolate* isolate, const Local& val) { + BaseDataWrapper* wrapper = tns::GetValue(isolate, val); + if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject && + static_cast(wrapper)->IsAdapterClaim() && + static_cast(wrapper)->Data() == nil) { + // Detached but not freed: the dying adapter owns its claim. With the slot + // empty, the replacement attaches its own and is found by later marshals. + tns::DeleteValue(isolate, val); + return nullptr; + } + return wrapper; +} + namespace { std::atomic releasedObjectPolicy_{static_cast(tns::ReleasedObjectPolicy::kReport)}; diff --git a/NativeScript/runtime/Interop.h b/NativeScript/runtime/Interop.h index f514f010..70d692f8 100644 --- a/NativeScript/runtime/Interop.h +++ b/NativeScript/runtime/Interop.h @@ -228,8 +228,9 @@ class Interop { static JSBlockDescriptor kJSBlockDescriptor; } JSBlock; - // Takes a reference to a cached JSBlock only while it is live. Block_copy - // would also revive a block whose last release already started its dispose. + friend class BlockWrapper; + // Called with the wrapper's mutex held, before its dispose can clear and + // free the cached block. Block_copy cannot reject a deallocating block. static bool TryRetainJSBlock(JSBlock* block); }; diff --git a/NativeScript/runtime/Interop.mm b/NativeScript/runtime/Interop.mm index 5d326c29..577c6061 100644 --- a/NativeScript/runtime/Interop.mm +++ b/NativeScript/runtime/Interop.mm @@ -31,6 +31,36 @@ static_cast(kUint64AllBitsSet << 53) + 1; // -9007199254740991 (-(2^53-1)) static constexpr int64_t kMaxSafeInteger = -kMinSafeInteger; // 9007199254740991 (2^53-1) +namespace { + +void DisposeJSBlockWrapper(MethodCallbackWrapper* wrapper, BlockWrapper* blockWrapper) { + if (wrapper->isolateWrapper_.HandOffTeardown( + [wrapper, blockWrapper]() { DisposeJSBlockWrapper(wrapper, blockWrapper); })) { + return; + } + { + IsolatePin pin = wrapper->isolateWrapper_.Pin(); + if (pin) { + Isolate* isolate = wrapper->isolateWrapper_.Isolate(); + v8::Locker locker(isolate); + if (!wrapper->isolateWrapper_.IsTornDown()) { + Isolate::Scope isolate_scope(isolate); + HandleScope handle_scope(isolate); + Local callback = wrapper->callback_->Get(isolate); + if (!callback.IsEmpty() && callback->IsObject() && + tns::GetValue(isolate, callback) == blockWrapper) { + tns::DeleteValue(isolate, callback); + } + wrapper->callback_->Reset(); + } + } + } + delete blockWrapper; + delete wrapper; +} + +} // namespace + Interop::JSBlock::JSBlockDescriptor Interop::JSBlock::kJSBlockDescriptor = { .reserved = 0, .size = sizeof(JSBlock), @@ -40,45 +70,12 @@ if (block->descriptor == &JSBlock::kJSBlockDescriptor) { MethodCallbackWrapper* wrapper = static_cast(block->userData); BlockWrapper* blockWrapper = block->blockWrapper; - // Runs on whatever thread drops the last native reference. That is - // safe inline: callback_ is a strong, unregistered persistent, so - // resetting it never touches the finalizer drain's bookkeeping, - // and a foreign-thread Locker into the block's own isolate is - // legitimate now that extended class names are worker-scoped. - // - // Validity is not the test here: the slot stays reachable after - // the isolate is invalidated, by JS still running under the Locker - // and by the teardown's own walk of registered objects. Until the - // gate closes, the wrapper is only freed once the slot is cleared. - { - IsolatePin pin = wrapper->isolateWrapper_.Pin(); - if (pin) { - Isolate* isolate = wrapper->isolateWrapper_.Isolate(); - v8::Locker locker(isolate); - if (!wrapper->isolateWrapper_.IsTornDown()) { - Isolate::Scope isolate_scope(isolate); - HandleScope handle_scope(isolate); - Local callback = wrapper->callback_->Get(isolate); - if (!callback.IsEmpty() && callback->IsObject()) { - // The slot may hold another wrapper by now; only our own is - // cleared from it. - if (tns::GetValue(isolate, callback) == blockWrapper) { - tns::DeleteValue(isolate, callback); - } - } - // Unconditional: an already-detached callback still owns its - // node, and dropping the persistent without a reset would leave - // that node rooted forever. - wrapper->callback_->Reset(); - } - } - } - // Outside the gate: once the teardown is done the cache slot is - // unreachable and nothing else can free the wrapper. - delete blockWrapper; - delete wrapper; + // libclosure frees the block as soon as this helper returns. + // Invalidate its cached address before deferring any V8 cleanup. + blockWrapper->ClearBlock(); ffi_closure_free(block->ffiClosure); block->~JSBlock(); + DisposeJSBlockWrapper(wrapper, blockWrapper); } }}; @@ -88,9 +85,9 @@ static constexpr int32_t kBlockDeallocating = 0x0001; static constexpr int32_t kBlockRefcountMask = 0xfffe; -// libclosure's increment ignores the deallocating bit, so Block_copy would hand -// out a block whose dispose is already waiting for the isolate's Locker and -// which libclosure frees right after. This CASes the same word libclosure does. +// libclosure's increment ignores the deallocating bit. The wrapper's mutex +// keeps the allocation alive here; this CAS also rejects a last release that +// won before dispose acquired that mutex. bool Interop::TryRetainJSBlock(JSBlock* block) { volatile int32_t* flags = &block->flags; int32_t old = __atomic_load_n(flags, __ATOMIC_RELAXED); @@ -108,6 +105,17 @@ } } +void* BlockWrapper::RetainBlock() { + if (ownsBlock_) { + return Block_copy(block_); + } + std::lock_guard lock(blockMutex_); + if (block_ == nullptr || !Interop::TryRetainJSBlock(static_cast(block_))) { + return nullptr; + } + return block_; +} + std::pair Interop::CreateMethodInternal(const uint8_t initialParamIndex, const uint8_t argsCount, const TypeEncoding* typeEncoding, @@ -618,15 +626,8 @@ static bool SnapshotArrayLike(Isolate* isolate, Local object, LocalOwnsBlock()) { - // A native block; the wrapper's own Block_copy keeps it alive. - blockPtr = CFAutorelease(Block_copy(wrapper->Block())); - } else if (TryRetainJSBlock(static_cast(wrapper->Block()))) { - // Reading the block is safe even when its last release raced ahead: - // until the isolate's teardown closes its gate, dispose clears this - // slot under the Locker this thread holds, before libclosure frees - // the block. - blockPtr = CFAutorelease(wrapper->Block()); + if (void* block = wrapper->RetainBlock()) { + blockPtr = CFAutorelease(block); } } if (blockPtr == nullptr) { @@ -742,7 +743,7 @@ static bool SnapshotArrayLike(Isolate* isolate, Local object, Local obj = arg.As(); - BaseDataWrapper* wrapper = tns::GetValue(isolate, obj); + BaseDataWrapper* wrapper = tns::GetValueForMarshalling(isolate, obj); if (wrapper != nullptr) { if (wrapper->Type() == WrapperType::Enum) { @@ -852,7 +853,7 @@ static bool SnapshotArrayLike(Isolate* isolate, Local object, LocalIsObject()) { - if (BaseDataWrapper* wrapper = tns::GetValue(isolate, arg)) { + if (BaseDataWrapper* wrapper = tns::GetValueForMarshalling(isolate, arg)) { switch (wrapper->Type()) { case WrapperType::ObjCObject: { ObjCDataWrapper* wr = static_cast(wrapper); diff --git a/NativeScript/runtime/InteropTypes.mm b/NativeScript/runtime/InteropTypes.mm index 568d9076..20c658c8 100644 --- a/NativeScript/runtime/InteropTypes.mm +++ b/NativeScript/runtime/InteropTypes.mm @@ -779,8 +779,7 @@ new PrimitiveDataWrapper(sizeof(void*), // A JS function does not keep its block alive: only native // references do. A live block is kept for the rest of the turn, // like one passed to a native call; a dying one is no handle. - JSBlock* block = static_cast(blockWrapper->Block()); - if (TryRetainJSBlock(block)) { + if (void* block = blockWrapper->RetainBlock()) { CFAutorelease(block); return Pointer::NewInstance(context, block); } diff --git a/NativeScript/runtime/IsolateWrapper.cpp b/NativeScript/runtime/IsolateWrapper.cpp index a7679221..4623de67 100644 --- a/NativeScript/runtime/IsolateWrapper.cpp +++ b/NativeScript/runtime/IsolateWrapper.cpp @@ -8,9 +8,12 @@ #include "IsolateWrapper.h" +#include + #include #include +#include "NativeScriptPlatform.h" #include "Runtime.h" #include "UnfairLock.h" #include "robin_hood.h" @@ -26,6 +29,56 @@ bool IsolateWrapper::IsValid() const { return cache->IsValid() && cache->getGateId() == gateId_; } +namespace { + +// Runs the teardown from the loop entry, or from the entry's destructor when +// the loop drops it unrun. +struct HandedOffTeardown { + explicit HandedOffTeardown(std::function teardown) + : teardown_(std::move(teardown)) {} + ~HandedOffTeardown() { Run(); } + void Run() { + std::function teardown = std::move(teardown_); + teardown_ = nullptr; + if (teardown) { + teardown(); + } + } + + private: + std::function teardown_; +}; + +} // namespace + +bool IsolateWrapper::HandOffTeardown(std::function teardown) const { + v8::Isolate* current = v8::Isolate::TryGetCurrent(); + if (current == nullptr || current == isolate_) { + return false; + } + // Keeps the isolate from being disposed while it is examined here. + IsolatePin pin = Pin(); + if (!pin || v8::Locker::IsLocked(isolate_)) { + return false; + } + std::shared_ptr loop = + NativeScriptPlatform::Instance()->LookupEventLoop(isolate_); + auto pending = std::make_shared(std::move(teardown)); + if (loop == nullptr || !loop->PostInternal([pending]() { pending->Run(); })) { + // A stopped loop can still have an active wrapper walk. Finish from a + // thread holding no isolate lock, so it may wait for that walk safely. + dispatch_async_f( + dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0), + new std::shared_ptr(pending), [](void* context) { + std::unique_ptr> entry( + static_cast*>(context)); + (*entry)->Run(); + }); + } + // The loop may already have run the teardown, which frees this wrapper. + return true; +} + namespace IsolateGates { namespace { diff --git a/NativeScript/runtime/IsolateWrapper.h b/NativeScript/runtime/IsolateWrapper.h index 96407565..4c7f3d14 100644 --- a/NativeScript/runtime/IsolateWrapper.h +++ b/NativeScript/runtime/IsolateWrapper.h @@ -9,6 +9,7 @@ #ifndef IsolateWrapper_h #define IsolateWrapper_h +#include #include #include "Caches.h" @@ -82,6 +83,18 @@ class IsolateWrapper { // True once the teardown has finished with every wrapper reachable from // JS. Read it while pinned. inline bool IsTornDown() const { return IsolateGates::IsClosed(gateId_); } + // For a teardown that needs this isolate's Locker and can be reached from + // any thread. A thread that is inside another isolate must not wait for + // this one's Locker: it may hold the other isolate's, and a thread inside + // this isolate may be waiting for that one. From such a thread `teardown` + // is posted to this isolate's loop and true is returned; the caller must + // then return without touching anything `teardown` frees, this wrapper + // included. `teardown` runs once: from the loop, or when the loop drops + // the entry while shutting down. If the loop refuses the post, a native + // queue runs it without holding any isolate lock. False means the caller + // runs the teardown itself. The teardown must pin and check IsTornDown() + // under the Locker before touching handles. + bool HandOffTeardown(std::function teardown) const; private: v8::Isolate* isolate_; diff --git a/NativeScript/runtime/NSDataAdapter.mm b/NativeScript/runtime/NSDataAdapter.mm index 346d7782..bad7b3de 100644 --- a/NativeScript/runtime/NSDataAdapter.mm +++ b/NativeScript/runtime/NSDataAdapter.mm @@ -102,6 +102,11 @@ - (NSUInteger)length { } - (void)dealloc { + // Reached from a thread inside another isolate, the teardown below runs on + // this adapter's own isolate instead; the object stays allocated until then. + if (wrapper_->HandOffTeardown([self]() { [self dealloc]; })) { + return; + } { IsolatePin pin = wrapper_->Pin(); if (pin) { @@ -114,27 +119,22 @@ - (void)dealloc { 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. + // Detach only a claim that is still the one we attached: marshalling + // the object once this adapter's last release has started detaches + // it, 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(); } } } - 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; - } + // No other path frees an adapter claim, attached or not. + delete dataWrapper_; + dataWrapper_ = nullptr; delete self->wrapper_; self->object_ = nullptr; diff --git a/NativeScript/runtime/ObjectManager.mm b/NativeScript/runtime/ObjectManager.mm index 904ab233..3ededf3c 100644 --- a/NativeScript/runtime/ObjectManager.mm +++ b/NativeScript/runtime/ObjectManager.mm @@ -207,6 +207,9 @@ void DisposeHandle(v8::Isolate* isolate, return true; case WrapperType::ObjCObject: { ObjCDataWrapper* objCObjectWrapper = static_cast(wrapper); + if (objCObjectWrapper->IsAdapterClaim()) { + return true; + } id target = objCObjectWrapper->Data(); if (target != nil) { // Instances is keyed on the raw address, so an entry rebuilt for a @@ -350,6 +353,9 @@ void DisposeHandle(v8::Isolate* isolate, } ObjCDataWrapper* objcWrapper = static_cast(wrapper); + if (objcWrapper->IsAdapterClaim()) { + return; + } id data = objcWrapper->Data(); if (data != nil) { std::shared_ptr cache = Caches::Get(isolate); @@ -380,21 +386,10 @@ void DisposeHandle(v8::Isolate* isolate, // wrapped or adopted from an alloc/new/copy return). For instances solely // owned by JS this deallocates immediately; for shared natives (e.g. an // NSNotificationCenter observer token) the remaining owners keep it alive. - // Calling [data dealloc] here, as this used to do, destroyed objects that - // were still referenced elsewhere and caused use-after-free crashes. - // Read before the release below: an adapter claim's -dealloc frees the - // wrapper, so it must not be touched afterwards. - bool adapterClaim = objcWrapper->IsAdapterClaim(); - [data release]; - // The release above can run a -dealloc that detaches this wrapper; the - // internal field owns it, so free only what is still attached. An - // adapter's claim is exempt: the adapter owns it exclusively and deletes - // it from its own -dealloc even after isolate teardown, so retiring it - // here would leave the adapter holding a stale pointer it later frees - // again. - if (!adapterClaim && tns::GetValue(isolate, value) == wrapper) { + // A native -dealloc may detach the wrapper through a nested callback. + if (tns::GetValue(isolate, value) == wrapper) { delete wrapper; tns::SetValue(isolate, value.As(), nullptr); } diff --git a/TestRunner/app/tests/GCFinalizerTests.js b/TestRunner/app/tests/GCFinalizerTests.js index 4d77e7a5..a5236624 100644 --- a/TestRunner/app/tests/GCFinalizerTests.js +++ b/TestRunner/app/tests/GCFinalizerTests.js @@ -497,4 +497,141 @@ describe("GC finalizer callbacks", function () { done(); }, 0); }); + + it("leaves borrowed adapter claims owned by their native containers", function () { + var value = { answer: 42 }; + var holder = NSMutableArray.alloc().init(); + holder.addObject(value); + __releaseNativeCounterpart(value); + __releaseNativeCounterpart(value); + expect(TNSTestNativeCallbacks.dictionaryValueForKeyKey(value, "answer")).toBe(42); + holder.removeAllObjects(); + }); + + describe("cross-isolate finalizer release", function () { + var worker, timer, observer, originalTimeout; + beforeEach(function () { + originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL; + jasmine.DEFAULT_TIMEOUT_INTERVAL = 15000; + }); + afterEach(function () { + clearTimeout(timer); + if (observer) { + NSNotificationCenter.defaultCenter.removeObserver(observer); + observer = null; + } + if (worker) { + worker.terminate(); + worker = null; + } + jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout; + }); + + it("reads adapters and enumerators while their worker shuts down", function (done) { + var values = []; + var center = NSNotificationCenter.defaultCenter; + observer = center.addObserverForNameObjectQueueUsingBlock( + "adapter-teardown-read", null, null, function (notification) { + values.push(notification.object); + }); + worker = new Worker("./adapterTeardownReadWorker.js"); + worker.onmessage = function () { + center.removeObserver(observer); + expect(values.length).toBe(4); + var keys = [values[0].keyEnumerator(), values[1].keyEnumerator()]; + worker.terminate(); + TNSTestNativeCallbacks.repeatPausingAfter(100, function () { + values[0].count; + values[1].objectForKey("answer"); + values[2].objectAtIndex(0); + keys[0].nextObject(); + keys[1].allObjects; + }); + var attempts = 100; + (function poll() { + if (values[0].count !== 0 && --attempts > 0) { + timer = setTimeout(poll, 10); + return; + } + expect(values[0].count).toBe(0); + expect(values[1].count).toBe(0); + expect(values[2].count).toBe(0); + expect(keys[0].nextObject()).toBeNull(); + expect(keys[1].nextObject()).toBeNull(); + expect(values[3].length).toBe(2); + done(); + })(); + }; + worker.onerror = function (e) { + center.removeObserver(observer); + expect(String(e && e.message ? e.message : e)).toBe(""); + done(); + }; + }); + + // The main thread posts under its Locker, then waits for the worker's + // observer. The worker collects notifications under its own Locker, + // dropping native objects whose teardown needs the main isolate. + ["adapters", "blocks"].forEach(function (kind) { + it("releases another isolate's " + kind + " from a worker finalizer", function (done) { + worker = new Worker("./adapterCrossIsolateReleaseWorker.js"); + var remaining = 400; + var sent = 0; + var identityMisses = 0; + var shared = [{ posted: 0 }, new Proxy({ value: 1 }, {}), [1, 2], new Uint8Array([3, 4])]; + var sharedCallback = function () {}; + var center = NSNotificationCenter.defaultCenter; + + function post(value) { + center.postNotificationNameObjectUserInfo("adapter-cross-isolate-release", value, null); + sent++; + } + function postBlock(callback) { + var operation = NSBlockOperation.blockOperationWithBlock(callback); + post(operation); + __releaseNativeCounterpart(operation); + } + function tick() { + if (kind === "adapters") { + post({ remaining: remaining }); + shared[0].posted++; + shared.forEach(post); + // Native code holding a shared object's adapter gets + // that adapter back for the object, also while the one + // the worker dropped still waits here for its teardown. + var holder = NSMutableArray.alloc().init(); + shared.forEach(function (value, index) { + holder.addObject(value); + if (holder.indexOfObjectIdenticalTo(value) !== index) { + identityMisses++; + } + }); + holder.removeAllObjects(); + } else { + postBlock(function () {}); + postBlock(sharedCallback); + } + if (--remaining === 0) { + worker.postMessage("stop"); + } else { + timer = setTimeout(tick, 1); + } + } + worker.onmessage = function (msg) { + if (msg.data === "observing") { + tick(); + return; + } + expect(msg.data.received).toBe(sent); + expect(remaining).toBe(0); + expect(identityMisses).toBe(0); + done(); + }; + worker.onerror = function (e) { + expect(String(e && e.message ? e.message : e)).toBe(""); + done(); + }; + }); + }); + }); }); diff --git a/TestRunner/app/tests/NsWorkerThreadsTests.js b/TestRunner/app/tests/NsWorkerThreadsTests.js index db84b3a2..8d5a691e 100644 --- a/TestRunner/app/tests/NsWorkerThreadsTests.js +++ b/TestRunner/app/tests/NsWorkerThreadsTests.js @@ -1,5 +1,20 @@ describe("ns:worker_threads", function () { var nsWorkerThreads = require("ns:worker_threads"); + var worker, originalTimeout; + + beforeEach(function () { + originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL; + jasmine.DEFAULT_TIMEOUT_INTERVAL = 30000; + }); + afterEach(function () { + if (worker) { + worker.onmessage = null; + worker.onerror = null; + worker.terminate(); + worker = null; + } + jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout; + }); it("exposes frozen exports", function () { expect(Object.isFrozen(nsWorkerThreads)).toBe(true); @@ -27,7 +42,7 @@ describe("ns:worker_threads", function () { }); it("constructs a worker with the runtime's options", function (done) { - var worker = new nsWorkerThreads.Worker("./workerResourceLimits/echoWorker.js", { + worker = new nsWorkerThreads.Worker("./workerResourceLimits/echoWorker.js", { ios: { priority: "utility" }, resourceLimits: { maxOldGenerationSizeMb: 64 }, }); @@ -37,7 +52,6 @@ describe("ns:worker_threads", function () { return; } settled = true; - worker.terminate(); done(); }; worker.onmessage = function (event) { @@ -51,14 +65,12 @@ describe("ns:worker_threads", function () { }); it("hands a worker realm its own constructor, not whatever the global names", function (done) { - var worker = new Worker("./nsWorkerThreadsIdentityWorker.js"); + worker = new Worker("./nsWorkerThreadsIdentityWorker.js"); worker.onmessage = function (event) { expect(event.data).toEqual({ isOriginal: true, isImpostor: false, frozen: true }); - worker.terminate(); done(); }; worker.onerror = function (event) { - worker.terminate(); fail("worker error: " + event.message); done(); }; diff --git a/TestRunner/app/tests/adapterCrossIsolateReleaseWorker.js b/TestRunner/app/tests/adapterCrossIsolateReleaseWorker.js new file mode 100644 index 00000000..a6a4c5f7 --- /dev/null +++ b/TestRunner/app/tests/adapterCrossIsolateReleaseWorker.js @@ -0,0 +1,23 @@ +// Collects notifications holding the parent's adapters or blocks, so their +// last native release can run inside this isolate's finalizers. +var name = "adapter-cross-isolate-release"; +var observing = true; +var received = 0; +var observer = NSNotificationCenter.defaultCenter.addObserverForNameObjectQueueUsingBlock( + name, null, null, function (notification) { received++; }); + +(function collect() { + if (!observing) { + return; + } + __collect(); + setTimeout(collect, 1); +})(); + +onmessage = function () { + observing = false; + NSNotificationCenter.defaultCenter.removeObserver(observer); + postMessage({ received: received }); +}; + +postMessage("observing"); diff --git a/TestRunner/app/tests/adapterTeardownReadWorker.js b/TestRunner/app/tests/adapterTeardownReadWorker.js new file mode 100644 index 00000000..0f9ff4d9 --- /dev/null +++ b/TestRunner/app/tests/adapterTeardownReadWorker.js @@ -0,0 +1,5 @@ +var center = NSNotificationCenter.defaultCenter; +[{ answer: 42 }, new Map([["answer", 42]]), [1, 2], new Uint8Array([3, 4])].forEach(function (value) { + center.postNotificationNameObjectUserInfo("adapter-teardown-read", value, null); +}); +postMessage("sent"); From d84fd12af3e98cafa36f2f0a4f3b5e42eae186f1 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Wed, 14 Oct 2026 11:25:32 +0300 Subject: [PATCH 4/4] fix(runtime): treat a claim whose adapter is being released as released While an adapter's teardown waits on its isolate's loop, the claim stays on the JS object and loads as nil. interop.bufferFromData asserted on it, and interop.handleof released a null reference. GetValueOrReport detaches such a claim and reports the object as released, as marshalling it would, and handleof returns no handle for it. --- NativeScript/runtime/Helpers.h | 4 +- NativeScript/runtime/Helpers.mm | 2 +- NativeScript/runtime/InteropTypes.mm | 4 ++ TestRunner/app/tests/GCFinalizerTests.js | 38 +++++++++++++++++++ .../app/tests/adapterClaimReleaseWorker.js | 17 +++++++++ 5 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 TestRunner/app/tests/adapterClaimReleaseWorker.js diff --git a/NativeScript/runtime/Helpers.h b/NativeScript/runtime/Helpers.h index 7aa48f1d..0ca99bed 100644 --- a/NativeScript/runtime/Helpers.h +++ b/NativeScript/runtime/Helpers.h @@ -312,8 +312,8 @@ enum class ReleasedObjectPolicy { ReleasedObjectPolicy GetReleasedObjectPolicy(); void SetReleasedObjectPolicy(ReleasedObjectPolicy policy); -// Like GetValue, but applies ReleasedObjectPolicy when the object carries no -// wrapper: throws (kThrow) or schedules a `releasednativeaccess` event +// Like GetValueForMarshalling, but applies ReleasedObjectPolicy when the object +// carries no wrapper: throws (kThrow) or schedules a `releasednativeaccess` event // (kReport, deduplicated per object), returning nullptr either way. // `operation` names the API surface for the event payload / error message. // Callers must no-op on nullptr; paths that would otherwise read the internal diff --git a/NativeScript/runtime/Helpers.mm b/NativeScript/runtime/Helpers.mm index 41ad23f6..47f987e5 100644 --- a/NativeScript/runtime/Helpers.mm +++ b/NativeScript/runtime/Helpers.mm @@ -534,7 +534,7 @@ void WriteDebugLine(tns::LogCategory category, const char* message) { tns::BaseDataWrapper* tns::GetValueOrReport(Isolate* isolate, const Local& val, const char* operation) { - BaseDataWrapper* wrapper = tns::GetValue(isolate, val); + BaseDataWrapper* wrapper = tns::GetValueForMarshalling(isolate, val); if (wrapper != nullptr) { return wrapper; } diff --git a/NativeScript/runtime/InteropTypes.mm b/NativeScript/runtime/InteropTypes.mm index 20c658c8..32e16b63 100644 --- a/NativeScript/runtime/InteropTypes.mm +++ b/NativeScript/runtime/InteropTypes.mm @@ -730,6 +730,10 @@ new PrimitiveDataWrapper(sizeof(void*), ObjCDataWrapper* w = static_cast(wrapper); @autoreleasepool { id target = w->Data(); + // A claim whose adapter's last release has started is no handle. + if (target == nil) { + break; + } CFTypeRef ref = CFBridgingRetain(target); void* handle = const_cast(ref); CFRelease(ref); diff --git a/TestRunner/app/tests/GCFinalizerTests.js b/TestRunner/app/tests/GCFinalizerTests.js index a5236624..e8a54521 100644 --- a/TestRunner/app/tests/GCFinalizerTests.js +++ b/TestRunner/app/tests/GCFinalizerTests.js @@ -569,6 +569,44 @@ describe("GC finalizer callbacks", function () { }; }); + // While this isolate runs JS, teardowns the worker hands off wait, and + // the claims stay on their objects with their adapters released. + it("treats a claim whose adapter another isolate released as released", function (done) { + worker = new Worker("./adapterClaimReleaseWorker.js"); + var object = { value: 1 }; + var bytes = new Uint8Array([1, 2]); + worker.onmessage = function (msg) { + if (msg.data === "released") { + done(); + return; + } + var center = NSNotificationCenter.defaultCenter; + center.postNotificationNameObjectUserInfo("adapter-claim-release", bytes, null); + center.postNotificationNameObjectUserInfo("adapter-claim-release", object, null); + // The notifications stay autoreleased until this turn ends. + timer = setTimeout(function () { + worker.postMessage("release"); + // The worker releases the bytes' adapter first, so it is + // released once the object's handle is gone. + var handleReleased = false; + var deadline = Date.now() + 5000; + while (!handleReleased && Date.now() < deadline) { + try { + interop.handleof(object); + } catch (e) { + handleReleased = true; + } + } + expect(handleReleased).toBe(true); + expect(interop.bufferFromData(bytes)).toBeUndefined(); + }); + }; + worker.onerror = function (e) { + expect(String(e && e.message ? e.message : e)).toBe(""); + done(); + }; + }); + // The main thread posts under its Locker, then waits for the worker's // observer. The worker collects notifications under its own Locker, // dropping native objects whose teardown needs the main isolate. diff --git a/TestRunner/app/tests/adapterClaimReleaseWorker.js b/TestRunner/app/tests/adapterClaimReleaseWorker.js new file mode 100644 index 00000000..17d6667a --- /dev/null +++ b/TestRunner/app/tests/adapterClaimReleaseWorker.js @@ -0,0 +1,17 @@ +// Keeps the parent's notifications until asked, then releases them on this +// worker's thread in the order they arrived. +var name = "adapter-claim-release"; +var notifications = []; +var observer = NSNotificationCenter.defaultCenter.addObserverForNameObjectQueueUsingBlock( + name, null, null, function (notification) { notifications.push(notification); }); + +onmessage = function () { + NSNotificationCenter.defaultCenter.removeObserver(observer); + notifications.forEach(function (notification) { + __releaseNativeCounterpart(notification); + }); + notifications = []; + postMessage("released"); +}; + +postMessage("observing");