[runtime] Fix possible race in terminate handler

backend.native/tests/build.gradle

@@ -3596,6 +3596,14 @@ createInterop("auxiliaryCppSources") {
     it.extraOpts "-Xsource-compiler-option", "-std=c++17"
 }
 
+createInterop("concurrentTerminate") {
+    it.defFile 'interop/concurrentTerminate/concurrentTerminate.def'
+    it.headers "$projectDir/interop/concurrentTerminate/async.h"
+    // TODO: Using `-Xcompile-source` does not imply dependency on that source, so the task will no re-run when the source is updated.
+    it.extraOpts "-Xcompile-source", "$projectDir/interop/concurrentTerminate/async.cpp"
+    it.extraOpts "-Xsource-compiler-option", "-std=c++11"
+}
+
 createInterop("incomplete_types") {
     it.defFile 'interop/incomplete_types/library.def'
     it.headers "$projectDir/interop/incomplete_types/library.h"
@@ -3857,14 +3865,21 @@ interopTest("interop_auxiliarySources") {
     interop = 'auxiliaryCppSources'
 }
 
+interopTest("interop_concurrentTerminate") {
+    disabled = (project.testTarget == 'wasm32') // No interop for wasm yet.
+    source = "interop/concurrentTerminate/main.kt"
+    interop = 'concurrentTerminate'
+    goldValue = "Reporting error!\n"
+    outputChecker = { str -> str.endsWith(goldValue) }
+    expectedExitStatus = 99
+}
+
 interopTest("interop_incompleteTypes") {
     disabled = (project.testTarget == 'wasm32') // No interop for wasm yet.
     source = "interop/incomplete_types/main.kt"
     interop = 'incomplete_types'
 }
 
-
-
 interopTest("interop_embedStaticLibraries") {
     disabled = (project.testTarget == 'wasm32') // No interop for wasm yet.
     source = "interop/embedStaticLibraries/main.kt"
@@ -4175,6 +4190,16 @@ dynamicTest("produce_dynamic") {
                 "Error handler: kotlin.Error: Expected error\n"
 }
 
+dynamicTest("interop_concurrentRuntime") {
+    disabled = (project.testTarget != null && project.testTarget != project.hostName)
+    source = "interop/concurrentTerminate/reverseInterop.kt"
+    cSource = "$projectDir/interop/concurrentTerminate/main.cpp"
+    clangTool = "clang++"
+    goldValue = "Uncaught Kotlin exception: kotlin.RuntimeException: Example"
+    outputChecker = { str -> str.startsWith(goldValue) }
+    expectedExitStatus = 99
+}
+
 task library_mismatch(type: KonanDriverTest) {
     // Does not work for cross targets yet.
     enabled = !(project.testTarget != null && project.target.name != project.hostName)

backend.native/tests/interop/concurrentTerminate/async.cpp

@@ -0,0 +1,29 @@
+#include <thread>
+#include <future>
+#include <chrono>
+#include <vector>
+#include <csignal>  // signal.h
+
+#include "async.h"
+
+int test_ConcurrentTerminate() {
+    signal(SIGABRT, *[](int){ exit(99); }); // Windows does not have sigaction
+
+    std::vector<std::future<void>> futures;
+#ifdef __linux__
+    // TODO: invalid terminate handler called from bridge on non-main thread on Linux X64
+    throw std::runtime_error("Reporting error!");
+#endif
+
+    for (size_t i = 0; i < 100; ++i) {
+        futures.emplace_back(std::async(std::launch::async,
+                [](size_t param) {
+                    std::this_thread::sleep_for(std::chrono::milliseconds(param));
+                    throw std::runtime_error("Reporting error!");
+                },
+                200 - i));
+    }
+
+    for (auto &future : futures) future.get();
+    return 0;
+}

backend.native/tests/interop/concurrentTerminate/async.h

@@ -0,0 +1,9 @@
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+int test_ConcurrentTerminate();
+
+#ifdef __cplusplus
+}
+#endif

backend.native/tests/interop/concurrentTerminate/concurrentTerminate.def

@@ -0,0 +1,5 @@
+package async
+
+---
+
+int test_ConcurrentTerminate();

backend.native/tests/interop/concurrentTerminate/main.cpp

@@ -0,0 +1,44 @@
+#include "testlib_api.h"
+
+#include <iostream>
+#include <thread>
+#include <future>
+#include <chrono>
+#include <vector>
+#include <csignal>  // signal.h
+
+using namespace std;
+
+static
+int runConcurrent() {
+
+    std::vector<std::future<void>> futures;
+
+    for (size_t i = 0; i < 100; ++i) {
+        futures.emplace_back(std::async(std::launch::async,
+                [](auto delay) {
+                    std::this_thread::sleep_for(std::chrono::milliseconds(delay));
+                    testlib_symbols()->kotlin.root.testTerminate();
+                },
+                100));
+    }
+
+    for (auto &future : futures) future.get();
+    return 0;
+}
+
+int main() {
+    signal(SIGABRT, *[](int){ exit(99); }); // Windows does not have sigaction
+
+    set_terminate([](){
+        cout << "This is the original terminate handler\n" << flush;
+        std::abort();
+    });
+
+    try {
+        runConcurrent();
+    } catch(...) {
+        std::cerr << "Unknown exception caught\n" << std::flush;
+    }
+    return 0;
+}

backend.native/tests/interop/concurrentTerminate/main.kt

@@ -0,0 +1,7 @@
+import async.*
+import kotlinx.cinterop.*
+
+fun main() {
+    test_ConcurrentTerminate()
+    println("This is not expected.")
+}

backend.native/tests/interop/concurrentTerminate/reverseInterop.kt

@@ -0,0 +1,5 @@
+import kotlin.system.exitProcess
+
+fun testTerminate() {
+    throw RuntimeException("Example")
+}

build-tools/src/main/kotlin/org/jetbrains/kotlin/KotlinNativeTest.kt

@@ -418,6 +418,8 @@ open class KonanDynamicTest : KonanStandaloneTest() {
     @Input
     lateinit var cSource: String
 
+    var clangTool = "clang"
+
     // Replace testlib_api.h and all occurrences of the testlib with the actual name of the test
     private fun processCSource(): String {
         val sourceFile = File(cSource)
@@ -439,7 +441,7 @@ open class KonanDynamicTest : KonanStandaloneTest() {
         val plugin = project.convention.getPlugin(ExecClang::class.java)
         val execResult = plugin.execKonanClang(project.testTarget, Action<ExecSpec> {
             it.workingDir = File(outputDirectory)
-            it.executable = "clang"
+            it.executable = clangTool
             val artifactsDir = "$outputDirectory/${project.testTarget}"
             it.args = listOf(processCSource(),
                     "-o", executable,

runtime/src/main/cpp/Exceptions.cpp

@@ -19,6 +19,7 @@
 #include <stdint.h>
 
 #include <exception>
+#include <unistd.h>
 
 #if KONAN_NO_EXCEPTIONS
 #define OMIT_BACKTRACE 1
@@ -113,8 +114,6 @@ SourceInfo getSourceInfo(KConstRef stackTrace, int index) {
 
 }  // namespace
 
-extern "C" {
-
 // TODO: this implementation is just a hack, e.g. the result is inexact;
 // however it is better to have an inexact stacktrace than not to have any.
 NO_INLINE OBJ_GETTER0(Kotlin_getCurrentStackTrace) {
@@ -137,7 +136,7 @@ NO_INLINE OBJ_GETTER0(Kotlin_getCurrentStackTrace) {
 
   int size = backtrace(buffer, maxSize);
   if (size < kSkipFrames)
-      return AllocArrayInstance(theNativePtrArrayTypeInfo, 0, OBJ_RESULT);
+    return AllocArrayInstance(theNativePtrArrayTypeInfo, 0, OBJ_RESULT);
 
   ObjHolder resultHolder;
   ObjHeader* result = AllocArrayInstance(theNativePtrArrayTypeInfo, size - kSkipFrames, resultHolder.slot());
@@ -235,71 +234,104 @@ void OnUnhandledException(KRef throwable) {
   }
 }
 
-#if KONAN_REPORT_BACKTRACE_TO_IOS_CRASH_LOG
-static bool terminating = false;
-static SimpleMutex terminatingMutex;
-#endif
+namespace {
 
-RUNTIME_NORETURN void TerminateWithUnhandledException(KRef throwable) {
-  OnUnhandledException(throwable);
+class {
+    /**
+     * Timeout 5 sec for concurrent (second) terminate attempt to give a chance the first one to finish.
+     * If the terminate handler hangs for 5 sec it is probably fatally broken, so let's do abnormal _Exit in that case.
+     */
+    unsigned int timeoutSec = 5;
+    int terminatingFlag = 0;
+  public:
+    template <class Fun> RUNTIME_NORETURN void operator()(Fun block) {
+      if (compareAndSet(&terminatingFlag, 0, 1)) {
+        block();
+        // block() is supposed to be NORETURN, otherwise go to normal abort()
+        konan::abort();
+      } else {
+        sleep(timeoutSec);
+        // We come here when another terminate handler hangs for 5 sec, that looks fatally broken. Go to forced exit now.
+      }
+      _Exit(EXIT_FAILURE); // force exit
+    }
+} concurrentTerminateWrapper;
 
+//! Process exception hook (if any) or just printStackTrace + write crash log
+void processUnhandledKotlinException(KRef throwable) {
+  OnUnhandledException(throwable);
 #if KONAN_REPORT_BACKTRACE_TO_IOS_CRASH_LOG
-  {
-    LockGuard<SimpleMutex> lock(terminatingMutex);
-    if (!terminating) {
-      ReportBacktraceToIosCrashLog(throwable);
-    }
-  }
+  ReportBacktraceToIosCrashLog(throwable);
 #endif
+}
+
+} // namespace
 
-  konan::abort();
+RUNTIME_NORETURN void TerminateWithUnhandledException(KRef throwable) {
+  concurrentTerminateWrapper([=]() {
+      processUnhandledKotlinException(throwable);
+    konan::abort();
+  });
 }
 
 // Some libstdc++-based targets has limited support for std::current_exception and other C++11 functions.
 // This restriction can be lifted later when toolchains will be updated.
 #if KONAN_HAS_CXX11_EXCEPTION_FUNCTIONS
 
-static void (*oldTerminateHandler)() = nullptr;
-
-static void callOldTerminateHandler() {
-#if KONAN_REPORT_BACKTRACE_TO_IOS_CRASH_LOG
-  {
-    LockGuard<SimpleMutex> lock(terminatingMutex);
-    terminating = true;
+namespace {
+class TerminateHandler {
+
+  // In fact, it's safe to call my_handler directly from outside: it will do the job and then invoke original handler,
+  // even if it has not been initialized yet. So one may want to make it public and/or not the class member
+  RUNTIME_NORETURN static void kotlinHandler() {
+    concurrentTerminateWrapper([]() {
+      if (auto currentException = std::current_exception()) {
+        try {
+          std::rethrow_exception(currentException);
+        } catch (ExceptionObjHolder& e) {
+          processUnhandledKotlinException(e.obj());
+          konan::abort();
+        } catch (...) {
+          // Not a Kotlin exception - call default handler
+          instance().queuedHandler_();
+        }
+      }
+      // Come here in case of direct terminate() call or unknown exception - go to default terminate handler.
+      instance().queuedHandler_();
+    });
   }
-#endif
 
-  RuntimeCheck(oldTerminateHandler != nullptr, "Underlying exception handler is not set.");
-  oldTerminateHandler();
-}
+  using QH = __attribute__((noreturn)) void(*)();
+  QH queuedHandler_;
 
-static void KonanTerminateHandler() {
-  auto currentException = std::current_exception();
-  if (!currentException) {
-    // No current exception.
-    callOldTerminateHandler();
-  } else {
-    try {
-      std::rethrow_exception(currentException);
-    } catch (ExceptionObjHolder& e) {
-      TerminateWithUnhandledException(e.obj());
-    } catch (...) {
-      // Not a Kotlin exception.
-      callOldTerminateHandler();
-    }
+  /// Use machinery like Meyers singleton to provide thread safety
+  TerminateHandler()
+    : queuedHandler_((QH)std::set_terminate(kotlinHandler)) {}
+
+  static TerminateHandler& instance() {
+    static TerminateHandler singleton [[clang::no_destroy]];
+    return singleton;
   }
-}
 
-static SimpleMutex konanTerminateHandlerInitializationMutex;
+  // Copy, move and assign would be safe, but not much useful, so let's delete all (rule of 5)
+  TerminateHandler(const TerminateHandler&) = delete;
+  TerminateHandler(TerminateHandler&&) = delete;
+  TerminateHandler& operator=(const TerminateHandler&) = delete;
+  TerminateHandler& operator=(TerminateHandler&&) = delete;
+  // Dtor might be in use to restore original handler. However, consequent install
+  // will not reconstruct handler anyway, so let's keep dtor deleted to avoid confusion.
+  ~TerminateHandler() = delete;
+public:
+  /// First call will do the job, all consequent will do nothing.
+  static void install() {
+    instance(); // Use side effect of warming up
+  }
+};
+} // anon namespace
 
+// Use one public function to limit access to the class declaration
 void SetKonanTerminateHandler() {
-  if (oldTerminateHandler != nullptr) return; // Already initialized.
-
-  LockGuard<SimpleMutex> lockGuard(konanTerminateHandlerInitializationMutex);
-
-  if (oldTerminateHandler != nullptr) return; // Already initialized.
-
-  oldTerminateHandler = std::set_terminate(&KonanTerminateHandler);
+  TerminateHandler::install();
 }
 
 #else // KONAN_OBJC_INTEROP
@@ -310,8 +342,6 @@ void SetKonanTerminateHandler() {
 
 #endif // KONAN_OBJC_INTEROP
 
-} // extern "C"
-
 void DisallowSourceInfo() {
   disallowSourceInfo = true;
 }

runtime/src/main/cpp/Exceptions.h

@@ -24,13 +24,17 @@ extern "C" {
 #endif
 
 // Returns current stacktrace as Array<String>.
-OBJ_GETTER0(GetCurrentStackTrace);
+OBJ_GETTER0(Kotlin_getCurrentStackTrace);
 
 OBJ_GETTER(GetStackTraceStrings, KConstRef stackTrace);
 
+OBJ_GETTER(Kotlin_setUnhandledExceptionHook, KRef hook);
+
 // Throws arbitrary exception.
 void ThrowException(KRef exception);
 
+void OnUnhandledException(KRef throwable);
+
 RUNTIME_NORETURN void TerminateWithUnhandledException(KRef exception);
 
 void SetKonanTerminateHandler();