From d738646a61d530cbc013736529adc770016f2ae2 Mon Sep 17 00:00:00 2001 From: Abduqodiri Qurbonzoda Date: Fri, 31 Mar 2023 16:04:26 +0000 Subject: [PATCH] [K/N] Move kotlin.native.internal.Cleaner to kotlin.native.ref.Cleaner As a part of efforts to stabilize Native stdlib #KT-55765. Co-authored-by: Alexander Shabalin Co-authored-by: Ilya Gorbunov Merge-request: KT-MR-9347 Merged-by: Abduqodiri Qurbonzoda --- .../kotlin/backend/konan/RuntimeNames.kt | 2 +- .../jetbrains/kotlin/backend/konan/ir/Ir.kt | 3 +- .../tests/runtime/basic/cleaner_basic.kt | 2 + .../basic/cleaner_in_main_with_checker.kt | 3 +- .../basic/cleaner_in_main_without_checker.kt | 2 +- .../basic/cleaner_in_tls_main_with_checker.kt | 3 +- .../cleaner_in_tls_main_without_checker.kt | 3 +- .../runtime/basic/cleaner_in_tls_worker.kt | 2 + .../basic/cleaner_leak_with_checker.kt | 3 +- .../basic/cleaner_leak_without_checker.kt | 4 +- .../tests/runtime/basic/cleaner_workers.kt | 2 + .../kotlin/kotlin/native/internal/Cleaner.kt | 36 ++--- .../main/kotlin/kotlin/native/ref/Cleaner.kt | 123 ++++++++++++++++++ 13 files changed, 151 insertions(+), 37 deletions(-) create mode 100644 kotlin-native/runtime/src/main/kotlin/kotlin/native/ref/Cleaner.kt diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/RuntimeNames.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/RuntimeNames.kt index 8996391cfad..b0b30cbdc9e 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/RuntimeNames.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/RuntimeNames.kt @@ -25,5 +25,5 @@ object RuntimeNames { val kotlinNativeCoroutinesInternalPackageName = FqName.fromSegments(listOf("kotlin", "coroutines", "native", "internal")) val associatedObjectKey = FqName("kotlin.reflect.AssociatedObjectKey") val typedIntrinsicAnnotation = FqName("kotlin.native.internal.TypedIntrinsic") - val cleaner = FqName("kotlin.native.internal.Cleaner") + val cleaner = FqName("kotlin.native.ref.Cleaner") } diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/ir/Ir.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/ir/Ir.kt index f618b224a06..57e63a8c4df 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/ir/Ir.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/ir/Ir.kt @@ -231,7 +231,8 @@ internal abstract class KonanSymbols( val executeImpl = irBuiltIns.findFunctions(Name.identifier("executeImpl"),"kotlin", "native", "concurrent").single() - val createCleaner = internalFunction("createCleaner") + val createCleaner = + irBuiltIns.findFunctions(Name.identifier("createCleaner"),"kotlin", "native", "ref").single() val areEqualByValue = internalFunctions("areEqualByValue").associateBy { it.descriptor.valueParameters[0].type.computePrimitiveBinaryTypeOrNull()!! diff --git a/kotlin-native/backend.native/tests/runtime/basic/cleaner_basic.kt b/kotlin-native/backend.native/tests/runtime/basic/cleaner_basic.kt index 3942c8eb692..9678ae6d94f 100644 --- a/kotlin-native/backend.native/tests/runtime/basic/cleaner_basic.kt +++ b/kotlin-native/backend.native/tests/runtime/basic/cleaner_basic.kt @@ -11,6 +11,8 @@ import kotlin.test.* import kotlin.native.internal.* import kotlin.native.concurrent.* import kotlin.native.ref.WeakReference +import kotlin.native.ref.Cleaner +import kotlin.native.ref.createCleaner class AtomicBoolean(initialValue: Boolean) { private val impl = AtomicInt(if (initialValue) 1 else 0) diff --git a/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_main_with_checker.kt b/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_main_with_checker.kt index e15f7a3210e..7cb79424169 100644 --- a/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_main_with_checker.kt +++ b/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_main_with_checker.kt @@ -4,7 +4,8 @@ */ @file:OptIn(ExperimentalStdlibApi::class) -import kotlin.native.internal.* +import kotlin.native.ref.Cleaner +import kotlin.native.ref.createCleaner import kotlin.native.Platform fun main() { diff --git a/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_main_without_checker.kt b/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_main_without_checker.kt index ce163b605f7..c8463daaad9 100644 --- a/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_main_without_checker.kt +++ b/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_main_without_checker.kt @@ -4,7 +4,7 @@ */ @file:OptIn(ExperimentalStdlibApi::class) -import kotlin.native.internal.* +import kotlin.native.ref.createCleaner import kotlin.native.Platform fun main() { diff --git a/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_tls_main_with_checker.kt b/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_tls_main_with_checker.kt index 718017a823e..5c893021335 100644 --- a/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_tls_main_with_checker.kt +++ b/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_tls_main_with_checker.kt @@ -7,7 +7,8 @@ import kotlin.test.* import kotlin.native.concurrent.* -import kotlin.native.internal.* +import kotlin.native.ref.Cleaner +import kotlin.native.ref.createCleaner import kotlin.native.Platform @ThreadLocal diff --git a/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_tls_main_without_checker.kt b/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_tls_main_without_checker.kt index 54267f712c6..82da27b8ca6 100644 --- a/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_tls_main_without_checker.kt +++ b/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_tls_main_without_checker.kt @@ -7,7 +7,8 @@ import kotlin.test.* import kotlin.native.concurrent.* -import kotlin.native.internal.* +import kotlin.native.ref.Cleaner +import kotlin.native.ref.createCleaner import kotlin.native.Platform @ThreadLocal diff --git a/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_tls_worker.kt b/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_tls_worker.kt index f43ee07f148..6d0a4b7168a 100644 --- a/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_tls_worker.kt +++ b/kotlin-native/backend.native/tests/runtime/basic/cleaner_in_tls_worker.kt @@ -8,6 +8,8 @@ import kotlin.test.* import kotlin.native.concurrent.* import kotlin.native.internal.* +import kotlin.native.ref.Cleaner +import kotlin.native.ref.createCleaner @ThreadLocal var tlsCleaner: Cleaner? = null diff --git a/kotlin-native/backend.native/tests/runtime/basic/cleaner_leak_with_checker.kt b/kotlin-native/backend.native/tests/runtime/basic/cleaner_leak_with_checker.kt index b0bd8060574..8a2d5909de0 100644 --- a/kotlin-native/backend.native/tests/runtime/basic/cleaner_leak_with_checker.kt +++ b/kotlin-native/backend.native/tests/runtime/basic/cleaner_leak_with_checker.kt @@ -6,7 +6,8 @@ import kotlin.test.* -import kotlin.native.internal.* +import kotlin.native.ref.Cleaner +import kotlin.native.ref.createCleaner import kotlin.native.Platform // This cleaner won't be run, because it's deinitialized with globals after diff --git a/kotlin-native/backend.native/tests/runtime/basic/cleaner_leak_without_checker.kt b/kotlin-native/backend.native/tests/runtime/basic/cleaner_leak_without_checker.kt index 76eb9c1e461..1c9cf7fef0b 100644 --- a/kotlin-native/backend.native/tests/runtime/basic/cleaner_leak_without_checker.kt +++ b/kotlin-native/backend.native/tests/runtime/basic/cleaner_leak_without_checker.kt @@ -6,8 +6,8 @@ import kotlin.test.* -import kotlin.native.internal.* -import kotlin.native.Platform +import kotlin.native.ref.Cleaner +import kotlin.native.ref.createCleaner // This cleaner won't be run, because it's deinitialized with globals after // cleaners are disabled. diff --git a/kotlin-native/backend.native/tests/runtime/basic/cleaner_workers.kt b/kotlin-native/backend.native/tests/runtime/basic/cleaner_workers.kt index 964d3cc272e..a5c4f573df9 100644 --- a/kotlin-native/backend.native/tests/runtime/basic/cleaner_workers.kt +++ b/kotlin-native/backend.native/tests/runtime/basic/cleaner_workers.kt @@ -11,6 +11,8 @@ import kotlin.test.* import kotlin.native.internal.* import kotlin.native.concurrent.* import kotlin.native.ref.WeakReference +import kotlin.native.ref.Cleaner +import kotlin.native.ref.createCleaner class AtomicBoolean(initialValue: Boolean) { private val impl = AtomicInt(if (initialValue) 1 else 0) diff --git a/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/Cleaner.kt b/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/Cleaner.kt index 1809dcfe0c2..2a199b15fc1 100644 --- a/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/Cleaner.kt +++ b/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/Cleaner.kt @@ -8,6 +8,8 @@ package kotlin.native.internal import kotlin.native.concurrent.* import kotlinx.cinterop.NativePtr +@Deprecated("Use kotlin.native.ref.Cleaner instead.", ReplaceWith("kotlin.native.ref.Cleaner")) +@DeprecatedSinceKotlin(warningSince = "1.9") public interface Cleaner /** @@ -67,26 +69,14 @@ public interface Cleaner */ // TODO: Consider just annotating the lambda argument rather than hardcoding checking // by function name in the compiler. +@Deprecated("Use kotlin.native.ref.createCleaner instead.", ReplaceWith("kotlin.native.ref.createCleaner(argument, block)")) +@DeprecatedSinceKotlin(warningSince = "1.9") +@Suppress("DEPRECATION") @ExperimentalStdlibApi @ExportForCompiler @OptIn(FreezingIsDeprecated::class) -fun createCleaner(argument: T, block: (T) -> Unit): Cleaner { - if (!argument.isShareable()) - throw IllegalArgumentException("$argument must be shareable") - - val clean = { - // TODO: Maybe if this fails with exception, it should be (optionally) reported. - block(argument) - }.freeze() - - // Make sure there's an extra reference to clean, so it's definitely alive when CleanerImpl is destroyed. - val cleanPtr = createStablePointer(clean) - - // Make sure cleaner worker is initialized. - getCleanerWorker() - - return CleanerImpl(cleanPtr).freeze() -} +fun createCleaner(argument: T, block: (T) -> Unit): Cleaner = + kotlin.native.ref.createCleanerImpl(argument, block) as Cleaner /** * Perform GC on a worker that executes Cleaner blocks. @@ -107,7 +97,7 @@ fun waitCleanerWorker() = }.result @GCUnsafeCall("Kotlin_CleanerImpl_getCleanerWorker") -external private fun getCleanerWorker(): Worker +external internal fun getCleanerWorker(): Worker @ExportForCppRuntime("Kotlin_CleanerImpl_shutdownCleanerWorker") private fun shutdownCleanerWorker(worker: Worker, executeScheduledCleaners: Boolean) { @@ -118,13 +108,3 @@ private fun shutdownCleanerWorker(worker: Worker, executeScheduledCleaners: Bool private fun createCleanerWorker(): Worker { return Worker.start(errorReporting = false, name = "Cleaner worker") } - -@NoReorderFields -@ExportTypeInfo("theCleanerImplTypeInfo") -@HasFinalizer -private class CleanerImpl( - private val cleanPtr: NativePtr, -): Cleaner {} - -@GCUnsafeCall("CreateStablePointer") -external private fun createStablePointer(obj: Any): NativePtr diff --git a/kotlin-native/runtime/src/main/kotlin/kotlin/native/ref/Cleaner.kt b/kotlin-native/runtime/src/main/kotlin/kotlin/native/ref/Cleaner.kt new file mode 100644 index 00000000000..bd29b32d29f --- /dev/null +++ b/kotlin-native/runtime/src/main/kotlin/kotlin/native/ref/Cleaner.kt @@ -0,0 +1,123 @@ +/* + * 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. + */ + +package kotlin.native.ref + +import kotlin.native.concurrent.isShareable +import kotlin.native.concurrent.freeze +import kotlin.native.internal.* +import kotlinx.cinterop.NativePtr + +/** + * The marker interface for objects that have a cleanup action associated with them. + * + * Use [createCleaner] to create an instance of this type. + */ +@ExperimentalStdlibApi +@SinceKotlin("1.9") +public sealed interface Cleaner + +/** + * Creates a [Cleaner] object that runs [cleanupAction] with given [resource] some time after its deallocation. + * + * Example of usage: + * ``` + * class ResourceWrapper { + * private val resource = Resource() + * + * private val cleaner = createCleaner(resource) { it.dispose() } + * } + * ``` + * + * When `ResourceWrapper` becomes unused and gets deallocated, its `cleaner` + * is also deallocated, and the resource is disposed later. + * + * It is not specified which thread runs [cleanupAction], as well as whether two or more + * cleanup actions from different cleaners can be run in parallel. + * + * Note: if [resource] refers (directly or indirectly) the cleaner, then both + * might leak, and the [cleanupAction] will not be called in this case. + * For example, the code below has a leak: + * ``` + * class LeakingResourceWrapper { + * private val resource = Resource() + * private val cleaner = createCleaner(this) { it.resource.dispose() } + * } + * ``` + * In this case cleaner's argument (`LeakingResourceWrapper`) can't be deallocated + * until [cleanupAction] (`it.resource.dispose()`) is executed, which can happen only strictly after + * the cleaner is deallocated, which can't happen until `LeakingResourceWrapper` + * is deallocated. So the requirements on object deallocations are contradictory + * in this case, which can't be handled gracefully. The cleanup action + * is not executed then, and cleaner and its argument might leak + * (depending on the implementation). + * The same problem occures when [cleanupAction] captures a value that refers (directly or indirectly) the cleaner: + * ``` + * class LeakingResourceWrapper { + * private val cleaner = createCleaner(...) { + * doSomething() + * ... + * } + * + * private fun doSomething() { + * ... + * } + * } + * ``` + * In the example above the cleanup lambda implicitly captures `this` object to call `doSomething()`. + * + * [cleanupAction] should not use `@ThreadLocal` globals, because it may + * be executed on a different thread. + * + * If [cleanupAction] throws an exception, the behavior is unspecified. + * + * Cleaners cannot be used to perform actions during the program shutdown: + * * cleaners that are referenced from globals will not be garbage collected at all, + * * cleaners that become unreferenced just before exiting main() might not be garbage collected, + because the GC might not get a chance to run. + * + * @param resource an object for which to perform [cleanupAction] + * @param cleanupAction a cleanup to perform on [resource]. Must not capture anything. + */ +// TODO: Consider just annotating the lambda argument rather than hardcoding checking +// by function name in the compiler. +@ExperimentalStdlibApi +@SinceKotlin("1.9") +@ExportForCompiler +public fun createCleaner(resource: T, cleanupAction: (resource: T) -> Unit): Cleaner = + createCleanerImpl(resource, cleanupAction) + +@ExperimentalStdlibApi +@OptIn(FreezingIsDeprecated::class) +internal fun createCleanerImpl(resource: T, cleanupAction: (T) -> Unit): Cleaner { + if (!resource.isShareable()) + throw IllegalArgumentException("$resource must be shareable") + + val clean = { + // TODO: Maybe if this fails with exception, it should be (optionally) reported. + cleanupAction(resource) + }.freeze() + + // Make sure there's an extra reference to clean, so it's definitely alive when CleanerImpl is destroyed. + val cleanPtr = createStablePointer(clean) + + // Make sure cleaner worker is initialized. + getCleanerWorker() + + return CleanerImpl(cleanPtr).freeze() +} + +@Suppress("DEPRECATION") +@ExperimentalStdlibApi +@NoReorderFields +@ExportTypeInfo("theCleanerImplTypeInfo") +@HasFinalizer +private class CleanerImpl( + private val cleanPtr: NativePtr, +): Cleaner, kotlin.native.internal.Cleaner {} + + +@GCUnsafeCall("CreateStablePointer") +external private fun createStablePointer(obj: Any): NativePtr \ No newline at end of file