From 8a8aeed9981c836376b26bf245a72a8f1507428b Mon Sep 17 00:00:00 2001 From: Alexander Shabalin Date: Fri, 31 Mar 2023 16:55:29 +0200 Subject: [PATCH] [K/N] Handle permanent objects in new MM MemorySharedRefs ^KT-56233 Do not create a foreign ref to the permanent objects. There's no need to do it, and, moreover, this ref would have leaked. --- .../src/legacymm/cpp/MemorySharedRefs.cpp | 8 ++++++++ .../runtime/src/main/cpp/MemorySharedRefs.hpp | 3 +++ .../runtime/src/mm/cpp/MemorySharedRefs.cpp | 11 +++++++++++ .../runtime/src/objc/cpp/ObjCExportClasses.mm | 17 +++++++++++++---- 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/kotlin-native/runtime/src/legacymm/cpp/MemorySharedRefs.cpp b/kotlin-native/runtime/src/legacymm/cpp/MemorySharedRefs.cpp index 1d6efa0fa37..d0c31d71f3b 100644 --- a/kotlin-native/runtime/src/legacymm/cpp/MemorySharedRefs.cpp +++ b/kotlin-native/runtime/src/legacymm/cpp/MemorySharedRefs.cpp @@ -104,6 +104,10 @@ void KRefSharedHolder::dispose() { DeinitForeignRef(obj_, context_); } +void BackRefFromAssociatedObject::initForPermanentObject(ObjHeader* obj) { + initAndAddRef(obj); +} + void BackRefFromAssociatedObject::initAndAddRef(ObjHeader* obj) { RuntimeAssert(obj != nullptr, "must not be null"); obj_ = obj; @@ -198,3 +202,7 @@ ObjHeader* BackRefFromAssociatedObject::ref() const { template ObjHeader* BackRefFromAssociatedObject::ref() const; template ObjHeader* BackRefFromAssociatedObject::ref() const; template ObjHeader* BackRefFromAssociatedObject::ref() const; + +ObjHeader* BackRefFromAssociatedObject::refPermanent() const { + return ref(); +} diff --git a/kotlin-native/runtime/src/main/cpp/MemorySharedRefs.hpp b/kotlin-native/runtime/src/main/cpp/MemorySharedRefs.hpp index ea3ccc2beea..e90b59423bf 100644 --- a/kotlin-native/runtime/src/main/cpp/MemorySharedRefs.hpp +++ b/kotlin-native/runtime/src/main/cpp/MemorySharedRefs.hpp @@ -46,6 +46,7 @@ static_assert(std::is_trivially_destructible_v, "KRefSharedHol class BackRefFromAssociatedObject { public: + void initForPermanentObject(ObjHeader* obj); void initAndAddRef(ObjHeader* obj); // Error if refCount is zero and it's called from the wrong worker with non-frozen obj_. @@ -65,6 +66,8 @@ class BackRefFromAssociatedObject { template ObjHeader* ref() const; + ObjHeader* refPermanent() const; + private: ObjHeader* obj_; // May be null before [initAndAddRef] or after [detach]. ForeignRefContext context_; diff --git a/kotlin-native/runtime/src/mm/cpp/MemorySharedRefs.cpp b/kotlin-native/runtime/src/mm/cpp/MemorySharedRefs.cpp index 01d5e81c950..af533703f21 100644 --- a/kotlin-native/runtime/src/mm/cpp/MemorySharedRefs.cpp +++ b/kotlin-native/runtime/src/mm/cpp/MemorySharedRefs.cpp @@ -41,8 +41,15 @@ void KRefSharedHolder::dispose() { DeinitForeignRef(obj_, context_); } +void BackRefFromAssociatedObject::initForPermanentObject(ObjHeader* obj) { + RuntimeAssert(obj != nullptr, "must not be null"); + RuntimeAssert(obj->permanent(), "Can only be called with permanent object"); + obj_ = obj; +} + void BackRefFromAssociatedObject::initAndAddRef(ObjHeader* obj) { RuntimeAssert(obj != nullptr, "must not be null"); + RuntimeAssert(!obj->permanent(), "Can only be called with non-permanent object"); obj_ = obj; // Generally a specialized addRef below: @@ -126,3 +133,7 @@ ObjHeader* BackRefFromAssociatedObject::ref() const { template ObjHeader* BackRefFromAssociatedObject::ref() const; template ObjHeader* BackRefFromAssociatedObject::ref() const; template ObjHeader* BackRefFromAssociatedObject::ref() const; + +ObjHeader* BackRefFromAssociatedObject::refPermanent() const { + return obj_; +} diff --git a/kotlin-native/runtime/src/objc/cpp/ObjCExportClasses.mm b/kotlin-native/runtime/src/objc/cpp/ObjCExportClasses.mm index 3c80493ce2d..173d7a2e236 100644 --- a/kotlin-native/runtime/src/objc/cpp/ObjCExportClasses.mm +++ b/kotlin-native/runtime/src/objc/cpp/ObjCExportClasses.mm @@ -42,7 +42,11 @@ static void injectToRuntime(); } -(KRef)toKotlin:(KRef*)OBJ_RESULT { - RETURN_OBJ(refHolder.ref()); + if (permanent) { + RETURN_OBJ(refHolder.refPermanent()); + } else { + RETURN_OBJ(refHolder.ref()); + } } +(void)load { @@ -88,10 +92,11 @@ static void injectToRuntime(); KotlinBase* candidate = [super allocWithZone:nil]; // TODO: should we call NSObject.init ? - candidate->refHolder.initAndAddRef(obj); - candidate->permanent = obj->permanent(); + bool permanent = obj->permanent(); + candidate->permanent = permanent; - if (!obj->permanent()) { // TODO: permanent objects should probably be supported as custom types. + if (!permanent) { // TODO: permanent objects should probably be supported as custom types. + candidate->refHolder.initAndAddRef(obj); if (!isShareable(obj)) { SetAssociatedObject(obj, candidate); } else { @@ -105,6 +110,8 @@ static void injectToRuntime(); return objc_retain(old); } } + } else { + candidate->refHolder.initForPermanentObject(obj); } return candidate; @@ -136,6 +143,8 @@ static void injectToRuntime(); } -(void)releaseAsAssociatedObject:(ReleaseMode)mode { + RuntimeAssert(!permanent, "Cannot be called on permanent objects"); + // This function is called by the GC. It made a decision to reclaim Kotlin object, and runs // deallocation hooks at the moment, including deallocation of the "associated object" ([self]) // using the [super release] call below.