From eb576258e32df2451642f50b10a0f8c55688c022 Mon Sep 17 00:00:00 2001 From: Alexander Shabalin Date: Thu, 22 Oct 2020 13:23:30 +0300 Subject: [PATCH] Fix cleaner tests on mingw (#4457) --- backend.native/tests/build.gradle | 1 + .../tests/runtime/basic/cleaner_basic.kt | 29 +++++-------------- .../runtime/basic/cleaner_in_tls_worker.kt | 8 ++--- .../tests/runtime/basic/cleaner_workers.kt | 29 ++++--------------- runtime/src/main/cpp/Worker.cpp | 14 ++++++--- .../kotlin/native/concurrent/Internal.kt | 5 ++++ .../kotlin/kotlin/native/internal/Cleaner.kt | 9 ++++++ 7 files changed, 41 insertions(+), 54 deletions(-) diff --git a/backend.native/tests/build.gradle b/backend.native/tests/build.gradle index 9c9c5a747d7..0f268e0e964 100644 --- a/backend.native/tests/build.gradle +++ b/backend.native/tests/build.gradle @@ -962,6 +962,7 @@ standaloneTest("cleaner_in_tls_main_with_checker") { standaloneTest("cleaner_in_tls_worker") { enabled = (project.testTarget != 'wasm32') // Cleaners need workers source = "runtime/basic/cleaner_in_tls_worker.kt" + flags = ['-Xopt-in=kotlin.native.internal.InternalForKotlinNative'] } standaloneTest("worker_bound_reference0") { diff --git a/backend.native/tests/runtime/basic/cleaner_basic.kt b/backend.native/tests/runtime/basic/cleaner_basic.kt index e68a78a5ea0..b7dd61581ff 100644 --- a/backend.native/tests/runtime/basic/cleaner_basic.kt +++ b/backend.native/tests/runtime/basic/cleaner_basic.kt @@ -2,7 +2,7 @@ * Copyright 2010-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license * that can be found in the LICENSE file. */ -@file:OptIn(ExperimentalTime::class, ExperimentalStdlibApi::class) +@file:OptIn(ExperimentalStdlibApi::class) package runtime.basic.cleaner_basic @@ -11,7 +11,6 @@ import kotlin.test.* import kotlin.native.internal.* import kotlin.native.concurrent.* import kotlin.native.ref.WeakReference -import kotlin.time.* class AtomicBoolean(initialValue: Boolean) { private val impl = AtomicInt(if (initialValue) 1 else 0) @@ -111,20 +110,6 @@ fun testCleanerFailWithNonShareableArgument() { } } -inline fun tryWithTimeout(timeoutSeconds: Int, f: () -> Unit): Unit { - val timeout = TimeSource.Monotonic.markNow() + timeoutSeconds.seconds - while (true) { - try { - f() - return - } catch (e: Throwable) { - if (timeout.hasPassedNow()) { - throw e - } - } - } -} - @Test fun testCleanerCleansWithoutGC() { val called = AtomicBoolean(false); @@ -145,12 +130,12 @@ fun testCleanerCleansWithoutGC() { GC.collect() assertNull(cleanerWeak!!.value) - tryWithTimeout(3) { - GC.collect() // Collect local stack (from previous iteration) - assertTrue(called.value) - // If this fails, GC has somehow ran on the cleaners worker. - assertNotNull(funBoxWeak!!.value) - } + + waitCleanerWorker() + + assertTrue(called.value) + // If this fails, GC has somehow ran on the cleaners worker. + assertNotNull(funBoxWeak!!.value) } val globalInt = AtomicInt(0) diff --git a/backend.native/tests/runtime/basic/cleaner_in_tls_worker.kt b/backend.native/tests/runtime/basic/cleaner_in_tls_worker.kt index e47116352aa..e6ef93faeba 100644 --- a/backend.native/tests/runtime/basic/cleaner_in_tls_worker.kt +++ b/backend.native/tests/runtime/basic/cleaner_in_tls_worker.kt @@ -2,13 +2,12 @@ * Copyright 2010-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license * that can be found in the LICENSE file. */ -@file:OptIn(ExperimentalTime::class, ExperimentalStdlibApi::class) +@file:OptIn(ExperimentalStdlibApi::class) import kotlin.test.* import kotlin.native.concurrent.* import kotlin.native.internal.* -import kotlin.time.* @ThreadLocal var tlsCleaner: Cleaner? = null @@ -25,9 +24,8 @@ fun main() { } worker.requestTermination().result - - val timeout = TimeSource.Monotonic.markNow() + 3.seconds - while (value.value == 0 || timeout.hasNotPassedNow()) {} + waitWorkerTermination(worker) + waitCleanerWorker() assertEquals(42, value.value) } diff --git a/backend.native/tests/runtime/basic/cleaner_workers.kt b/backend.native/tests/runtime/basic/cleaner_workers.kt index b82c6644895..f87e4ab2dbb 100644 --- a/backend.native/tests/runtime/basic/cleaner_workers.kt +++ b/backend.native/tests/runtime/basic/cleaner_workers.kt @@ -2,7 +2,7 @@ * Copyright 2010-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license * that can be found in the LICENSE file. */ -@file:OptIn(ExperimentalTime::class, ExperimentalStdlibApi::class) +@file:OptIn(ExperimentalStdlibApi::class) package runtime.basic.cleaner_workers @@ -11,7 +11,6 @@ import kotlin.test.* import kotlin.native.internal.* import kotlin.native.concurrent.* import kotlin.native.ref.WeakReference -import kotlin.time.* class AtomicBoolean(initialValue: Boolean) { private val impl = AtomicInt(if (initialValue) 1 else 0) @@ -59,20 +58,6 @@ fun testCleanerDestroyInChild() { worker.requestTermination().result } -inline fun tryWithTimeout(timeoutSeconds: Int, f: () -> Unit): Unit { - val timeout = TimeSource.Monotonic.markNow() + timeoutSeconds.seconds - while (true) { - try { - f() - return - } catch (e: Throwable) { - if (timeout.hasPassedNow()) { - throw e - } - } - } -} - @Test fun testCleanerDestroyWithChild() { val worker = Worker.start() @@ -92,15 +77,13 @@ fun testCleanerDestroyWithChild() { GC.collect() worker.requestTermination().result + waitWorkerTermination(worker) - tryWithTimeout(3) { - GC.collect() // Collect local stack (from previous iteration) - performGCOnCleanerWorker() // Collect cleaners stack + performGCOnCleanerWorker() // Collect cleaners stack - assertNull(cleanerWeak!!.value) - assertTrue(called.value) - assertNull(funBoxWeak!!.value) - } + assertNull(cleanerWeak!!.value) + assertTrue(called.value) + assertNull(funBoxWeak!!.value) } @Test diff --git a/runtime/src/main/cpp/Worker.cpp b/runtime/src/main/cpp/Worker.cpp index 0dc6ada9f3f..1da5f31ee9d 100644 --- a/runtime/src/main/cpp/Worker.cpp +++ b/runtime/src/main/cpp/Worker.cpp @@ -474,12 +474,14 @@ class State { } template - void waitNativeWorkersTerminationUnlocked(F waitForWorker) { + void waitNativeWorkersTerminationUnlocked(bool checkLeaks, F waitForWorker) { std::vector> workersToWait; { Locker locker(&lock_); - checkNativeWorkersLeakLocked(); + if (checkLeaks) { + checkNativeWorkersLeakLocked(); + } for (auto& kvp : terminating_native_workers_) { RuntimeAssert(!pthread_equal(kvp.second, pthread_self()), "Native worker is joining with itself"); @@ -742,13 +744,13 @@ void WorkerDestroyThreadDataIfNeeded(KInt id) { void WaitNativeWorkersTermination() { #if WITH_WORKERS - theState()->waitNativeWorkersTerminationUnlocked([](KInt worker) { return true; }); + theState()->waitNativeWorkersTerminationUnlocked(true, [](KInt worker) { return true; }); #endif } void WaitNativeWorkerTermination(KInt id) { #if WITH_WORKERS - theState()->waitNativeWorkersTerminationUnlocked([id](KInt worker) { return worker == id; }); + theState()->waitNativeWorkersTerminationUnlocked(false, [id](KInt worker) { return worker == id; }); #endif } @@ -1060,4 +1062,8 @@ void Kotlin_Worker_ensureNeverFrozen(KRef object) { EnsureNeverFrozen(object); } +void Kotlin_Worker_waitTermination(KInt id) { + WaitNativeWorkerTermination(id); +} + } // extern "C" diff --git a/runtime/src/main/kotlin/kotlin/native/concurrent/Internal.kt b/runtime/src/main/kotlin/kotlin/native/concurrent/Internal.kt index eb13f3309d6..0d13130f25b 100644 --- a/runtime/src/main/kotlin/kotlin/native/concurrent/Internal.kt +++ b/runtime/src/main/kotlin/kotlin/native/concurrent/Internal.kt @@ -7,6 +7,7 @@ package kotlin.native.concurrent import kotlin.native.internal.DescribeObjectForDebugging import kotlin.native.internal.ExportForCppRuntime +import kotlin.native.internal.InternalForKotlinNative import kotlin.native.internal.debugDescription import kotlin.native.identityHashCode import kotlin.reflect.KClass @@ -100,3 +101,7 @@ internal fun ThrowIllegalObjectSharingException(typeInfo: NativePtr, address: Na @SymbolName("Kotlin_AtomicReference_checkIfFrozen") external internal fun checkIfFrozen(ref: Any?) + +@InternalForKotlinNative +@SymbolName("Kotlin_Worker_waitTermination") +external public fun waitWorkerTermination(worker: Worker) diff --git a/runtime/src/main/kotlin/kotlin/native/internal/Cleaner.kt b/runtime/src/main/kotlin/kotlin/native/internal/Cleaner.kt index b4f72e6675b..382dd03d572 100644 --- a/runtime/src/main/kotlin/kotlin/native/internal/Cleaner.kt +++ b/runtime/src/main/kotlin/kotlin/native/internal/Cleaner.kt @@ -96,6 +96,15 @@ fun performGCOnCleanerWorker() = GC.collect() }.result +/** + * Wait for a worker that executes Cleaner blocks to complete its scheduled tasks. + */ +@InternalForKotlinNative +fun waitCleanerWorker() = + getCleanerWorker().execute(TransferMode.SAFE, {}) { + Unit + }.result + @SymbolName("Kotlin_CleanerImpl_getCleanerWorker") external private fun getCleanerWorker(): Worker