From dccd5fcb9d87fc7a06b597b49944141128df77ad Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Fri, 13 Nov 2026 15:55:01 -0300 Subject: [PATCH 1/4] fix(runtime): release wrappers made for JS methods called from native code When native code called a JS-implemented method on an object JS had no wrapper for yet (a JS initializer run by +new or by UIKit, as with @nativescript/core's ListView cells), MethodCallback wrapped `self` with a strong, unregistered handle and a retain that were never released: the object, its wrapper and everything its JS state referenced leaked. The wrapper is now registered like any other, but its handle is held strong while something besides the Instances entry retains the object and made weak once that entry is the only owner. That is reconciled from the retain/release swizzle of TypeScript-extended classes and, for every class, when a major GC starts. Until native code lets go the wrapper behaves as before (a strong root, so its JS state is kept); afterwards it is collected and the object released. An object reached from its own -dealloc gets a wrapper that is neither cached nor retaining. Related leaks fixed alongside: - Owned returns (new/alloc/copy families) that hit a cached wrapper now give back their +1, as the uncached path and ConstructObject already did; +new on a class with a JS initializer kept an extra retain. - The nativeException wrapper attached to errors built from an NSError or NSException was strong and never released; it is registered now. - ~NativeScriptException deleted its Persistent without resetting it, which leaks the V8 handle: every JS error thrown through it, and what it references, stayed alive. --- NativeScript/runtime/ArgConverter.h | 3 + NativeScript/runtime/ArgConverter.mm | 63 +++++- NativeScript/runtime/ClassBuilder.h | 4 +- NativeScript/runtime/ClassBuilder.mm | 35 ++-- NativeScript/runtime/DataWrapper.h | 8 + NativeScript/runtime/Interop.mm | 14 +- NativeScript/runtime/NativeScriptException.mm | 8 +- NativeScript/runtime/ObjectManager.h | 9 + NativeScript/runtime/ObjectManager.mm | 101 ++++++++- .../app/tests/NativeCallbackSelfTests.js | 194 ++++++++++++++++++ TestRunner/app/tests/index.js | 1 + 11 files changed, 412 insertions(+), 28 deletions(-) create mode 100644 TestRunner/app/tests/NativeCallbackSelfTests.js diff --git a/NativeScript/runtime/ArgConverter.h b/NativeScript/runtime/ArgConverter.h index 9fa21ee4d..6afe7330a 100644 --- a/NativeScript/runtime/ArgConverter.h +++ b/NativeScript/runtime/ArgConverter.h @@ -54,6 +54,9 @@ class ArgConverter { std::vector()); static std::shared_ptr> CreateEmptyObject( v8::Local context, bool skipGCRegistration = false); + // The JS `this` for native code calling a JS-implemented method on an + // object JS has no wrapper for yet. + static v8::Local WrapNativeCallbackSelf(v8::Local context, id self); static const Meta* FindMeta(Class klass, const TypeEncoding* typeEncoding = nullptr); // Looks up the JS wrapper cached for `target` in Caches::Instances, dropping diff --git a/NativeScript/runtime/ArgConverter.mm b/NativeScript/runtime/ArgConverter.mm index d5464db87..b7c9fe424 100644 --- a/NativeScript/runtime/ArgConverter.mm +++ b/NativeScript/runtime/ArgConverter.mm @@ -12,11 +12,40 @@ #include "Runtime.h" #include "RuntimeConfig.h" +// Exported by libobjc; not declared in a public header for MRC code. +extern "C" id objc_initWeakOrNil(id* location, id value); +extern "C" void objc_destroyWeak(id* location); + using namespace v8; using namespace std; namespace tns { +namespace { + +// Also true for instances of the rare classes that refuse weak references. +bool IsDeallocating(id obj) { + id slot = nil; + objc_initWeakOrNil(&slot, obj); + bool deallocating = slot == nil; + objc_destroyWeak(&slot); + return deallocating; +} + +struct TransientSelf { + Global handle; + ObjCDataWrapper* wrapper; +}; + +void DisposeTransientSelf(const WeakCallbackInfo& info) { + TransientSelf* transient = info.GetParameter(); + transient->handle.Reset(); + delete transient->wrapper; + delete transient; +} + +} // namespace + void ArgConverter::Init(Local context) { Isolate* isolate = v8::Isolate::GetCurrent(); auto cache = Caches::Get(isolate); @@ -358,9 +387,7 @@ if (it != cache->Instances.end()) { thiz = it->second->Get(data->isolateWrapper_.Isolate()).As(); } else { - ObjCDataWrapper* wrapper = new ObjCDataWrapper(self_); - thiz = ArgConverter::CreateJsWrapper(context, wrapper, Local(), true).As(); - tns::DeleteWrapperIfUnused(isolate, thiz, wrapper); + thiz = ArgConverter::WrapNativeCallbackSelf(context, self_); } } @@ -1069,6 +1096,36 @@ return protocolMeta; } +Local ArgConverter::WrapNativeCallbackSelf(Local context, id self) { + Isolate* isolate = v8::Isolate::GetCurrent(); + auto cache = Caches::Get(isolate); + ObjCDataWrapper* wrapper = new ObjCDataWrapper(self); + + if (IsDeallocating(self)) { + // Called from -dealloc: a retain cannot keep the object alive, so the + // wrapper is neither cached nor owning. + Local receiver = NewEmptyInstance(context, cache->EmptyObjCtorFunc.get()); + CreateJsWrapper(context, wrapper, receiver); + auto* transient = new TransientSelf{Global(isolate, receiver), wrapper}; + transient->handle.SetWeak(transient, DisposeTransientSelf, WeakCallbackType::kParameter); + return receiver; + } + + Local result = CreateJsWrapper(context, wrapper, Local()).As(); + if (tns::DeleteWrapperIfUnused(isolate, result, wrapper)) { + return result; + } + // Native code usually still holds the object (it is mid-initialization or + // owned by whoever called in), and its JS state must outlive this call: the + // handle stays strong until the Instances entry is the object's only owner. + auto it = cache->Instances.find(self); + if (it != cache->Instances.end() && it->second->IsWeak()) { + ObjectManager::ManageOwnership(isolate, wrapper, + it->second->ClearWeak()); + } + return result; +} + std::shared_ptr> ArgConverter::CreateEmptyObject(Local context, bool skipGCRegistration) { Isolate* isolate = v8::Isolate::GetCurrent(); diff --git a/NativeScript/runtime/ClassBuilder.h b/NativeScript/runtime/ClassBuilder.h index 8ab4a9f6a..730649a12 100644 --- a/NativeScript/runtime/ClassBuilder.h +++ b/NativeScript/runtime/ClassBuilder.h @@ -37,6 +37,9 @@ class ClassBuilder { v8::Local context); static std::string GetTypeEncoding(const TypeEncoding* typeEncoding, int argsCount); + // The implementation of `method` from the nearest superclass not defined + // from JS. + static IMP FindNotOverridenMethod(Class klass, SEL method); private: static std::atomic classNameCounter_; @@ -72,7 +75,6 @@ class ClassBuilder { static std::string GetTypeEncoding(const TypeEncoding* typeEncoding); static BinaryTypeEncodingType GetTypeEncodingType(v8::Isolate* isolate, v8::Local value); - static IMP FindNotOverridenMethod(Class klass, SEL method); struct CacheItem { public: diff --git a/NativeScript/runtime/ClassBuilder.mm b/NativeScript/runtime/ClassBuilder.mm index ce792f4aa..ffbcd0a00 100644 --- a/NativeScript/runtime/ClassBuilder.mm +++ b/NativeScript/runtime/ClassBuilder.mm @@ -332,7 +332,11 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { BaseDataWrapper* wrapper = tns::GetValue(isolate, value); if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) { ObjCDataWrapper* objcWrapper = static_cast(wrapper); - objcWrapper->GcProtect(); + if (objcWrapper->OwnershipState() != nullptr) { + ObjectManager::ReconcileOwnership(isolate, objcWrapper); + } else { + objcWrapper->GcProtect(); + } } } }; @@ -341,7 +345,10 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { // like the performed block it replaces runtime->GetEventLoop()->PostInternalBare(gcProtect); } else { + // After the retain: reconciling reads the new retain count. + id result = retain(self, @selector(retain)); gcProtect(); + return result; } } @@ -371,7 +378,11 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { BaseDataWrapper* wrapper = tns::GetValue(isolate, value); if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) { ObjCDataWrapper* objcWrapper = static_cast(wrapper); - objcWrapper->GcUnprotect(); + if (objcWrapper->OwnershipState() != nullptr) { + ObjectManager::ReconcileOwnership(isolate, objcWrapper); + } else { + objcWrapper->GcUnprotect(); + } } } } @@ -383,21 +394,11 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { // 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(); - } - } - } + // After the release: reconciling reads the new retain count, and + // the Instances entry still holds the object. + release(self, @selector(release)); + gcUnprotect(); + return; } } diff --git a/NativeScript/runtime/DataWrapper.h b/NativeScript/runtime/DataWrapper.h index a98655a54..6b19d2232 100644 --- a/NativeScript/runtime/DataWrapper.h +++ b/NativeScript/runtime/DataWrapper.h @@ -416,8 +416,16 @@ class ObjCDataWrapper : public BaseDataWrapper { // anything keyed on the raw pointer needs this to detect a recycled slot. Class Klass() { return this->klass_; } + // Set for a wrapper whose handle ObjectManager::ReconcileOwnership keeps + // strong while native code also retains the object, and weak once the + // Instances entry is its only owner. The handle's finalizer state, needed to + // re-arm it after a strong period. + ObjectWeakCallbackState* OwnershipState() { return this->ownershipState_; } + void SetOwnershipState(ObjectWeakCallbackState* state) { this->ownershipState_ = state; } + private: bool adapterClaim_ = false; + ObjectWeakCallbackState* ownershipState_ = nullptr; id data_; const tns::TypeEncoding* typeEncoding_; Class klass_; diff --git a/NativeScript/runtime/Interop.mm b/NativeScript/runtime/Interop.mm index 196714558..95ee23545 100644 --- a/NativeScript/runtime/Interop.mm +++ b/NativeScript/runtime/Interop.mm @@ -1363,6 +1363,12 @@ static bool SnapshotArrayLike(Isolate* isolate, Local object, LocalGet(isolate); } @@ -1867,8 +1873,8 @@ throw NativeScriptException( } ObjCDataWrapper* wrapper = new ObjCDataWrapper((id)e); - Local nativeWrapper = - ArgConverter::CreateJsWrapper(context, wrapper, Local(), true); + Local nativeWrapper = ArgConverter::CreateJsWrapper(context, wrapper, Local()); + tns::DeleteWrapperIfUnused(isolate, nativeWrapper, wrapper); jsErrObj->Set(context, tns::ToV8String(isolate, "nativeException"), nativeWrapper) .FromMaybe(false); @@ -1911,8 +1917,8 @@ throw NativeScriptException( // Wrap the native NSError instance into a JS object and attach as nativeException ObjCDataWrapper* wrapper = new ObjCDataWrapper(error); - Local nativeWrapper = - ArgConverter::CreateJsWrapper(context, wrapper, Local(), true); + Local nativeWrapper = ArgConverter::CreateJsWrapper(context, wrapper, Local()); + tns::DeleteWrapperIfUnused(isolate, nativeWrapper, wrapper); jsErrObj->Set(context, tns::ToV8String(isolate, "nativeException"), nativeWrapper) .FromMaybe(false); diff --git a/NativeScript/runtime/NativeScriptException.mm b/NativeScript/runtime/NativeScriptException.mm index 0a3fe5b30..10bb3dcf7 100644 --- a/NativeScript/runtime/NativeScriptException.mm +++ b/NativeScript/runtime/NativeScriptException.mm @@ -124,7 +124,13 @@ static void ConsiderStackCandidate(PendingErrorDisplay& state, v8::Isolate* isol this->name_ = "NativeScriptException"; } -NativeScriptException::~NativeScriptException() { delete this->javascriptException_; } +NativeScriptException::~NativeScriptException() { + // Deleting a Persistent does not release its V8 handle. + if (this->javascriptException_ != nullptr) { + this->javascriptException_->Reset(); + delete this->javascriptException_; + } +} void NativeScriptException::OnUncaughtError(Local message, Local error) { @try { diff --git a/NativeScript/runtime/ObjectManager.h b/NativeScript/runtime/ObjectManager.h index 67691ff32..66c65145e 100644 --- a/NativeScript/runtime/ObjectManager.h +++ b/NativeScript/runtime/ObjectManager.h @@ -6,6 +6,7 @@ namespace tns { class ObjectManager; +class ObjCDataWrapper; // Parameter of the kFinalizer weak callback armed on target_. // @@ -53,6 +54,14 @@ class ObjectManager { // Disposes every handle from Register() that is still alive. Replaces the // old Isolate::VisitHandlesWithClassIds walk, which V8 removed. static void DisposeAllRegistered(v8::Isolate* isolate); + // Takes over the handle `state` belongs to (the wrapper's Instances entry): + // from now on it is strong while something besides that entry retains the + // object and weak otherwise, re-checked whenever ReconcileOwnership runs and + // when a major collection starts. + static void ManageOwnership(v8::Isolate* isolate, ObjCDataWrapper* wrapper, + ObjectWeakCallbackState* state); + // Runs on the isolate's thread with the isolate entered. + static void ReconcileOwnership(v8::Isolate* isolate, ObjCDataWrapper* wrapper); private: static void ReleaseNativeCounterpartCallback( diff --git a/NativeScript/runtime/ObjectManager.mm b/NativeScript/runtime/ObjectManager.mm index 904ab2333..ef3da4502 100644 --- a/NativeScript/runtime/ObjectManager.mm +++ b/NativeScript/runtime/ObjectManager.mm @@ -2,7 +2,9 @@ #include #include #include +#include #include "Caches.h" +#include "ClassBuilder.h" #include "Constants.h" #include "DataWrapper.h" #include "FFICall.h" @@ -15,9 +17,63 @@ static Class NSTimerClass = objc_getClass("NSTimer"); +namespace { + +// Wrappers whose handle ObjectManager::ManageOwnership took over, keyed by the +// finalizer state their handle carries. +struct OwnershipTracking { + std::unordered_map managed; +}; + +void ForgetOwnership(Isolate* isolate, ObjectWeakCallbackState* state) { + if (OwnershipTracking* tracking = Caches::StateFor(isolate)) { + tracking->managed.erase(state); + } +} + +void ApplyOwnership(ObjectWeakCallbackState* state, bool nativelyOwned) { + bool weak = state->target_->IsWeak(); + if (nativelyOwned && weak) { + state->target_->ClearWeak(); + } else if (!nativelyOwned && !weak) { + state->target_->SetWeak(state, ObjectManager::FinalizerCallback, + WeakCallbackType::kFinalizer); + } +} + +// The GC prologue may not enter JS, so a JS-implemented -retainCount is +// skipped. Matches ObjectManager::GetRetainCount, which counts timers as unowned. +bool NativelyOwnedWithoutJS(id obj) { + for (Class c = object_getClass(obj); c != nil; c = class_getSuperclass(c)) { + if (c == NSTimerClass) { + return false; + } + } + IMP imp = ClassBuilder::FindNotOverridenMethod(object_getClass(obj), @selector(retainCount)); + return ((NSUInteger(*)(id, SEL))imp)(obj, @selector(retainCount)) > 1; +} + +void ReconcileOwnershipBeforeGC(Isolate* isolate, GCType, GCCallbackFlags) { + OwnershipTracking* tracking = Caches::StateFor(isolate); + if (tracking == nullptr) { + return; + } + for (auto& [state, wrapper] : tracking->managed) { + if (state->disposing_ || state->target_ == nullptr || state->target_->IsEmpty()) { + continue; + } + ApplyOwnership(state, NativelyOwnedWithoutJS(wrapper->Data())); + } +} + +} // namespace + void ObjectManager::Init(Isolate* isolate, Local globalTemplate) { globalTemplate->Set(tns::ToV8String(isolate, "__releaseNativeCounterpart"), FunctionTemplate::New(isolate, ReleaseNativeCounterpartCallback)); + isolate->AddGCPrologueCallback( + ReconcileOwnershipBeforeGC, + static_cast(kGCTypeMarkSweepCompact | kGCTypeIncrementalMarking)); } namespace { @@ -108,6 +164,10 @@ void DisposeHandle(v8::Isolate* isolate, // claim every state up front: disposing one entry can re-enter JS from // -dealloc and reach __releaseNativeCounterpart for any other entry this // walk still owns. + if (OwnershipTracking* tracking = Caches::StateFor(isolate)) { + tracking->managed.clear(); + } + ObjectWeakCallbackState* state = cache->ObjectManagedValues; cache->ObjectManagedValues = nullptr; for (ObjectWeakCallbackState* claimed = state; claimed != nullptr; claimed = claimed->next_) { @@ -139,6 +199,29 @@ void DisposeHandle(v8::Isolate* isolate, DisposeHandle(isolate, cache->FunctionReferenceCtorFunc); } +void ObjectManager::ManageOwnership(Isolate* isolate, ObjCDataWrapper* wrapper, + ObjectWeakCallbackState* state) { + OwnershipTracking* tracking = Caches::StateFor(isolate); + if (tracking == nullptr || state == nullptr) { + return; + } + wrapper->SetOwnershipState(state); + tracking->managed[state] = wrapper; + ObjectManager::ReconcileOwnership(isolate, wrapper); +} + +void ObjectManager::ReconcileOwnership(Isolate* isolate, ObjCDataWrapper* wrapper) { + ObjectWeakCallbackState* state = wrapper->OwnershipState(); + if (state == nullptr || state->disposing_ || state->target_ == nullptr || + state->target_->IsEmpty()) { + return; + } + // The handle's strength carries the ownership; a GcProtect from the retain + // swizzle would keep a weak handle from ever being disposed. + wrapper->GcUnprotect(); + ApplyOwnership(state, ObjectManager::GetRetainCount(wrapper->Data()) > 1); +} + void ObjectManager::FinalizerCallback(const WeakCallbackInfo& data) { ObjectWeakCallbackState* state = data.GetParameter(); Isolate* isolate = data.GetIsolate(); @@ -167,6 +250,7 @@ void DisposeHandle(v8::Isolate* isolate, if (disposed || state->target_->IsEmpty()) { state->target_->Reset(); UnlinkRegistered(state); + ForgetOwnership(isolate, state); delete state; return; } @@ -200,6 +284,17 @@ void DisposeHandle(v8::Isolate* isolate, return false; } + // A handle ReconcileOwnership keeps is weak while the Instances entry looked + // like the only owner; a retain from another thread can still be on its way + // to making it strong again. + if (!isFinalDisposal && wrapper->Type() == WrapperType::ObjCObject) { + ObjCDataWrapper* objcWrapper = static_cast(wrapper); + if (objcWrapper->OwnershipState() != nullptr && + ObjectManager::GetRetainCount(objcWrapper->Data()) > 1) { + return false; + } + } + std::shared_ptr cache = Caches::Get(isolate); switch (wrapper->Type()) { case WrapperType::Struct: @@ -356,8 +451,9 @@ void DisposeHandle(v8::Isolate* isolate, auto it = cache->Instances.find(data); if (it != cache->Instances.end()) { std::shared_ptr> handle = it->second; - ObjectWeakCallbackState* state = - handle->IsWeak() ? handle->ClearWeak() : nullptr; + ObjectWeakCallbackState* state = handle->IsWeak() + ? handle->ClearWeak() + : objcWrapper->OwnershipState(); if (state != nullptr && state->disposing_) { // Reached from a -dealloc running inside this handle's own finalizer: // that frame already released the native counterpart and owns the @@ -372,6 +468,7 @@ void DisposeHandle(v8::Isolate* isolate, // parameter. handle->Reset(); UnlinkRegistered(state); + ForgetOwnership(isolate, state); delete state; } } diff --git a/TestRunner/app/tests/NativeCallbackSelfTests.js b/TestRunner/app/tests/NativeCallbackSelfTests.js new file mode 100644 index 000000000..d43808854 --- /dev/null +++ b/TestRunner/app/tests/NativeCallbackSelfTests.js @@ -0,0 +1,194 @@ +// Wrappers created for objects that native code hands to JS-implemented +// methods, and the native objects attached to errors: kept while native code +// owns them, released exactly once afterwards. +describe("Native callback self wrappers", function () { + function scrub(n) { + return n > 0 ? scrub(n - 1) + n : 0; + } + + // Collect, then give autorelease pools and posted retain/release work a + // turn of the run loop. + function settle(rounds, callback) { + scrub(300); + __collect(); + __collect(); + setTimeout(function () { + NSObject.new(); + if (rounds > 1) { + settle(rounds - 1, callback); + } else { + callback(); + } + }, 10); + } + + function deallocs() { + return (TNSGetOutput().match(/TNSAllocLog dealloc/g) || []).length; + } + + var initCalls = 0; + var JSInit = (function (_super) { + __extends(JSInit, _super); + function JSInit() { + return (_super !== null && _super.apply(this, arguments)) || this; + } + JSInit.prototype.init = function () { + var self = _super.prototype.init.call(this); + initCalls++; + self.initialized = true; + return self; + }; + return JSInit; + })(TNSAllocLog); + + beforeEach(function () { + TNSClearOutput(); + initCalls = 0; + }); + + // +new runs the JS-implemented init from native code, before JS has a + // wrapper for the object. + it("keeps the JS state of a natively owned object whose JS initializer created its wrapper, then frees it once", function (done) { + var owner = NSMutableArray.new(); + (function () { + var obj = JSInit.new(); + obj.state = { i: 1 }; + owner.addObject(obj); + })(); + settle(3, function () { + var obj = owner.objectAtIndex(0); + expect(initCalls).toBe(1); + expect(obj.initialized).toBe(true); + expect(obj.state && obj.state.i).toBe(1); + obj = null; + expect(deallocs()).toBe(0); + owner.removeAllObjects(); + settle(3, function () { + expect(deallocs()).toBe(1); + done(); + }); + }); + }); + + it("frees an object JS alone holds after +new ran its JS initializer", function (done) { + var obj = JSInit.new(); + obj.state = { i: 2 }; + settle(2, function () { + expect(obj.state.i).toBe(2); + expect(deallocs()).toBe(0); + obj = null; + settle(3, function () { + expect(deallocs()).toBe(1); + done(); + }); + }); + }); + + it("keeps the JS state of cells UIKit creates from a registered class until their superview lets them go", function (done) { + var cellInits = 0; + var Cell = UITableViewCell.extend({ + initWithStyleReuseIdentifier: function (style, reuseIdentifier) { + var cell = UITableViewCell.prototype.initWithStyleReuseIdentifier.call(this, style, reuseIdentifier); + cellInits++; + cell.fromInitializer = true; + return cell; + } + }, { name: "NativeCallbackSelfCell" }); + var watch = NSHashTable.weakObjectsHashTable(); + var host = UIView.new(); + (function () { + var table = UITableView.new(); + table.registerClassForCellReuseIdentifier(Cell.class(), "cell"); + for (var i = 0; i < 4; i++) { + var cell = table.dequeueReusableCellWithIdentifier("cell"); + cell.owner = { row: i }; + host.addSubview(cell); + watch.addObject(cell); + } + })(); + settle(3, function () { + expect(cellInits).toBe(4); + var subviews = host.subviews; + expect(subviews.count).toBe(4); + for (var i = 0; i < subviews.count; i++) { + var cell = subviews.objectAtIndex(i); + expect(cell.fromInitializer).toBe(true); + expect(cell.owner && cell.owner.row).toBe(i); + } + subviews = null; + cell = null; + (function () { + var views = host.subviews; + for (var j = views.count - 1; j >= 0; j--) { + views.objectAtIndex(j).removeFromSuperview(); + } + })(); + settle(4, function () { + expect(host.subviews.count).toBe(0); + expect(watch.allObjects.count).toBe(0); + done(); + }); + }); + }); + + it("balances owned returns that hand back an object JS already wraps", function (done) { + var array = NSArray.arrayWithArray([1, 2, 3]); + settle(2, function () { + var before = array.retainCount(); + for (var i = 0; i < 50; i++) { + var copy = array.copy(); + expect(copy).toBe(array); + } + copy = null; + settle(2, function () { + expect(array.retainCount()).toBe(before); + expect(array.count).toBe(3); + done(); + }); + }); + }); + + it("keeps an error's NSError while the error lives and frees it afterwards", function (done) { + var watch = NSHashTable.weakObjectsHashTable(); + var error = null; + try { + NSFileManager.defaultManager.contentsOfDirectoryAtPathError("/nonexistent-native-callback-self"); + } catch (e) { + error = e; + } + expect(error).not.toBeNull(); + expect(error.nativeException).toBeDefined(); + watch.addObject(error.nativeException); + settle(3, function () { + expect(watch.allObjects.count).toBe(1); + expect(error.nativeException.code).toBe(error.code); + error = null; + settle(3, function () { + expect(watch.allObjects.count).toBe(0); + done(); + }); + }); + }); + + it("keeps an error's NSException while the error lives and frees it afterwards", function (done) { + var watch = NSHashTable.weakObjectsHashTable(); + var error = null; + try { + NSArray.array().objectAtIndex(3); + } catch (e) { + error = e; + } + expect(error).not.toBeNull(); + expect(error.nativeException).toBeDefined(); + watch.addObject(error.nativeException); + settle(3, function () { + expect(watch.allObjects.count).toBe(1); + expect(error.nativeException.name).toBe("NSRangeException"); + error = null; + settle(3, function () { + expect(watch.allObjects.count).toBe(0); + done(); + }); + }); + }); +}); diff --git a/TestRunner/app/tests/index.js b/TestRunner/app/tests/index.js index 9c9ce44f4..96de9c100 100644 --- a/TestRunner/app/tests/index.js +++ b/TestRunner/app/tests/index.js @@ -134,6 +134,7 @@ require("./NsWorkerThreadsTests"); require("./NsUrlTests"); require("./NsInteropTests"); require("./GCFinalizerTests"); +require("./NativeCallbackSelfTests"); require("./BlockCacheRaceTests"); require("./WorkerConcurrentStartupTests"); require("./WorkerOptionsTests"); From 01082bc8d3047f874a0787eb0239ec2c13a5e490 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Fri, 13 Nov 2026 16:52:32 -0300 Subject: [PATCH 2/4] fix(runtime): address review of the self-wrap leak fix - Release a cached owned return's +1 only for class methods and C functions. An instance method can return its receiver without a net retain (NS_REPLACES_RECEIVER, e.g. -awakeAfterUsingCoder:) or come from a JS implementation that returns at +0, and releasing those freed live objects. - Wrap a deallocating self in a borrowed wrapper that MethodCallback detaches when the call returns, instead of one freed by a weak callback: a `this` JS keeps reads as released rather than pointing at the freed object, nothing is left behind at teardown, and __releaseNativeCounterpart neither releases it nor frees it twice. Deallocation is detected through -allowsWeakReference. - __releaseNativeCounterpart retires a managed wrapper's own handle, whatever Instances maps its object to. - ~NativeScriptException releases its handle under the isolate's Locker when the current thread does not hold it (event loop tasks catch after their Locker is gone), and only while the isolate is alive. - Reconcile with the same non-JS retain count everywhere, keep a refused managed handle strong instead of re-arming it weak, and post the off-thread reconcile after the real retain/release. --- NativeScript/runtime/ArgConverter.h | 7 +- NativeScript/runtime/ArgConverter.mm | 54 ++++++----- NativeScript/runtime/ClassBuilder.mm | 14 +-- NativeScript/runtime/DataWrapper.h | 6 ++ NativeScript/runtime/Interop.h | 3 +- NativeScript/runtime/Interop.mm | 17 ++-- NativeScript/runtime/NativeScriptException.h | 1 + NativeScript/runtime/NativeScriptException.mm | 21 +++- NativeScript/runtime/ObjectManager.mm | 48 +++++++--- .../app/tests/NativeCallbackSelfTests.js | 95 ++++++++++++++++--- 10 files changed, 194 insertions(+), 72 deletions(-) diff --git a/NativeScript/runtime/ArgConverter.h b/NativeScript/runtime/ArgConverter.h index 6afe7330a..6a09203aa 100644 --- a/NativeScript/runtime/ArgConverter.h +++ b/NativeScript/runtime/ArgConverter.h @@ -55,8 +55,11 @@ class ArgConverter { static std::shared_ptr> CreateEmptyObject( v8::Local context, bool skipGCRegistration = false); // The JS `this` for native code calling a JS-implemented method on an - // object JS has no wrapper for yet. - static v8::Local WrapNativeCallbackSelf(v8::Local context, id self); + // object JS has no wrapper for yet. For an object already deallocating it + // is a borrowed wrapper, stored in `borrowed`, that the caller must detach + // and delete once the call returns. + static v8::Local WrapNativeCallbackSelf(v8::Local context, id self, + ObjCDataWrapper** borrowed); static const Meta* FindMeta(Class klass, const TypeEncoding* typeEncoding = nullptr); // Looks up the JS wrapper cached for `target` in Caches::Instances, dropping diff --git a/NativeScript/runtime/ArgConverter.mm b/NativeScript/runtime/ArgConverter.mm index b7c9fe424..649c23f78 100644 --- a/NativeScript/runtime/ArgConverter.mm +++ b/NativeScript/runtime/ArgConverter.mm @@ -12,10 +12,6 @@ #include "Runtime.h" #include "RuntimeConfig.h" -// Exported by libobjc; not declared in a public header for MRC code. -extern "C" id objc_initWeakOrNil(id* location, id value); -extern "C" void objc_destroyWeak(id* location); - using namespace v8; using namespace std; @@ -23,27 +19,32 @@ namespace { -// Also true for instances of the rare classes that refuse weak references. +// -allowsWeakReference answers NO once deallocation has begun, and for the +// rare classes that refuse weak references altogether. bool IsDeallocating(id obj) { - id slot = nil; - objc_initWeakOrNil(&slot, obj); - bool deallocating = slot == nil; - objc_destroyWeak(&slot); - return deallocating; + static SEL allowsWeakReference = sel_registerName("allowsWeakReference"); + if (![obj respondsToSelector:allowsWeakReference]) { + return false; + } + return !((BOOL (*)(id, SEL))objc_msgSend)(obj, allowsWeakReference); } -struct TransientSelf { - Global handle; - ObjCDataWrapper* wrapper; +// The borrowed wrapper WrapNativeCallbackSelf made for an object in -dealloc +// is detached once the native call returns: the object is gone afterwards, +// so a receiver JS kept must read as released. +struct BorrowedSelfScope { + explicit BorrowedSelfScope(Isolate* isolate) : isolate(isolate) {} + ~BorrowedSelfScope() { + if (wrapper != nullptr && tns::GetValue(isolate, receiver) == wrapper) { + tns::SetValue(isolate, receiver, nullptr); + delete wrapper; + } + } + Isolate* isolate; + Local receiver; + ObjCDataWrapper* wrapper = nullptr; }; -void DisposeTransientSelf(const WeakCallbackInfo& info) { - TransientSelf* transient = info.GetParameter(); - transient->handle.Reset(); - delete transient->wrapper; - delete transient; -} - } // namespace void ArgConverter::Init(Local context) { @@ -381,13 +382,15 @@ void DisposeTransientSelf(const WeakCallbackInfo& info) { } Local thiz = context->Global(); + BorrowedSelfScope borrowedSelf(isolate); if (data->initialParamIndex_ > 1) { id self_ = *static_cast(argValues[0]); auto it = cache->Instances.find(self_); if (it != cache->Instances.end()) { thiz = it->second->Get(data->isolateWrapper_.Isolate()).As(); } else { - thiz = ArgConverter::WrapNativeCallbackSelf(context, self_); + thiz = ArgConverter::WrapNativeCallbackSelf(context, self_, &borrowedSelf.wrapper); + borrowedSelf.receiver = thiz; } } @@ -1096,18 +1099,19 @@ void DisposeTransientSelf(const WeakCallbackInfo& info) { return protocolMeta; } -Local ArgConverter::WrapNativeCallbackSelf(Local context, id self) { +Local ArgConverter::WrapNativeCallbackSelf(Local context, id self, + ObjCDataWrapper** borrowed) { Isolate* isolate = v8::Isolate::GetCurrent(); auto cache = Caches::Get(isolate); ObjCDataWrapper* wrapper = new ObjCDataWrapper(self); if (IsDeallocating(self)) { // Called from -dealloc: a retain cannot keep the object alive, so the - // wrapper is neither cached nor owning. + // wrapper is neither cached nor owning, and the caller detaches it. + wrapper->MarkBorrowed(); Local receiver = NewEmptyInstance(context, cache->EmptyObjCtorFunc.get()); CreateJsWrapper(context, wrapper, receiver); - auto* transient = new TransientSelf{Global(isolate, receiver), wrapper}; - transient->handle.SetWeak(transient, DisposeTransientSelf, WeakCallbackType::kParameter); + *borrowed = wrapper; return receiver; } diff --git a/NativeScript/runtime/ClassBuilder.mm b/NativeScript/runtime/ClassBuilder.mm index ffbcd0a00..e56a20ae4 100644 --- a/NativeScript/runtime/ClassBuilder.mm +++ b/NativeScript/runtime/ClassBuilder.mm @@ -340,16 +340,16 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { } } }; + // After the retain: reconciling reads the new retain count. + id result = retain(self, @selector(retain)); if (CFRunLoopGetCurrent() != runtimeLoop) { // bare entry: the closure does its own Locker ceremony, exactly // like the performed block it replaces runtime->GetEventLoop()->PostInternalBare(gcProtect); } else { - // After the retain: reconciling reads the new retain count. - id result = retain(self, @selector(retain)); gcProtect(); - return result; } + return result; } return retain(self, @selector(retain)); @@ -389,17 +389,17 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { }; auto runtime = Runtime::GetRuntime(isolate); auto runtimeLoop = runtime->RuntimeLoop(); + // After the release: reconciling reads the new retain count, and + // the Instances entry still holds the object. + release(self, @selector(release)); if (CFRunLoopGetCurrent() != runtimeLoop) { // bare entry: the closure does its own Locker ceremony, exactly // like the performed block it replaces runtime->GetEventLoop()->PostInternalBare(gcUnprotect); } else { - // After the release: reconciling reads the new retain count, and - // the Instances entry still holds the object. - release(self, @selector(release)); gcUnprotect(); - return; } + return; } release(self, @selector(release)); diff --git a/NativeScript/runtime/DataWrapper.h b/NativeScript/runtime/DataWrapper.h index 6b19d2232..7b2b2c591 100644 --- a/NativeScript/runtime/DataWrapper.h +++ b/NativeScript/runtime/DataWrapper.h @@ -423,8 +423,14 @@ class ObjCDataWrapper : public BaseDataWrapper { ObjectWeakCallbackState* OwnershipState() { return this->ownershipState_; } void SetOwnershipState(ObjectWeakCallbackState* state) { this->ownershipState_ = state; } + // A wrapper that holds no reference to its object (one made for an object + // already deallocating), so nothing may release on its behalf. + bool IsBorrowed() { return this->borrowed_; } + void MarkBorrowed() { this->borrowed_ = true; } + private: bool adapterClaim_ = false; + bool borrowed_ = false; ObjectWeakCallbackState* ownershipState_ = nullptr; id data_; const tns::TypeEncoding* typeEncoding_; diff --git a/NativeScript/runtime/Interop.h b/NativeScript/runtime/Interop.h index 085bb416f..9dcfed691 100644 --- a/NativeScript/runtime/Interop.h +++ b/NativeScript/runtime/Interop.h @@ -107,7 +107,8 @@ class Interop { BaseCall* call, bool marshalToPrimitive, v8::Local parentStruct = v8::Local(), bool isStructMember = false, bool ownsReturnedObject = false, - bool returnsUnmanaged = false, bool isInitializer = false); + bool returnsUnmanaged = false, bool isInitializer = false, + bool releaseCachedOwnedReturn = false); static void SetStructPropertyValue(v8::Local context, StructWrapper* wrapper, const StructField& field, diff --git a/NativeScript/runtime/Interop.mm b/NativeScript/runtime/Interop.mm index 95ee23545..9c755cde4 100644 --- a/NativeScript/runtime/Interop.mm +++ b/NativeScript/runtime/Interop.mm @@ -1029,7 +1029,7 @@ static bool SnapshotArrayLike(Isolate* isolate, Local object, Local Interop::GetResult(Local context, const TypeEncoding* typeEncoding, BaseCall* call, bool marshalToPrimitive, Local parentStruct, bool isStructMember, bool ownsReturnedObject, bool returnsUnmanaged, - bool isInitializer) { + bool isInitializer, bool releaseCachedOwnedReturn) { Isolate* isolate = v8::Isolate::GetCurrent(); if (returnsUnmanaged) { @@ -1363,13 +1363,16 @@ static bool SnapshotArrayLike(Isolate* isolate, Local object, Local cached = poInstance->Get(isolate); + // The Instances entry holds its own reference, so a returned +1 is + // surplus. Only class methods and C functions qualify: an instance + // method can hand back its receiver without a net retain (initializers, + // NS_REPLACES_RECEIVER) or come from a JS implementation, which returns + // at +0. + if (ownsReturnedObject && releaseCachedOwnedReturn) { [result release]; } - return poInstance->Get(isolate); + return cached; } // For NSProxy we will try to read the metadata from @@ -1936,7 +1939,7 @@ throw NativeScriptException( Local result = Interop::GetResult(methodCall.context_, methodCall.typeEncoding_, &call, marshalToPrimitive, Local(), false, methodCall.ownsReturnedObject_, - methodCall.returnsUnmanaged_, methodCall.isInitializer_); + methodCall.returnsUnmanaged_, methodCall.isInitializer_, !isInstanceMethod); // Owned returns, initializers and class factory methods hand out objects JS // most likely holds the last reference to. Instance getters do not: their diff --git a/NativeScript/runtime/NativeScriptException.h b/NativeScript/runtime/NativeScriptException.h index f11b77de2..9928fcdb2 100644 --- a/NativeScript/runtime/NativeScriptException.h +++ b/NativeScript/runtime/NativeScriptException.h @@ -91,6 +91,7 @@ class NativeScriptException { private: v8::Persistent* javascriptException_; + v8::Isolate* isolate_ = nullptr; std::string name_; std::string message_; std::string stackTrace_; diff --git a/NativeScript/runtime/NativeScriptException.mm b/NativeScript/runtime/NativeScriptException.mm index 10bb3dcf7..ee29e0ac1 100644 --- a/NativeScript/runtime/NativeScriptException.mm +++ b/NativeScript/runtime/NativeScriptException.mm @@ -96,6 +96,7 @@ static void ConsiderStackCandidate(PendingErrorDisplay& state, v8::Isolate* isol } Local error = tc.Exception(); this->javascriptException_ = new Persistent(isolate, tc.Exception()); + this->isolate_ = isolate; this->message_ = GetErrorMessage(isolate, error, message); this->stackTrace_ = tns::GetSmartStackTrace(isolate, &tc, error); this->fullMessage_ = GetFullMessage(isolate, tc, this->message_); @@ -112,6 +113,7 @@ static void ConsiderStackCandidate(PendingErrorDisplay& state, v8::Isolate* isol ->Set(context, ToV8String(isolate, "name"), ToV8String(isolate, this->name_)) .FromMaybe(false); this->javascriptException_ = new Persistent(isolate, error); + this->isolate_ = isolate; this->message_ = GetErrorMessage(isolate, error, message); this->stackTrace_ = GetErrorStackTrace(isolate, Exception::GetStackTrace(error)); this->fullMessage_ = @@ -120,16 +122,27 @@ static void ConsiderStackCandidate(PendingErrorDisplay& state, v8::Isolate* isol NativeScriptException::NativeScriptException(Isolate* isolate, Local jsError, const std::string& message) { this->javascriptException_ = new Persistent(isolate, jsError); + this->isolate_ = isolate; this->message_ = message; this->name_ = "NativeScriptException"; } NativeScriptException::~NativeScriptException() { - // Deleting a Persistent does not release its V8 handle. - if (this->javascriptException_ != nullptr) { - this->javascriptException_->Reset(); - delete this->javascriptException_; + if (this->javascriptException_ == nullptr) { + return; + } + // Deleting a Persistent does not release its V8 handle. The exception can + // be caught after the Locker that ran the throwing code is gone (event + // loop tasks), so the handle is released under one. + if (Runtime::IsAlive(this->isolate_)) { + if (v8::Locker::IsLocked(this->isolate_)) { + this->javascriptException_->Reset(); + } else { + v8::Locker locker(this->isolate_); + this->javascriptException_->Reset(); + } } + delete this->javascriptException_; } void NativeScriptException::OnUncaughtError(Local message, Local error) { diff --git a/NativeScript/runtime/ObjectManager.mm b/NativeScript/runtime/ObjectManager.mm index ef3da4502..344603f80 100644 --- a/NativeScript/runtime/ObjectManager.mm +++ b/NativeScript/runtime/ObjectManager.mm @@ -164,10 +164,6 @@ void DisposeHandle(v8::Isolate* isolate, // claim every state up front: disposing one entry can re-enter JS from // -dealloc and reach __releaseNativeCounterpart for any other entry this // walk still owns. - if (OwnershipTracking* tracking = Caches::StateFor(isolate)) { - tracking->managed.clear(); - } - ObjectWeakCallbackState* state = cache->ObjectManagedValues; cache->ObjectManagedValues = nullptr; for (ObjectWeakCallbackState* claimed = state; claimed != nullptr; claimed = claimed->next_) { @@ -219,7 +215,7 @@ void DisposeHandle(v8::Isolate* isolate, // The handle's strength carries the ownership; a GcProtect from the retain // swizzle would keep a weak handle from ever being disposed. wrapper->GcUnprotect(); - ApplyOwnership(state, ObjectManager::GetRetainCount(wrapper->Data()) > 1); + ApplyOwnership(state, NativelyOwnedWithoutJS(wrapper->Data())); } void ObjectManager::FinalizerCallback(const WeakCallbackInfo& data) { @@ -257,6 +253,16 @@ void DisposeHandle(v8::Isolate* isolate, state->target_->ClearWeak(); state->target_->SetWeak(state, FinalizerCallback, WeakCallbackType::kFinalizer); + + // Refused because native code took the object back: keep the handle strong + // rather than resurrecting it at every collection. + BaseDataWrapper* wrapper = tns::GetValue(isolate, value); + if (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject) { + ObjCDataWrapper* objcWrapper = static_cast(wrapper); + if (objcWrapper->OwnershipState() == state) { + ApplyOwnership(state, NativelyOwnedWithoutJS(objcWrapper->Data())); + } + } } bool ObjectManager::DisposeValue(Isolate* isolate, Local value, bool isFinalDisposal) { @@ -289,8 +295,7 @@ void DisposeHandle(v8::Isolate* isolate, // to making it strong again. if (!isFinalDisposal && wrapper->Type() == WrapperType::ObjCObject) { ObjCDataWrapper* objcWrapper = static_cast(wrapper); - if (objcWrapper->OwnershipState() != nullptr && - ObjectManager::GetRetainCount(objcWrapper->Data()) > 1) { + if (objcWrapper->OwnershipState() != nullptr && NativelyOwnedWithoutJS(objcWrapper->Data())) { return false; } } @@ -445,15 +450,36 @@ void DisposeHandle(v8::Isolate* isolate, } ObjCDataWrapper* objcWrapper = static_cast(wrapper); + if (objcWrapper->IsBorrowed()) { + if (tns::GetValue(isolate, value) == wrapper) { + tns::SetValue(isolate, value.As(), nullptr); + delete wrapper; + } + return; + } + id data = objcWrapper->Data(); if (data != nil) { std::shared_ptr cache = Caches::Get(isolate); auto it = cache->Instances.find(data); - if (it != cache->Instances.end()) { + if (ObjectWeakCallbackState* managed = objcWrapper->OwnershipState()) { + // The handle ReconcileOwnership manages is this wrapper's own, whatever + // Instances maps the object to now. + if (managed->disposing_) { + return; + } + if (it != cache->Instances.end() && it->second == managed->target_) { + cache->Instances.erase(it); + } + managed->target_->Reset(); + UnlinkRegistered(managed); + ForgetOwnership(isolate, managed); + delete managed; + objcWrapper->SetOwnershipState(nullptr); + } else if (it != cache->Instances.end()) { std::shared_ptr> handle = it->second; - ObjectWeakCallbackState* state = handle->IsWeak() - ? handle->ClearWeak() - : objcWrapper->OwnershipState(); + ObjectWeakCallbackState* state = + handle->IsWeak() ? handle->ClearWeak() : nullptr; if (state != nullptr && state->disposing_) { // Reached from a -dealloc running inside this handle's own finalizer: // that frame already released the native counterpart and owns the diff --git a/TestRunner/app/tests/NativeCallbackSelfTests.js b/TestRunner/app/tests/NativeCallbackSelfTests.js index d43808854..331e65b95 100644 --- a/TestRunner/app/tests/NativeCallbackSelfTests.js +++ b/TestRunner/app/tests/NativeCallbackSelfTests.js @@ -41,9 +41,14 @@ describe("Native callback self wrappers", function () { return JSInit; })(TNSAllocLog); - beforeEach(function () { - TNSClearOutput(); - initCalls = 0; + // Objects earlier specs dropped are collected first, so the dealloc log + // only counts this spec's objects. + beforeEach(function (done) { + settle(2, function () { + TNSClearOutput(); + initCalls = 0; + done(); + }); }); // +new runs the JS-implemented init from native code, before JS has a @@ -131,23 +136,83 @@ describe("Native callback self wrappers", function () { }); }); - it("balances owned returns that hand back an object JS already wraps", function (done) { - var array = NSArray.arrayWithArray([1, 2, 3]); + it("frees a managed wrapper's object once when __releaseNativeCounterpart runs on it", function (done) { + var owner = NSMutableArray.new(); + var natively = JSInit.new(); + owner.addObject(natively); + var jsOnly = JSInit.new(); settle(2, function () { - var before = array.retainCount(); - for (var i = 0; i < 50; i++) { - var copy = array.copy(); - expect(copy).toBe(array); - } - copy = null; - settle(2, function () { - expect(array.retainCount()).toBe(before); - expect(array.count).toBe(3); - done(); + __releaseNativeCounterpart(natively); + __releaseNativeCounterpart(jsOnly); + natively = null; + jsOnly = null; + settle(3, function () { + expect(deallocs()).toBe(1); + owner.removeAllObjects(); + settle(3, function () { + expect(deallocs()).toBe(2); + done(); + }); }); }); }); + it("does not release an instance method's owned return that is its receiver", function (done) { + var obj = TNSAllocLog.alloc().init(); + var same = obj.awakeAfterUsingCoder(null); + expect(same).toBe(obj); + settle(3, function () { + expect(deallocs()).toBe(0); + expect(obj.hash > 0).toBe(true); + done(); + }); + }); + + it("does not release what a JS copyWithZone hands to a native -copy", function (done) { + var target = TNSAllocLog.alloc().init(); + var Copyable = TNSAllocLog.extend({ + copyWithZone: function (zone) { + return target; + } + }, { name: "NativeCallbackSelfCopyable", protocols: [NSCopying] }); + var original = Copyable.alloc().init(); + var copy = original.copy(); + expect(copy).toBe(target); + copy = null; + settle(3, function () { + expect(deallocs()).toBe(0); + expect(target.hash > 0).toBe(true); + expect(original.hash > 0).toBe(true); + done(); + }); + }); + + it("reports a receiver kept from a JS method run by -dealloc as released", function (done) { + var saved = null; + var Keeper = TNSApi.extend({ + methodCalledInDealloc: function () { + saved = this; + } + }, { name: "NativeCallbackSelfDeallocKeeper" }); + (function () { + var holder = NSMutableArray.alloc().init(); + holder.addObject(Keeper.alloc().init()); + })(); + settle(4, function () { + expect(saved).not.toBeNull(); + var value; + var threw = false; + try { + value = saved.hash; + } catch (e) { + threw = e instanceof ReferenceError; + } + expect(threw || value === undefined).toBe(true); + saved = null; + done(); + }); + }); + it("keeps an error's NSError while the error lives and frees it afterwards", function (done) { var watch = NSHashTable.weakObjectsHashTable(); var error = null; From ee736af3ca73cf8582b0d873576932b85196a02f Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Sat, 14 Nov 2026 14:46:08 -0300 Subject: [PATCH 3/4] fix(runtime): keep JS overrides out of receiver wrapping, own init results Wrapping the receiver of a JS method called from native code asked the object -respondsToSelector: and -allowsWeakReference before caching it. On a class that also implements -respondsToSelector: in JS, that send re-entered MethodCallback for the same uncached object until the stack overflowed: X.new() or X.alloc().init() with a JS init did it every time. Both questions now go through the ObjC runtime to the nearest class not defined from JS (ClassBuilder::FindNativeClass). A JS -respondsToSelector: that called its base implementation also recursed on its own: Interop::CallFunctionInternal asked the target -respondsToSelector: for the swizzled selector, which reached the JS override again. It now asks the target's class with class_respondsToSelector. A JS-implemented init-family method that returns an object other than its receiver handed it to the native caller at +0, while +new and [[X alloc] init...] callers treat it as +1. With the cache-hit release in ArgConverter::ConstructObject and the class-method release in Interop::GetResult, new X() and X.new() consumed the reference Caches::Instances holds and freed the object under a live wrapper. This covers both a JS initializer returning another wrapped object and one returning what a substituting super initializer answered. MethodCallback now retains such a result. The receiver is not released: a super initializer that substituted it already did, and the two cases cannot be told apart. X.alloc().init() keeps leaking a retain of the result (tracked in #566). Also: the ReconcileOwnership contract says what it needs (isolate locked and entered, any thread), the NSTimer exclusion states its reason, and the -dealloc receiver spec accepts only the runtime's disposed-object error or an undefined read. --- NativeScript/runtime/ArgConverter.mm | 57 ++++++- NativeScript/runtime/ClassBuilder.h | 5 +- NativeScript/runtime/ClassBuilder.mm | 8 +- NativeScript/runtime/Interop.mm | 4 +- NativeScript/runtime/ObjectManager.h | 5 +- NativeScript/runtime/ObjectManager.mm | 8 +- TestFixtures/Marshalling/TNSAllocLog.h | 7 + TestFixtures/Marshalling/TNSAllocLog.m | 8 + .../app/tests/NativeCallbackSelfTests.js | 158 +++++++++++++++++- 9 files changed, 243 insertions(+), 17 deletions(-) diff --git a/NativeScript/runtime/ArgConverter.mm b/NativeScript/runtime/ArgConverter.mm index 649c23f78..8b9b58cf0 100644 --- a/NativeScript/runtime/ArgConverter.mm +++ b/NativeScript/runtime/ArgConverter.mm @@ -2,6 +2,7 @@ #include #include #include +#include "ClassBuilder.h" #include "DictionaryAdapter.h" #include "ExternalMemory.h" #include "Helpers.h" @@ -20,13 +21,60 @@ namespace { // -allowsWeakReference answers NO once deallocation has begun, and for the -// rare classes that refuse weak references altogether. +// rare classes that refuse weak references altogether. Runs before `obj` has +// a cached wrapper, so nothing here may be a message a JS class implements: +// it would re-enter MethodCallback for the same object. bool IsDeallocating(id obj) { static SEL allowsWeakReference = sel_registerName("allowsWeakReference"); - if (![obj respondsToSelector:allowsWeakReference]) { + Class nativeClass = ClassBuilder::FindNativeClass(object_getClass(obj)); + // class_getMethodImplementation answers the forwarding IMP for a selector + // the class lacks. + if (!class_respondsToSelector(nativeClass, allowsWeakReference)) { return false; } - return !((BOOL (*)(id, SEL))objc_msgSend)(obj, allowsWeakReference); + IMP imp = class_getMethodImplementation(nativeClass, allowsWeakReference); + return !((BOOL (*)(id, SEL))imp)(obj, allowsWeakReference); +} + +// Clang's init method family: optional leading underscores, then "init", then +// the end of the selector's first piece or anything but a lowercase letter. +bool IsInitFamily(SEL selector) { + const char* name = sel_getName(selector); + while (*name == '_') { + name++; + } + return strncmp(name, "init", 4) == 0 && (name[4] < 'a' || name[4] > 'z'); +} + +// The native caller of an init-family method owns its result. SetValue hands +// a wrapped object or a string over at +0: for the receiver that is the +// reference the caller passed in, any other object needs its own. The +// receiver is left alone: when a super initializer substituted it, that +// initializer already released it, and nothing here can tell that case apart. +void RetainInitializerResult(Isolate* isolate, void** argValues, void* retValue, + const TypeEncoding* typeEncoding, Local value) { + BinaryTypeEncodingType type = typeEncoding->type; + if (type != BinaryTypeEncodingType::IdEncoding && + type != BinaryTypeEncodingType::InterfaceDeclarationReference && + type != BinaryTypeEncodingType::InstanceTypeEncoding) { + return; + } + + id returned = *static_cast(retValue); + if (returned == nil || returned == *static_cast(argValues[0]) || + !IsInitFamily(*static_cast(argValues[1]))) { + return; + } + + // A plain JS object becomes a DictionaryAdapter SetValue already returns + // at +1. + value = tns::UnwrapProxy(value); + BaseDataWrapper* wrapper = value->IsObject() ? tns::GetValue(isolate, value) : nullptr; + bool atPlusZero = + value->IsString() || (wrapper != nullptr && wrapper->Type() == WrapperType::ObjCObject); + if (atPlusZero) { + [returned retain]; + } } // The borrowed wrapper WrapNativeCallbackSelf made for an object in -dealloc @@ -467,6 +515,9 @@ explicit BorrowedSelfScope(Isolate* isolate) : isolate(isolate) {} memset(retValue, 0, cif->rtype->size); } else { ArgConverter::SetValue(context, retValue, cif->rtype->size, result, data->typeEncoding_); + if (data->initialParamIndex_ == 2) { + RetainInitializerResult(isolate, argValues, retValue, data->typeEncoding_, result); + } } } else { memset(retValue, 0, cif->rtype->size); diff --git a/NativeScript/runtime/ClassBuilder.h b/NativeScript/runtime/ClassBuilder.h index 730649a12..d6a353279 100644 --- a/NativeScript/runtime/ClassBuilder.h +++ b/NativeScript/runtime/ClassBuilder.h @@ -37,8 +37,9 @@ class ClassBuilder { v8::Local context); static std::string GetTypeEncoding(const TypeEncoding* typeEncoding, int argsCount); - // The implementation of `method` from the nearest superclass not defined - // from JS. + // `klass` or its nearest superclass not defined from JS. + static Class FindNativeClass(Class klass); + // The implementation of `method` from FindNativeClass(klass). static IMP FindNotOverridenMethod(Class klass, SEL method); private: diff --git a/NativeScript/runtime/ClassBuilder.mm b/NativeScript/runtime/ClassBuilder.mm index e56a20ae4..3baac7706 100644 --- a/NativeScript/runtime/ClassBuilder.mm +++ b/NativeScript/runtime/ClassBuilder.mm @@ -1017,12 +1017,16 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) { info.GetReturnValue().Set(superValue); } -IMP ClassBuilder::FindNotOverridenMethod(Class klass, SEL method) { +Class ClassBuilder::FindNativeClass(Class klass) { while (class_conformsToProtocol(klass, @protocol(TNSDerivedClass))) { klass = class_getSuperclass(klass); } - return class_getMethodImplementation(klass, method); + return klass; +} + +IMP ClassBuilder::FindNotOverridenMethod(Class klass, SEL method) { + return class_getMethodImplementation(FindNativeClass(klass), method); } std::atomic ClassBuilder::classNameCounter_{0}; diff --git a/NativeScript/runtime/Interop.mm b/NativeScript/runtime/Interop.mm index 9c755cde4..18601a196 100644 --- a/NativeScript/runtime/Interop.mm +++ b/NativeScript/runtime/Interop.mm @@ -1804,7 +1804,9 @@ throw NativeScriptException( SEL selector = methodCall.selector_; if (isInstanceMethod) { SEL swizzledMethodSelector = Interop::GetSwizzledMethodSelector(selector); - if ([methodCall.target_ respondsToSelector:swizzledMethodSelector]) { + // Not -respondsToSelector:, which a JS class may implement by calling + // its base implementation through this very path. + if (class_respondsToSelector(object_getClass(methodCall.target_), swizzledMethodSelector)) { selector = swizzledMethodSelector; } diff --git a/NativeScript/runtime/ObjectManager.h b/NativeScript/runtime/ObjectManager.h index 66c65145e..145ba894e 100644 --- a/NativeScript/runtime/ObjectManager.h +++ b/NativeScript/runtime/ObjectManager.h @@ -60,8 +60,9 @@ class ObjectManager { // when a major collection starts. static void ManageOwnership(v8::Isolate* isolate, ObjCDataWrapper* wrapper, ObjectWeakCallbackState* state); - // Runs on the isolate's thread with the isolate entered. - static void ReconcileOwnership(v8::Isolate* isolate, ObjCDataWrapper* wrapper); + // Requires the isolate locked and entered, on any thread. + static void ReconcileOwnership(v8::Isolate* isolate, + ObjCDataWrapper* wrapper); private: static void ReleaseNativeCounterpartCallback( diff --git a/NativeScript/runtime/ObjectManager.mm b/NativeScript/runtime/ObjectManager.mm index 344603f80..793612cd3 100644 --- a/NativeScript/runtime/ObjectManager.mm +++ b/NativeScript/runtime/ObjectManager.mm @@ -36,13 +36,13 @@ void ApplyOwnership(ObjectWeakCallbackState* state, bool nativelyOwned) { if (nativelyOwned && weak) { state->target_->ClearWeak(); } else if (!nativelyOwned && !weak) { - state->target_->SetWeak(state, ObjectManager::FinalizerCallback, - WeakCallbackType::kFinalizer); + state->target_->SetWeak(state, ObjectManager::FinalizerCallback, WeakCallbackType::kFinalizer); } } // The GC prologue may not enter JS, so a JS-implemented -retainCount is -// skipped. Matches ObjectManager::GetRetainCount, which counts timers as unowned. +// skipped. Timers read as unowned: the retain count of an uninitialized NSTimer +// (NSTimer.alloc()) can hang. bool NativelyOwnedWithoutJS(id obj) { for (Class c = object_getClass(obj); c != nil; c = class_getSuperclass(c)) { if (c == NSTimerClass) { @@ -50,7 +50,7 @@ bool NativelyOwnedWithoutJS(id obj) { } } IMP imp = ClassBuilder::FindNotOverridenMethod(object_getClass(obj), @selector(retainCount)); - return ((NSUInteger(*)(id, SEL))imp)(obj, @selector(retainCount)) > 1; + return ((NSUInteger (*)(id, SEL))imp)(obj, @selector(retainCount)) > 1; } void ReconcileOwnershipBeforeGC(Isolate* isolate, GCType, GCCallbackFlags) { diff --git a/TestFixtures/Marshalling/TNSAllocLog.h b/TestFixtures/Marshalling/TNSAllocLog.h index 78c263782..ef80f9a93 100644 --- a/TestFixtures/Marshalling/TNSAllocLog.h +++ b/TestFixtures/Marshalling/TNSAllocLog.h @@ -12,4 +12,11 @@ @end +// -init answers a new TNSAllocLog in place of the receiver. +@interface TNSSubstitutingInit : NSObject + +- (instancetype)init; + +@end + #endif /* TNSAllocLog_h */ diff --git a/TestFixtures/Marshalling/TNSAllocLog.m b/TestFixtures/Marshalling/TNSAllocLog.m index 448da23a4..e04bb917d 100644 --- a/TestFixtures/Marshalling/TNSAllocLog.m +++ b/TestFixtures/Marshalling/TNSAllocLog.m @@ -21,3 +21,11 @@ - (void)dealloc { } @end + +@implementation TNSSubstitutingInit + +- (instancetype)init { + return (id)[[TNSAllocLog alloc] init]; +} + +@end diff --git a/TestRunner/app/tests/NativeCallbackSelfTests.js b/TestRunner/app/tests/NativeCallbackSelfTests.js index 331e65b95..049ae5e21 100644 --- a/TestRunner/app/tests/NativeCallbackSelfTests.js +++ b/TestRunner/app/tests/NativeCallbackSelfTests.js @@ -187,6 +187,154 @@ describe("Native callback self wrappers", function () { }); }); + // The JS init runs from native code on an object JS has no wrapper for + // yet; wrapping it must not send it a message its JS class implements. + function expectRespondsToSelectorOverrideWorks(Responder, counter) { + var creators = [ + function () { + return Responder.alloc().init(); + }, + function () { + return Responder.new(); + } + ]; + creators.forEach(function (create) { + counter.inits = 0; + counter.responds = 0; + counter.voidCalls = 0; + var obj = create(); + expect(obj instanceof Responder).toBe(true); + expect(counter.inits).toBe(1); + TNSTestNativeCallbacks.inheritanceVoidSelector(obj); + expect(counter.responds).toBeGreaterThan(0); + expect(counter.voidCalls).toBe(1); + }); + } + + it("wraps objects of an extend() class that implements respondsToSelector", function () { + var counter = {}; + var Responder = NSObject.extend({ + init: function () { + counter.inits++; + return NSObject.prototype.init.call(this); + }, + respondsToSelector: function (selector) { + counter.responds++; + return NSObject.prototype.respondsToSelector.call(this, selector); + }, + voidSelector: function () { + counter.voidCalls++; + } + }, { + name: "NativeCallbackSelfResponder", + exposedMethods: { + voidSelector: { returns: interop.types.void } + } + }); + expectRespondsToSelectorOverrideWorks(Responder, counter); + }); + + it("wraps objects of an __extends class that implements respondsToSelector", function () { + var counter = {}; + var Responder = (function (_super) { + __extends(Responder, _super); + function Responder() { + return (_super !== null && _super.apply(this, arguments)) || this; + } + Responder.prototype.init = function () { + counter.inits++; + return _super.prototype.init.call(this); + }; + Responder.prototype.respondsToSelector = function (selector) { + counter.responds++; + return _super.prototype.respondsToSelector.call(this, selector); + }; + Responder.prototype.voidSelector = function () { + counter.voidCalls++; + }; + Responder.ObjCExposedMethods = { + voidSelector: { returns: interop.types.void } + }; + return Responder; + })(NSObject); + expectRespondsToSelectorOverrideWorks(Responder, counter); + }); + + // Native callers own what an init-family method returns, including an + // object a JS initializer answers in place of its receiver. + function expectInitializerResultOwnedByCaller(create, done) { + var owner = NSMutableArray.new(); + (function () { + var obj = create(); + expect(obj instanceof TNSAllocLog).toBe(true); + owner.addObject(obj); + })(); + settle(3, function () { + expect(initCalls).toBe(1); + expect(deallocs()).toBe(0); + var obj = owner.objectAtIndex(0); + expect(obj instanceof TNSAllocLog).toBe(true); + expect(obj.hash > 0).toBe(true); + obj = null; + owner.removeAllObjects(); + settle(3, function () { + expect(deallocs()).toBe(1); + done(); + }); + }); + } + + var initResult = null; + var InitReturnsOther = NSObject.extend({ + init: function () { + initCalls++; + return initResult; + } + }, { name: "NativeCallbackSelfInitReturnsOther" }); + + function createThroughInitReturningOther(construct) { + initResult = TNSAllocLog.new(); + var obj = construct(); + expect(obj).toBe(initResult); + initResult = null; + return obj; + } + + it("keeps a wrapped object a JS initializer returns instead of its receiver (+new)", function (done) { + expectInitializerResultOwnedByCaller(function () { + return createThroughInitReturningOther(function () { + return InitReturnsOther.new(); + }); + }, done); + }); + + it("keeps a wrapped object a JS initializer returns instead of its receiver (new)", function (done) { + expectInitializerResultOwnedByCaller(function () { + return createThroughInitReturningOther(function () { + return new InitReturnsOther(); + }); + }, done); + }); + + var InitReturnsSubstitute = TNSSubstitutingInit.extend({ + init: function () { + initCalls++; + return TNSSubstitutingInit.prototype.init.call(this); + } + }, { name: "NativeCallbackSelfInitReturnsSubstitute" }); + + it("keeps the substitute a super initializer returned through a JS initializer (+new)", function (done) { + expectInitializerResultOwnedByCaller(function () { + return InitReturnsSubstitute.new(); + }, done); + }); + + it("keeps the substitute a super initializer returned through a JS initializer (new)", function (done) { + expectInitializerResultOwnedByCaller(function () { + return new InitReturnsSubstitute(); + }, done); + }); + it("reports a receiver kept from a JS method run by -dealloc as released", function (done) { var saved = null; var Keeper = TNSApi.extend({ @@ -201,13 +349,17 @@ describe("Native callback self wrappers", function () { settle(4, function () { expect(saved).not.toBeNull(); var value; - var threw = false; + var error = null; try { value = saved.hash; } catch (e) { - threw = e instanceof ReferenceError; + error = e; + } + if (error !== null) { + expect(error.message).toContain("disposed native object"); + } else { + expect(value).toBeUndefined(); } - expect(threw || value === undefined).toBe(true); saved = null; done(); }); From ad6045afdf5e2d8d5f6b408cd841c188bed171f0 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Sat, 14 Nov 2026 14:51:04 -0300 Subject: [PATCH 4/4] refactor(runtime): drop the unused skipGCRegistration parameter No caller asks ConvertArgument or CreateJsWrapper for a strong, unregistered handle any more, so the parameter and its plumbing through CreateEmptyObject and CreateEmptyInstance go: every wrapper handle comes from ObjectManager::Register. --- NativeScript/runtime/ArgConverter.h | 14 +++++--------- NativeScript/runtime/ArgConverter.mm | 28 ++++++++-------------------- NativeScript/runtime/Interop.mm | 11 +++++------ 3 files changed, 18 insertions(+), 35 deletions(-) diff --git a/NativeScript/runtime/ArgConverter.h b/NativeScript/runtime/ArgConverter.h index 6a09203aa..b8bcfe5ac 100644 --- a/NativeScript/runtime/ArgConverter.h +++ b/NativeScript/runtime/ArgConverter.h @@ -40,26 +40,23 @@ class ArgConverter { v8::Local receiver, V8Args& args, const MethodMeta* meta, bool isMethodCallback); - // skipGCRegistration: for ObjC wrappers the object gets a strong handle and - // no finalizer. static v8::Local ConvertArgument( v8::Local context, BaseDataWrapper* wrapper, - bool skipGCRegistration = false, const std::vector& additionalProtocols = std::vector()); static v8::Local CreateJsWrapper( v8::Local context, BaseDataWrapper* wrapper, - v8::Local receiver, bool skipGCRegistration = false, + v8::Local receiver, const std::vector& additionalProtocols = std::vector()); static std::shared_ptr> CreateEmptyObject( - v8::Local context, bool skipGCRegistration = false); + v8::Local context); // The JS `this` for native code calling a JS-implemented method on an // object JS has no wrapper for yet. For an object already deallocating it // is a borrowed wrapper, stored in `borrowed`, that the caller must detach // and delete once the call returns. - static v8::Local WrapNativeCallbackSelf(v8::Local context, id self, - ObjCDataWrapper** borrowed); + static v8::Local WrapNativeCallbackSelf( + v8::Local context, id self, ObjCDataWrapper** borrowed); static const Meta* FindMeta(Class klass, const TypeEncoding* typeEncoding = nullptr); // Looks up the JS wrapper cached for `target` in Caches::Instances, dropping @@ -108,8 +105,7 @@ class ArgConverter { static v8::Local NewEmptyInstance( v8::Local context, v8::Persistent* ctorFunc); static std::shared_ptr> CreateEmptyInstance( - v8::Local context, v8::Persistent* ctorFunc, - bool skipGCRegistration = false); + v8::Local context, v8::Persistent* ctorFunc); static void FindMethodOverloads(Class klass, std::string methodName, MemberType type, std::vector& overloads); diff --git a/NativeScript/runtime/ArgConverter.mm b/NativeScript/runtime/ArgConverter.mm index 8b9b58cf0..f83403f87 100644 --- a/NativeScript/runtime/ArgConverter.mm +++ b/NativeScript/runtime/ArgConverter.mm @@ -197,15 +197,13 @@ explicit BorrowedSelfScope(Isolate* isolate) : isolate(isolate) {} } Local ArgConverter::ConvertArgument(Local context, BaseDataWrapper* wrapper, - bool skipGCRegistration, const std::vector& additionalProtocols) { Isolate* isolate = v8::Isolate::GetCurrent(); if (wrapper == nullptr) { return Null(isolate); } - Local result = - CreateJsWrapper(context, wrapper, Local(), skipGCRegistration, additionalProtocols); + Local result = CreateJsWrapper(context, wrapper, Local(), additionalProtocols); return result; } @@ -926,7 +924,7 @@ explicit BorrowedSelfScope(Isolate* isolate) : isolate(isolate) {} } Local ArgConverter::CreateJsWrapper(Local context, BaseDataWrapper* wrapper, - Local receiver, bool skipGCRegistration, + Local receiver, const std::vector& additionalProtocols) { Isolate* isolate = v8::Isolate::GetCurrent(); @@ -942,7 +940,7 @@ explicit BorrowedSelfScope(Isolate* isolate) : isolate(isolate) {} ObjCAllocDataWrapper* allocDataWrapper = static_cast(wrapper); Class klass = allocDataWrapper->Klass(); - std::shared_ptr> poValue = CreateEmptyObject(context, false); + std::shared_ptr> poValue = CreateEmptyObject(context); receiver = poValue->Get(isolate).As(); const Meta* meta = FindMeta(klass); @@ -985,7 +983,7 @@ explicit BorrowedSelfScope(Isolate* isolate) : isolate(isolate) {} if (it != cache->Instances.end()) { receiver = it->second->Get(isolate).As(); } else { - std::shared_ptr> poValue = CreateEmptyObject(context, skipGCRegistration); + std::shared_ptr> poValue = CreateEmptyObject(context); receiver = poValue->Get(isolate).As(); tns::SetValue(isolate, receiver, wrapper); cache->Instances.emplace(target, poValue); @@ -1181,12 +1179,11 @@ explicit BorrowedSelfScope(Isolate* isolate) : isolate(isolate) {} return result; } -std::shared_ptr> ArgConverter::CreateEmptyObject(Local context, - bool skipGCRegistration) { +std::shared_ptr> ArgConverter::CreateEmptyObject(Local context) { Isolate* isolate = v8::Isolate::GetCurrent(); Persistent* ctorFunc = Caches::Get(isolate)->EmptyObjCtorFunc.get(); tns::Assert(ctorFunc != nullptr, isolate); - return ArgConverter::CreateEmptyInstance(context, ctorFunc, skipGCRegistration); + return ArgConverter::CreateEmptyInstance(context, ctorFunc); } Local ArgConverter::NewEmptyInstance(Local context, @@ -1202,18 +1199,9 @@ explicit BorrowedSelfScope(Isolate* isolate) : isolate(isolate) {} } std::shared_ptr> ArgConverter::CreateEmptyInstance( - Local context, Persistent* ctorFunc, bool skipGCRegistration) { - Isolate* isolate = v8::Isolate::GetCurrent(); + Local context, Persistent* ctorFunc) { Local result = NewEmptyInstance(context, ctorFunc); - - std::shared_ptr> poValue; - if (!skipGCRegistration) { - poValue = ObjectManager::Register(context, result); - } else { - poValue = std::make_shared>(isolate, result); - } - - return poValue; + return ObjectManager::Register(context, result); } Local ArgConverter::CreateEmptyInstanceFunction(Local context) { diff --git a/NativeScript/runtime/Interop.mm b/NativeScript/runtime/Interop.mm index 18601a196..5cbc8f408 100644 --- a/NativeScript/runtime/Interop.mm +++ b/NativeScript/runtime/Interop.mm @@ -1382,8 +1382,7 @@ static bool SnapshotArrayLike(Isolate* isolate, Local object, Local additionalProtocols = Interop::GetAdditionalProtocols(typeEncoding); - Local jsResult = - ArgConverter::ConvertArgument(context, wrapper, false, additionalProtocols); + Local jsResult = ArgConverter::ConvertArgument(context, wrapper, additionalProtocols); if (ownsReturnedObject || isInitializer) { [result release]; @@ -1938,10 +1937,10 @@ throw NativeScriptException( } } - Local result = - Interop::GetResult(methodCall.context_, methodCall.typeEncoding_, &call, marshalToPrimitive, - Local(), false, methodCall.ownsReturnedObject_, - methodCall.returnsUnmanaged_, methodCall.isInitializer_, !isInstanceMethod); + Local result = Interop::GetResult( + methodCall.context_, methodCall.typeEncoding_, &call, marshalToPrimitive, Local(), + false, methodCall.ownsReturnedObject_, methodCall.returnsUnmanaged_, + methodCall.isInitializer_, !isInstanceMethod); // Owned returns, initializers and class factory methods hand out objects JS // most likely holds the last reference to. Instance getters do not: their