From 063e7258eeb9e57e6f4b7e6842dbcc38b514ede4 Mon Sep 17 00:00:00 2001 From: Svyatoslav Scherbina Date: Mon, 5 Jun 2023 17:48:09 +0200 Subject: [PATCH] Native: don't dispatch release for null Obj-C refs to the main thread Currently, if a Kotlin object happens to have null for an associated object but FLAGS_RELEASE_ON_MAIN_QUEUE flag set, GC will dispatch Kotlin_ObjCExport_releaseAssociatedObject(null) to the main thread anyway. This couldn't happen before, but can now, with disposeObjCObject. The commit prevents this, by moving the null check out from Kotlin_ObjCExport_releaseAssociatedObject to call sites. ^KT-59134 --- .../runtime/src/legacymm/cpp/Memory.cpp | 4 +++- kotlin-native/runtime/src/main/cpp/ObjCExport.mm | 9 ++++----- .../runtime/src/mm/cpp/ExtraObjectData.cpp | 16 +++++++++------- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/kotlin-native/runtime/src/legacymm/cpp/Memory.cpp b/kotlin-native/runtime/src/legacymm/cpp/Memory.cpp index 44e19fb9478..730a5273211 100644 --- a/kotlin-native/runtime/src/legacymm/cpp/Memory.cpp +++ b/kotlin-native/runtime/src/legacymm/cpp/Memory.cpp @@ -3085,7 +3085,9 @@ void ObjHeader::destroyMetaObject(ObjHeader* object) { } #ifdef KONAN_OBJC_INTEROP - Kotlin_ObjCExport_releaseAssociatedObject(meta->associatedObject_); + if (void* associatedObject = meta->associatedObject_) { + Kotlin_ObjCExport_releaseAssociatedObject(associatedObject); + } #endif std_support::allocator_delete(objectAllocator, meta); diff --git a/kotlin-native/runtime/src/main/cpp/ObjCExport.mm b/kotlin-native/runtime/src/main/cpp/ObjCExport.mm index 13fe611475f..165fb4eb10c 100644 --- a/kotlin-native/runtime/src/main/cpp/ObjCExport.mm +++ b/kotlin-native/runtime/src/main/cpp/ObjCExport.mm @@ -107,11 +107,10 @@ ALWAYS_INLINE void send_releaseAsAssociatedObject(void* associatedObject) { } // namespace extern "C" ALWAYS_INLINE void Kotlin_ObjCExport_releaseAssociatedObject(void* associatedObject) { - if (associatedObject != nullptr) { - // May already be in the native state if was scheduled on the main queue. - NativeOrUnregisteredThreadGuard guard(/*reentrant=*/ true); - send_releaseAsAssociatedObject(associatedObject); - } + RuntimeAssert(associatedObject != nullptr, "Kotlin_ObjCExport_releaseAssociatedObject(nullptr)"); + // May already be in the native state if was scheduled on the main queue. + NativeOrUnregisteredThreadGuard guard(/*reentrant=*/ true); + send_releaseAsAssociatedObject(associatedObject); } extern "C" id Kotlin_ObjCExport_convertUnitToRetained(ObjHeader* unitInstance) { diff --git a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp index f42c3181bfd..b2f70845d36 100644 --- a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.cpp @@ -60,14 +60,16 @@ void mm::ExtraObjectData::Uninstall() noexcept { this); #ifdef KONAN_OBJC_INTEROP - if (getFlag(FLAGS_RELEASE_ON_MAIN_QUEUE) && isMainQueueProcessorAvailable()) { - runOnMainQueue(associatedObject_, [](void* obj) { - Kotlin_ObjCExport_releaseAssociatedObject(obj); - }); - } else { - Kotlin_ObjCExport_releaseAssociatedObject(associatedObject_); + if (void* associatedObject = associatedObject_) { + if (getFlag(FLAGS_RELEASE_ON_MAIN_QUEUE) && isMainQueueProcessorAvailable()) { + runOnMainQueue(associatedObject, [](void* obj) { + Kotlin_ObjCExport_releaseAssociatedObject(obj); + }); + } else { + Kotlin_ObjCExport_releaseAssociatedObject(associatedObject); + } + associatedObject_ = nullptr; } - associatedObject_ = nullptr; #endif }