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; } 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");