From 1eda35a59cc6445fe348647688637d2f0695a19b Mon Sep 17 00:00:00 2001 From: "Aleksei.Glushko" Date: Thu, 27 Jul 2023 14:17:48 +0000 Subject: [PATCH] [K/N] Process objc run loop on the finalizer thread ^KT-58851 Merge-request: KT-MR-11024 Merged-by: Alexey Glushko --- .../backend.native/tests/build.gradle | 25 +++ .../objc/autorelease_from_dealloc/main.kt | 136 ++++++++++++++++ .../objc/autorelease_from_dealloc/objclib.def | 3 + .../objc/autorelease_from_dealloc/objclib.h | 11 ++ .../objc/autorelease_from_dealloc/objclib.m | 36 +++++ .../src/gc/common/cpp/FinalizerProcessor.hpp | 150 +++++++++++++++--- .../runtime/src/main/cpp/ObjCInterop.mm | 2 +- 7 files changed, 339 insertions(+), 24 deletions(-) create mode 100644 kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/main.kt create mode 100644 kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/objclib.def create mode 100644 kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/objclib.h create mode 100644 kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/objclib.m diff --git a/kotlin-native/backend.native/tests/build.gradle b/kotlin-native/backend.native/tests/build.gradle index 84d0cdb3d76..8fa1efab5a7 100644 --- a/kotlin-native/backend.native/tests/build.gradle +++ b/kotlin-native/backend.native/tests/build.gradle @@ -4147,6 +4147,10 @@ if (PlatformInfo.isAppleTarget(project)) { it.defFile 'interop/objc/kt56402/objclib.def' it.headers "$projectDir/interop/objc/kt56402/objclib.h" } + createInterop("objc_autorelease_from_dealloc") { + it.defFile 'interop/objc/autorelease_from_dealloc/objclib.def' + it.headers "$projectDir/interop/objc/autorelease_from_dealloc/objclib.h" + } createInterop("objCAction") { it.defFile 'interop/objc/objCAction/objclib.def' it.headers "$projectDir/interop/objc/objCAction/objclib.h" @@ -4934,6 +4938,7 @@ if (PlatformInfo.isAppleTarget(project)) { mkdir(buildDir) project.extensions.execClang.execClangForCompilerTests(project.target) { args "$projectDir/interop/objc/kt56402/objclib.m" + args "-g" args "-lobjc", '-fobjc-arc' args '-fPIC', '-shared', '-o', "$buildDir/libobjc_kt56402.dylib" args '-framework', 'AppKit' @@ -4944,6 +4949,26 @@ if (PlatformInfo.isAppleTarget(project)) { } } + interopTest("interop_objc_autorelease_from_dealloc") { + enabled = !isNoopGC // requires some GC + source = 'interop/objc/autorelease_from_dealloc/main.kt' + interop = "objc_autorelease_from_dealloc" + flags = ["-tr"] + + doBeforeBuild { + mkdir(buildDir) + project.extensions.execClang.execClangForCompilerTests(project.target) { + args "$projectDir/interop/objc/autorelease_from_dealloc/objclib.m" + args "-lobjc", '-fno-objc-arc' + args '-fPIC', '-shared', '-o', "$buildDir/libobjc_autorelease_from_dealloc.dylib" + } + if (UtilsKt.isSimulatorTarget(project, project.target)) { + UtilsKt.codesign(project, "$buildDir/libobjc_autorelease_from_dealloc.dylib") + } + } + UtilsKt.dependsOnPlatformLibs(it) + } + interopTest("interop_objCAction") { source = 'interop/objc/objCAction/main.kt' interop = "objCAction" diff --git a/kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/main.kt b/kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/main.kt new file mode 100644 index 00000000000..8311c639268 --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/main.kt @@ -0,0 +1,136 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ +@file:OptIn(kotlin.experimental.ExperimentalNativeApi::class) + +import objclib.* +import platform.objc.* +import platform.Foundation.* + +import kotlin.concurrent.* +import kotlin.native.concurrent.* +import kotlin.native.internal.test.testLauncherEntryPoint +import kotlin.system.exitProcess +import kotlin.test.* +import kotlin.time.* +import kotlin.time.Duration.Companion.seconds +import kotlin.time.Duration.Companion.minutes +import kotlinx.cinterop.* + +val timeout = 10.seconds + +fun allocCollectable(ctor: () -> ULong): ULong = autoreleasepool { + ctor() +} + +class Event() : NSObject() { + @Volatile + private var triggered = false + + fun isTriggered() = triggered + + @ObjCAction + fun trigger() { + assertFalse(isTriggered()) + triggered = true; + } +} + +val trigerSelector = sel_registerName("trigger") + +fun waitTriggered(event: Event) { + val timeoutMark = TimeSource.Monotonic.markNow() + timeout + + kotlin.native.internal.GC.collect() + while (true) { + if (event.isTriggered()) { + return + } + assertFalse(timeoutMark.hasPassedNow(), "Timed out") + } +} + +// All the tests are run on a secondary (non main) thread in order to prevent `objcDisposeOnMain` hacks from messing things up +@Test +fun testAutorelease() { + val event = withWorker { + execute(TransferMode.SAFE, {}) { + val event = Event() + assertFalse(event.isTriggered()) + + val victimId = allocCollectable { + val v = OnDestroyHook { + event.trigger() + } + retain(v.identity()) + v.identity() + } + + allocCollectable { + OnDestroyHook { + autorelease(victimId) + }.identity() + } + + event + }.result + } + waitTriggered(event) +} + +@Test +fun testTimer() { + val event = Event() + + withWorker { + execute(TransferMode.SAFE, { event }) { event -> + allocCollectable { + OnDestroyHook { + assertFalse(event.isTriggered()) + NSTimer.scheduledTimerWithTimeInterval(0.0, target=event, selector=trigerSelector, userInfo=null, repeats=false) + }.identity() + } + } + } + + waitTriggered(event) +} + +@Test +fun testSelector() { + val event = Event() + + withWorker { + execute(TransferMode.SAFE, { event }) { event -> + allocCollectable { + OnDestroyHook { + assertFalse(event.isTriggered()) + NSRunLoop.currentRunLoop().performSelector(trigerSelector, target=event, argument=null, order=0, modes=listOf(NSDefaultRunLoopMode)) + }.identity() + } + } + } + + waitTriggered(event) +} + +@Test +fun testPerformBlock() { + val event = Event() + + withWorker { + execute(TransferMode.SAFE, { event }) { event -> + allocCollectable { + OnDestroyHook { + NSRunLoop.currentRunLoop().performBlock({ + assertFalse(event.isTriggered()) + event.trigger() + }); + }.identity() + } + } + } + + waitTriggered(event) +} diff --git a/kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/objclib.def b/kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/objclib.def new file mode 100644 index 00000000000..f6d62ebdb1c --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/objclib.def @@ -0,0 +1,3 @@ +language = Objective-C +headerFilter = **/objclib.h +linkerOpts = -lobjc_autorelease_from_dealloc diff --git a/kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/objclib.h b/kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/objclib.h new file mode 100644 index 00000000000..30eac378e89 --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/objclib.h @@ -0,0 +1,11 @@ +#include +#include + +@interface OnDestroyHook: NSObject +-(instancetype)init:(void(^)(uintptr_t))onDestroy; +-(uintptr_t)identity; +@end + +void retain(uint64_t); +void release(uint64_t); +void autorelease(uint64_t); diff --git a/kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/objclib.m b/kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/objclib.m new file mode 100644 index 00000000000..6313b0ecdd7 --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/objc/autorelease_from_dealloc/objclib.m @@ -0,0 +1,36 @@ +#include "objclib.h" + +@implementation OnDestroyHook { + void (^_onDestroy)(uintptr_t); +} + +-(uintptr_t)identity { + return (uintptr_t)self; +} + +-(instancetype)init:(void (^)(uintptr_t))onDestroy { + if (self = [super init]) { + _onDestroy = [onDestroy retain]; + } + return self; +} + +-(void)dealloc { + _onDestroy([self identity]); + [_onDestroy release]; + [super dealloc]; +} + +@end + +void retain(uint64_t obj) { + [((id) obj) retain]; +} + +void release(uint64_t obj) { + [((id) obj) release]; +} + +void autorelease(uint64_t obj) { + [((id) obj) autorelease]; +} diff --git a/kotlin-native/runtime/src/gc/common/cpp/FinalizerProcessor.hpp b/kotlin-native/runtime/src/gc/common/cpp/FinalizerProcessor.hpp index 7333e1b2770..e0be64dff98 100644 --- a/kotlin-native/runtime/src/gc/common/cpp/FinalizerProcessor.hpp +++ b/kotlin-native/runtime/src/gc/common/cpp/FinalizerProcessor.hpp @@ -17,6 +17,13 @@ #include "Runtime.h" #include "ScopedThread.hpp" #include "Utils.hpp" +#include "Logging.hpp" + +#if KONAN_OBJC_INTEROP +#include "ObjCMMAPI.h" +#include +#include +#endif namespace kotlin::gc { @@ -26,7 +33,7 @@ public: // epochDoneCallback could be called on any subset of them. // If no new tasks are set, epochDoneCallback will be eventually called on last epoch explicit FinalizerProcessor(std::function epochDoneCallback) noexcept : - epochDoneCallback_(std::move(epochDoneCallback)) {} + epochDoneCallback_(std::move(epochDoneCallback)), processingLoop_(*this) {} ~FinalizerProcessor() { StopFinalizerThread(); } @@ -40,7 +47,7 @@ public: StartFinalizerThreadIfNone(); FinalizerQueueTraits::add(finalizerQueue_, std::move(tasks)); finalizerQueueEpoch_ = epoch; - finalizerQueueCondVar_.notify_all(); + processingLoop_.notify(); } void StopFinalizerThread() noexcept { @@ -48,7 +55,7 @@ public: std::unique_lock guard(finalizerQueueMutex_); if (!finalizerThread_.joinable()) return; shutdownFlag_ = true; - finalizerQueueCondVar_.notify_all(); + processingLoop_.notify(); } finalizerThread_.join(); shutdownFlag_ = false; @@ -71,26 +78,7 @@ public: initialized_ = true; } initializedCondVar_.notify_all(); - int64_t finalizersEpoch = 0; - while (true) { - std::unique_lock lock(finalizerQueueMutex_); - finalizerQueueCondVar_.wait(lock, [this, &finalizersEpoch] { - return !FinalizerQueueTraits::isEmpty(finalizerQueue_) || finalizerQueueEpoch_ != finalizersEpoch || shutdownFlag_; - }); - if (FinalizerQueueTraits::isEmpty(finalizerQueue_) && finalizerQueueEpoch_ == finalizersEpoch) { - newTasksAllowed_ = false; - RuntimeAssert(shutdownFlag_, "Nothing to do, but no shutdownFlag_ is set on wakeup"); - break; - } - auto queue = std::move(finalizerQueue_); - finalizersEpoch = finalizerQueueEpoch_; - lock.unlock(); - if (!FinalizerQueueTraits::isEmpty(queue)) { - ThreadStateGuard guard(ThreadState::kRunnable); - FinalizerQueueTraits::process(std::move(queue)); - } - epochDoneCallback_(finalizersEpoch); - } + processingLoop_.body(); { std::unique_lock guard(initializedMutex_); initialized_ = false; @@ -105,6 +93,119 @@ public: } private: + // should be called under the finalizerQueueMutex_ + bool shouldShutdown(int64_t lastProcessedEpoch) noexcept { + bool shouldShutdown = FinalizerQueueTraits::isEmpty(finalizerQueue_) && finalizerQueueEpoch_ == lastProcessedEpoch; + if (shouldShutdown) { + RuntimeAssert(shutdownFlag_, "Nothing to do, but no shutdownFlag_ is set on wakeup"); + } + return shouldShutdown; + } + + void processSingle(FinalizerQueue&& queue, int64_t currentEpoch) noexcept { + if (!FinalizerQueueTraits::isEmpty(queue)) { +#if KONAN_OBJC_INTEROP + konan::AutoreleasePool autoreleasePool; +#endif + ThreadStateGuard guard(ThreadState::kRunnable); + FinalizerQueueTraits::process(std::move(queue)); + } + epochDoneCallback_(currentEpoch); + } + +#if KONAN_OBJC_INTEROP + class ProcessingLoop { + public: + explicit ProcessingLoop(FinalizerProcessor& owner) : + owner_(owner), + sourceContext_{ + 0, this, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, + [](void* info) { + auto& self = *reinterpret_cast(info); + self.handleNewFinalizers(); + }}, + runLoopSource_(CFRunLoopSourceCreate(nullptr, 0, &sourceContext_)) {} + + ~ProcessingLoop() { + CFRelease(runLoopSource_); + } + + void notify() { + // wait until runLoop_ ptr is published + while (runLoop_.load(std::memory_order_acquire) == nullptr) { + std::this_thread::yield(); + } + // notify + CFRunLoopSourceSignal(runLoopSource_); + CFRunLoopWakeUp(runLoop_); + } + + void body() { + konan::AutoreleasePool autoreleasePool; + auto mode = kCFRunLoopDefaultMode; + CFRunLoopAddSource(CFRunLoopGetCurrent(), runLoopSource_, mode); + runLoop_.store(CFRunLoopGetCurrent(), std::memory_order_release); + + CFRunLoopRun(); + + runLoop_.store(nullptr, std::memory_order_release); + + CFRunLoopRemoveSource(CFRunLoopGetCurrent(), runLoopSource_, mode); + } + private: + void handleNewFinalizers() { + std::unique_lock lock(owner_.finalizerQueueMutex_); + if (owner_.shouldShutdown(finishedEpoch_)) { + owner_.newTasksAllowed_ = false; + CFRunLoopStop(runLoop_.load(std::memory_order_acquire)); + return; + } + auto queue = std::move(owner_.finalizerQueue_); + int64_t currentEpoch = owner_.finalizerQueueEpoch_; + lock.unlock(); + + owner_.processSingle(std::move(queue), currentEpoch); + finishedEpoch_ = currentEpoch; + } + + FinalizerProcessor& owner_; + int64_t finishedEpoch_ = 0; + CFRunLoopSourceContext sourceContext_; + std::atomic runLoop_ = nullptr; + CFRunLoopSourceRef runLoopSource_; + }; +#else + class ProcessingLoop { + public: + explicit ProcessingLoop(FinalizerProcessor& owner) : owner_(owner) {} + + void notify() { + owner_.finalizerQueueCondVar_.notify_all(); + } + + void body() { + int64_t finishedEpoch = 0; + while (true) { + std::unique_lock lock(owner_.finalizerQueueMutex_); + owner_.finalizerQueueCondVar_.wait(lock, [this, &finishedEpoch] { + return !FinalizerQueueTraits::isEmpty(owner_.finalizerQueue_) || owner_.finalizerQueueEpoch_ != finishedEpoch || owner_.shutdownFlag_; + }); + if (owner_.shouldShutdown(finishedEpoch)) { + owner_.newTasksAllowed_ = false; + break; + } + auto queue = std::move(owner_.finalizerQueue_); + auto currentEpoch = owner_.finalizerQueueEpoch_; + lock.unlock(); + owner_.processSingle(std::move(queue), currentEpoch); + finishedEpoch = currentEpoch; + } + } + private: + FinalizerProcessor& owner_; + }; +#endif + ScopedThread finalizerThread_; FinalizerQueue finalizerQueue_; std::condition_variable finalizerQueueCondVar_; @@ -114,11 +215,14 @@ private: bool shutdownFlag_ = false; bool newTasksAllowed_ = true; + ProcessingLoop processingLoop_; + std::mutex initializedMutex_; std::condition_variable initializedCondVar_; bool initialized_ = false; std::mutex threadCreatingMutex_; + }; } // namespace kotlin::gc diff --git a/kotlin-native/runtime/src/main/cpp/ObjCInterop.mm b/kotlin-native/runtime/src/main/cpp/ObjCInterop.mm index ed4aa181a9e..bfe04e67951 100644 --- a/kotlin-native/runtime/src/main/cpp/ObjCInterop.mm +++ b/kotlin-native/runtime/src/main/cpp/ObjCInterop.mm @@ -339,7 +339,7 @@ konan::AutoreleasePool::AutoreleasePool() : handle(objc_autoreleasePoolPush()) {} konan::AutoreleasePool::~AutoreleasePool() { - kotlin::ThreadStateGuard guard(kotlin::ThreadState::kNative); + kotlin::ThreadStateGuard guard(kotlin::ThreadState::kNative, true); objc_autoreleasePoolPop(handle); }