From cb19b9a88fb1a7b5f37e4c88cc3c6ecf13fc49ff Mon Sep 17 00:00:00 2001 From: Anders Ha Date: Tue, 19 Jan 2021 17:19:51 +0000 Subject: [PATCH] [native-mt] Fix ObjC autorelease object leaks with native-mt dispatchers (#2477) Fixes #2322 --- .../common/src/EventLoop.common.kt | 19 ++++++++- kotlinx-coroutines-core/js/src/EventLoop.kt | 2 + kotlinx-coroutines-core/jvm/src/EventLoop.kt | 2 + .../nativeDarwin/src/EventLoop.kt | 9 +++++ .../nativeDarwin/test/AutoreleaseLeakTest.kt | 40 +++++++++++++++++++ .../nativeOther/src/EventLoop.kt | 7 ++++ 6 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 kotlinx-coroutines-core/nativeDarwin/src/EventLoop.kt create mode 100644 kotlinx-coroutines-core/nativeDarwin/test/AutoreleaseLeakTest.kt create mode 100644 kotlinx-coroutines-core/nativeOther/src/EventLoop.kt diff --git a/kotlinx-coroutines-core/common/src/EventLoop.common.kt b/kotlinx-coroutines-core/common/src/EventLoop.common.kt index 3f50dda1f8..fbd478f493 100644 --- a/kotlinx-coroutines-core/common/src/EventLoop.common.kt +++ b/kotlinx-coroutines-core/common/src/EventLoop.common.kt @@ -66,7 +66,9 @@ internal abstract class EventLoop : CoroutineDispatcher() { public fun processUnconfinedEvent(): Boolean { val queue = unconfinedQueue ?: return false val task = queue.removeFirstOrNull() ?: return false - task.run() + platformAutoreleasePool { + task.run() + } return true } /** @@ -271,7 +273,9 @@ internal abstract class EventLoopImplBase: EventLoopImplPlatform(), Delay { // then process one event from queue val task = dequeue() if (task != null) { - task.run() + platformAutoreleasePool { + task.run() + } return 0 } return nextTime @@ -526,6 +530,17 @@ internal abstract class EventLoopImplBase: EventLoopImplPlatform(), Delay { internal expect fun createEventLoop(): EventLoop +/** + * Used by Darwin targets to wrap a [Runnable.run] call in an Objective-C Autorelease Pool. It is a no-op on JVM, JS and + * non-Darwin native targets. + * + * Coroutines on Darwin targets can call into the Objective-C world, where a callee may push a to-be-returned object to + * the Autorelease Pool, so as to avoid a premature ARC release before it reaches the caller. This means the pool must + * be eventually drained to avoid leaks. Since Kotlin Coroutines does not use [NSRunLoop], which provides automatic + * pool management, it must manage the pool creation and pool drainage manually. + */ +internal expect inline fun platformAutoreleasePool(crossinline block: () -> Unit) + internal expect fun nanoTime(): Long internal expect object DefaultExecutor { diff --git a/kotlinx-coroutines-core/js/src/EventLoop.kt b/kotlinx-coroutines-core/js/src/EventLoop.kt index 0039678a93..f90774cc41 100644 --- a/kotlinx-coroutines-core/js/src/EventLoop.kt +++ b/kotlinx-coroutines-core/js/src/EventLoop.kt @@ -8,6 +8,8 @@ import kotlin.coroutines.* internal actual fun createEventLoop(): EventLoop = UnconfinedEventLoop() +internal actual inline fun platformAutoreleasePool(crossinline block: () -> Unit) = block() + internal actual fun nanoTime(): Long = unsupported() internal class UnconfinedEventLoop : EventLoop() { diff --git a/kotlinx-coroutines-core/jvm/src/EventLoop.kt b/kotlinx-coroutines-core/jvm/src/EventLoop.kt index d86f632a69..bd4916e776 100644 --- a/kotlinx-coroutines-core/jvm/src/EventLoop.kt +++ b/kotlinx-coroutines-core/jvm/src/EventLoop.kt @@ -25,6 +25,8 @@ internal class BlockingEventLoop( internal actual fun createEventLoop(): EventLoop = BlockingEventLoop(Thread.currentThread()) +internal actual inline fun platformAutoreleasePool(crossinline block: () -> Unit) = block() + /** * Processes next event in the current thread's event loop. * diff --git a/kotlinx-coroutines-core/nativeDarwin/src/EventLoop.kt b/kotlinx-coroutines-core/nativeDarwin/src/EventLoop.kt new file mode 100644 index 0000000000..00a155de17 --- /dev/null +++ b/kotlinx-coroutines-core/nativeDarwin/src/EventLoop.kt @@ -0,0 +1,9 @@ +/* + * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines + +import kotlinx.cinterop.* + +internal actual inline fun platformAutoreleasePool(crossinline block: () -> Unit): Unit = autoreleasepool { block() } diff --git a/kotlinx-coroutines-core/nativeDarwin/test/AutoreleaseLeakTest.kt b/kotlinx-coroutines-core/nativeDarwin/test/AutoreleaseLeakTest.kt new file mode 100644 index 0000000000..7a5c4c7c93 --- /dev/null +++ b/kotlinx-coroutines-core/nativeDarwin/test/AutoreleaseLeakTest.kt @@ -0,0 +1,40 @@ +/* + * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines + +import kotlin.native.ref.* +import kotlin.native.concurrent.* +import kotlin.native.internal.* +import platform.Foundation.* +import platform.darwin.NSObject +import kotlinx.cinterop.* +import kotlin.test.* + +class AutoreleaseLeakTest : TestBase() { + private val testThread = currentThread() + + @Test + fun testObjCAutorelease() { + val weakRef = AtomicReference?>(null) + + runTest { + withContext(Dispatchers.Default) { + val job = launch { + assertNotEquals(testThread, currentThread()) + val objcObj = NSObject() + weakRef.value = WeakReference(objcObj).freeze() + + // Emulate an autorelease return value in native code. + objc_retainAutoreleaseReturnValue(objcObj.objcPtr()) + } + job.join() + GC.collect() + } + + assertNotNull(weakRef.value) + assertNull(weakRef.value?.value) + } + } +} diff --git a/kotlinx-coroutines-core/nativeOther/src/EventLoop.kt b/kotlinx-coroutines-core/nativeOther/src/EventLoop.kt new file mode 100644 index 0000000000..1dcc66ce1e --- /dev/null +++ b/kotlinx-coroutines-core/nativeOther/src/EventLoop.kt @@ -0,0 +1,7 @@ +/* + * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines + +internal actual inline fun platformAutoreleasePool(crossinline block: () -> Unit): Unit = block()