Skip to content

fix(runtime): pin the isolate gate at the remaining foreign-thread entries #506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
edusperoni wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix(runtime): pin the isolate gate at the remaining foreign-thread entries #506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da0b7c8
test(runtime): reproduce collection adapters read across worker teardown
edusperoni Oct 9, 2026
efb92ef
fix(runtime): pin the isolate gate at the remaining foreign-thread en…
edusperoni Oct 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 44 additions & 31 deletions NativeScript/runtime/ArrayAdapter.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,21 @@ - (instancetype)initWithJSObject:(Local<Object>)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);

Expand Down Expand Up @@ -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;
Expand All @@ -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);

Expand All @@ -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> 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> 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;
}
Expand Down
3 changes: 3 additions & 0 deletions NativeScript/runtime/ClassBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define ClassBuilder_h

#include "Common.h"
#include "IsolateWrapper.h"
#include "Metadata.h"

namespace tns {
Expand All @@ -14,10 +15,12 @@ struct PropertyCallbackContext {
std::shared_ptr<v8::Persistent<v8::Object>> implementationObject,
const PropertyMeta* meta)
: isolate_(isolate),
isolateWrapper_(isolate),
callback_(callback),
implementationObject_(implementationObject),
meta_(meta) {}
v8::Isolate* isolate_;
IsolateWrapper isolateWrapper_;
std::shared_ptr<v8::Persistent<v8::Function>> callback_;
std::shared_ptr<v8::Persistent<v8::Object>> implementationObject_;
const PropertyMeta* meta_;
Expand Down
130 changes: 70 additions & 60 deletions NativeScript/runtime/ClassBuilder.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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> context = Caches::Get(isolate)->GetContext();
Expand Down Expand Up @@ -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> value = it->second->Get(isolate);
BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) {
ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(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> value = it->second->Get(isolate);
BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) {
ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
objcWrapper->GcProtect();
}
}
};
if (CFRunLoopGetCurrent() != runtimeLoop) {
// bare entry: the closure does its own Locker ceremony
std::shared_ptr<EventLoop> 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();
}
}

Expand All @@ -352,51 +363,34 @@ 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> value = it->second->Get(isolate);
BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) {
ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
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> value = it->second->Get(isolate);
BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) {
ObjCDataWrapper* objcWrapper = static_cast<ObjCDataWrapper*>(wrapper);
objcWrapper->GcUnprotect();
}
};
if (CFRunLoopGetCurrent() != runtimeLoop) {
// bare entry: the closure does its own Locker ceremony
std::shared_ptr<EventLoop> loop =
NativeScriptPlatform::Instance()->LookupEventLoop(isolate);
if (loop != nullptr) {
loop->PostInternalBare(gcUnprotect);
}
} else {
gcUnprotect();
}
}
}
Expand Down Expand Up @@ -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<v8::Function> getterFunc = context->callback_->Get(isolate);
Expand Down Expand Up @@ -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<v8::Function> setterFunc = context->callback_->Get(isolate);
Expand Down
Loading
Loading