Address review comments

JetBrains · Nov 12, 2020 · 037fe02 · 037fe02
1 parent 0c99715
commit 037fe02
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 31 deletions.
diff --git a/...ive/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/IrToBitcode.kt b/...ive/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/IrToBitcode.kt
@@ -363,7 +363,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
 
             context.coverage.writeRegionInfo()
             appendDebugSelector()
-            appendCompilerGlobals()
+            overrideRuntimeGlobals()
             appendLlvmUsed("llvm.used", context.llvm.usedFunctions + context.llvm.usedGlobals)
             appendLlvmUsed("llvm.compiler.used", context.llvm.compilerUsedGlobals)
             if (context.isNativeLibrary) {
@@ -2389,7 +2389,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
         llvmDebugSelector.setLinkage(LLVMLinkage.LLVMExternalLinkage)
     }
 
-    private fun appendGlobal(name: String, value: ConstValue) {
+    private fun overrideRuntimeGlobal(name: String, value: ConstValue) {
         // TODO: A similar mechanism is used in `ObjCExportCodeGenerator`. Consider merging them.
         if (context.llvmModuleSpecification.importsKotlinDeclarationsFromOtherSharedLibraries()) {
             // When some dynamic caches are used, we consider that stdlib is in the dynamic cache as well.
@@ -2415,10 +2415,11 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
         }
     }
 
-    private fun appendCompilerGlobals() {
+    private fun overrideRuntimeGlobals() {
         if (!context.config.produce.isFinalBinary)
             return
-        appendGlobal("Kotlin_destroyRuntimeMode", Int32(context.config.destroyRuntimeMode.value))
+
+        overrideRuntimeGlobal("Kotlin_destroyRuntimeMode", Int32(context.config.destroyRuntimeMode.value))
     }
 
     //-------------------------------------------------------------------------//

diff --git a/backend.native/tests/interop/migrating_main_thread/lib.kt b/backend.native/tests/interop/migrating_main_thread/lib.kt
@@ -13,4 +13,10 @@ fun writeToA(i: Int) {
     globalA.i = i
 }
 
-fun readFromA() = globalA.i
+fun tryReadFromA(default: Int): Int {
+    return try {
+        globalA.i
+    } catch (e: IncorrectDereferenceException) {
+        default
+    }
+}
diff --git a/backend.native/tests/interop/migrating_main_thread/main.cpp b/backend.native/tests/interop/migrating_main_thread/main.cpp
@@ -9,25 +9,26 @@
 #include <thread>
 
 constexpr int kInitialValue = 0;
-constexpr int kNewValue = 0;
-
-#if defined(IS_LEGACY)
-// Globals were reinitialized.
-constexpr int kResultValue = kInitialValue;
-#else
-// Globals were kept.
-constexpr int kResultValue = kNewValue;
-#endif
+constexpr int kNewValue = 1;
+constexpr int kErrorValue = 2;
 
 int main() {
     std::thread main1([]() {
-        assert(testlib_symbols()->kotlin.root.readFromA() == kInitialValue);
+        assert(testlib_symbols()->kotlin.root.tryReadFromA(kErrorValue) == kInitialValue);
         testlib_symbols()->kotlin.root.writeToA(kNewValue);
-        assert(testlib_symbols()->kotlin.root.readFromA() == kNewValue);
+        assert(testlib_symbols()->kotlin.root.tryReadFromA(kErrorValue) == kNewValue);
     });
     main1.join();
 
-    std::thread main2([]() { assert(testlib_symbols()->kotlin.root.readFromA() == kResultValue); });
+    std::thread main2([]() {
+#if defined(IS_LEGACY)
+        // Globals were reinitialized.
+        assert(testlib_symbols()->kotlin.root.tryReadFromA(kErrorValue) == kInitialValue);
+#else
+        // Globals are not accessible.
+        assert(testlib_symbols()->kotlin.root.tryReadFromA(kErrorValue) == kErrorValue);
+#endif
+    });
     main2.join();
 
     return 0;

diff --git a/runtime/src/legacymm/cpp/Memory.cpp b/runtime/src/legacymm/cpp/Memory.cpp
@@ -2019,31 +2019,20 @@ MemoryState* initMemory(bool firstRuntime) {
 #endif  // USE_CYCLIC_GC
     memoryState->isMainThread = true;
   }
-  if (!firstRuntime && firstMemoryState) {
-    memoryState->isMainThread = true;
-    // This thread is now the main thread. And there was a previous main thread, because this is not the first runtime.
-    // Make sure this thread sees all the updates to Kotlin globals from the previous main thread.
-    synchronize();
-  }
   return memoryState;
 }
 
 void deinitMemory(MemoryState* memoryState, bool destroyRuntime) {
   static int pendingDeinit = 0;
   atomicAdd(&pendingDeinit, 1);
 #if USE_GC
+  bool lastMemoryState = atomicAdd(&aliveMemoryStatesCount, -1) == 0;
   switch (Kotlin_getDestroyRuntimeMode()) {
     case DESTROY_RUNTIME_LEGACY:
-      destroyRuntime = atomicAdd(&aliveMemoryStatesCount, -1) == 0;
+      destroyRuntime = lastMemoryState;
       break;
     case DESTROY_RUNTIME_ON_SHUTDOWN:
-      if (!destroyRuntime && memoryState->isMainThread) {
-        // If we are not destroying the runtime but we were the main thread, publish all changes to Kotlin globals.
-        // This `synchronize` should be before `aliveMemoryStatesCount` decrement to synchronize with
-        // `initMemory` which does this in the reverse order.
-        synchronize();
-      }
-      atomicAdd(&aliveMemoryStatesCount, -1);
+      // Nothing to do
       break;
   }
   bool checkLeaks = Kotlin_memoryLeakCheckerEnabled() && destroyRuntime;

diff --git a/runtime/src/main/cpp/Runtime.cpp b/runtime/src/main/cpp/Runtime.cpp
@@ -31,6 +31,7 @@ struct InitNode {
   InitNode* next;
 };
 
+// This global is overriden by the compiler.
 RUNTIME_WEAK DestroyRuntimeMode Kotlin_destroyRuntimeMode = DESTROY_RUNTIME_ON_SHUTDOWN;
 
 DestroyRuntimeMode Kotlin_getDestroyRuntimeMode() {