From 783517dbe851fd4cb186948bd1d019c834179713 Mon Sep 17 00:00:00 2001 From: Alexander Shabalin Date: Tue, 8 Dec 2020 10:17:52 +0300 Subject: [PATCH] Fix unchecked runtime shutdown (#4575) --- .../backend.native/tests/build.gradle | 22 ++++++++++++++++++ .../leakMemoryWithRunningThread/checked.kt | 23 +++++++++++++++++++ .../leakMemory.cpp | 15 ++++++++++++ .../leakMemory.def | 5 ++++ .../leakMemoryWithRunningThread/leakMemory.h | 9 ++++++++ .../leakMemoryWithRunningThread/unchecked.kt | 23 +++++++++++++++++++ .../runtime/src/main/cpp/Runtime.cpp | 13 +++++++---- 7 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/checked.kt create mode 100644 kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/leakMemory.cpp create mode 100644 kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/leakMemory.def create mode 100644 kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/leakMemory.h create mode 100644 kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/unchecked.kt diff --git a/kotlin-native/backend.native/tests/build.gradle b/kotlin-native/backend.native/tests/build.gradle index 98d48b094bd..a38d5dfc86e 100644 --- a/kotlin-native/backend.native/tests/build.gradle +++ b/kotlin-native/backend.native/tests/build.gradle @@ -3752,6 +3752,12 @@ createInterop("kt43265") { it.defFile 'interop/kt43265/kt43265.def' } +createInterop("leakMemoryWithRunningThread") { + it.defFile 'interop/leakMemoryWithRunningThread/leakMemory.def' + it.headers "$projectDir/interop/leakMemoryWithRunningThread/leakMemory.h" + it.extraOpts "-Xcompile-source", "$projectDir/interop/leakMemoryWithRunningThread/leakMemory.cpp" +} + if (PlatformInfo.isAppleTarget(project)) { createInterop("objcSmoke") { it.defFile 'interop/objc/objcSmoke.def' @@ -4061,6 +4067,22 @@ interopTest("interop_kt43265") { source = "interop/kt43265/usage.kt" } +interopTest("interop_leakMemoryWithRunningThreadUnchecked") { + disabled = project.globalTestArgs.contains('-opt') || (project.testTarget == 'wasm32') // Needs debug build. + interop = 'leakMemoryWithRunningThread' + source = "interop/leakMemoryWithRunningThread/unchecked.kt" + flags = ['-g'] +} + +interopTest("interop_leakMemoryWithRunningThreadChecked") { + disabled = project.globalTestArgs.contains('-opt') || (project.testTarget == 'wasm32') // Needs debug build. + interop = 'leakMemoryWithRunningThread' + source = "interop/leakMemoryWithRunningThread/checked.kt" + flags = ['-g'] + expectedExitStatusChecker = { it != 0 } + outputChecker = { s -> s.contains("Cannot run checkers when there are 1 alive runtimes at the shutdown") } +} + standaloneTest("interop_pinning") { disabled = (project.testTarget == 'wasm32') // Uses exceptions. source = "interop/basics/pinning.kt" diff --git a/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/checked.kt b/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/checked.kt new file mode 100644 index 00000000000..309fce99f4a --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/checked.kt @@ -0,0 +1,23 @@ +import leakMemory.* +import kotlin.native.concurrent.* +import kotlin.test.* +import kotlinx.cinterop.* + +val global = AtomicInt(0) + +fun ensureInititalized() { + kotlin.native.initRuntimeIfNeeded() + // Leak memory + StableRef.create(Any()) + global.value = 1 +} + +fun main() { + kotlin.native.internal.Debugging.forceCheckedShutdown = true + assertTrue(global.value == 0) + // Created a thread, made sure Kotlin is initialized there. + test_RunInNewThread(staticCFunction(::ensureInititalized)) + assertTrue(global.value == 1) + // Now exiting. With checked shutdown we will fail, complaining there're + // unfinished threads with runtimes. +} diff --git a/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/leakMemory.cpp b/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/leakMemory.cpp new file mode 100644 index 00000000000..7e213990ab3 --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/leakMemory.cpp @@ -0,0 +1,15 @@ +#include "leakMemory.h" + +#include +#include + +extern "C" void test_RunInNewThread(void (*f)()) { + std::atomic haveRun(false); + std::thread t([f, &haveRun]() { + f(); + haveRun = true; + while (true) {} + }); + t.detach(); + while (!haveRun) {} +} diff --git a/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/leakMemory.def b/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/leakMemory.def new file mode 100644 index 00000000000..e487579d85a --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/leakMemory.def @@ -0,0 +1,5 @@ +package leakMemory + +--- + +void test_RunInNewThread(void (*)()); diff --git a/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/leakMemory.h b/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/leakMemory.h new file mode 100644 index 00000000000..2342ed9c920 --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/leakMemory.h @@ -0,0 +1,9 @@ +#ifdef __cplusplus +extern "C" { +#endif + +void test_RunInNewThread(void (*)()); + +#ifdef __cplusplus +} +#endif diff --git a/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/unchecked.kt b/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/unchecked.kt new file mode 100644 index 00000000000..db770cabe48 --- /dev/null +++ b/kotlin-native/backend.native/tests/interop/leakMemoryWithRunningThread/unchecked.kt @@ -0,0 +1,23 @@ +import leakMemory.* +import kotlin.native.concurrent.* +import kotlin.test.* +import kotlinx.cinterop.* + +val global = AtomicInt(0) + +fun ensureInititalized() { + kotlin.native.initRuntimeIfNeeded() + // Leak memory + StableRef.create(Any()) + global.value = 1 +} + +fun main() { + kotlin.native.internal.Debugging.forceCheckedShutdown = false + assertTrue(global.value == 0) + // Created a thread, made sure Kotlin is initialized there. + test_RunInNewThread(staticCFunction(::ensureInititalized)) + assertTrue(global.value == 1) + // Now exiting. With unchecked shutdown, we exit quietly, even though there're + // unfinished threads with runtimes. +} diff --git a/kotlin-native/runtime/src/main/cpp/Runtime.cpp b/kotlin-native/runtime/src/main/cpp/Runtime.cpp index 42e6594c0a4..9a1f6feb01d 100644 --- a/kotlin-native/runtime/src/main/cpp/Runtime.cpp +++ b/kotlin-native/runtime/src/main/cpp/Runtime.cpp @@ -227,23 +227,28 @@ void Kotlin_shutdownRuntime() { auto lastStatus = compareAndSwap(&globalRuntimeStatus, kGlobalRuntimeRunning, kGlobalRuntimeShutdown); RuntimeAssert(lastStatus == kGlobalRuntimeRunning, "Invalid runtime status for shutdown"); + bool canDestroyRuntime = true; + // TODO: When legacy mode is gone, this `if` will become unnecessary. if (Kotlin_forceCheckedShutdown() || Kotlin_memoryLeakCheckerEnabled() || Kotlin_cleanersLeakCheckerEnabled()) { // First make sure workers are gone. WaitNativeWorkersTermination(); + // Now check for existence of any other runtimes. + auto otherRuntimesCount = atomicGet(&aliveRuntimesCount) - 1; + RuntimeAssert(otherRuntimesCount >= 0, "Cannot be negative"); if (Kotlin_forceCheckedShutdown()) { - // Now check for existence of any other runtimes. - auto otherRuntimesCount = atomicGet(&aliveRuntimesCount) - 1; - RuntimeAssert(otherRuntimesCount >= 0, "Cannot be negative"); if (otherRuntimesCount > 0) { konan::consoleErrorf("Cannot run checkers when there are %d alive runtimes at the shutdown", otherRuntimesCount); konan::abort(); } + } else { + // Cannot destroy runtime globally if there're some other threads with Kotlin runtime on them. + canDestroyRuntime = otherRuntimesCount == 0; } } - deinitRuntime(runtime, true); + deinitRuntime(runtime, canDestroyRuntime); ::runtimeState = kInvalidRuntime; }