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 2dfbdb9e..67af9501 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,35 +121,39 @@ - (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_; + // 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) { + 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 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); + } + } + 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. - 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/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/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 62e29628..3bf7fc87 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,42 +389,46 @@ - (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_; + // 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) { + 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 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); + } + } + // 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. - 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..0ca99bed 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 @@ -308,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 41d5361b..47f987e5 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)}; @@ -521,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/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..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); @@ -779,8 +783,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/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..bad7b3de 100644 --- a/NativeScript/runtime/NSDataAdapter.mm +++ b/NativeScript/runtime/NSDataAdapter.mm @@ -102,35 +102,39 @@ - (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_; + // 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) { + 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 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); + } + } + 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. - 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/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/GCFinalizerTests.js b/TestRunner/app/tests/GCFinalizerTests.js index 4d77e7a5..e8a54521 100644 --- a/TestRunner/app/tests/GCFinalizerTests.js +++ b/TestRunner/app/tests/GCFinalizerTests.js @@ -497,4 +497,179 @@ 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(); + }; + }); + + // 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. + ["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/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/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/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"); 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"); 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");