From 066429831e571a865655670549816e3d1e9948dc Mon Sep 17 00:00:00 2001
From: Nikolay Igotti <olonho@users.noreply.github.com>
Date: Fri, 24 Aug 2018 12:37:59 +0300
Subject: [PATCH] Top level values can be only accessed from the main thread.
 (#1922)

---
 CONCURRENCY.md                                | 17 ++++++++++
 .../jetbrains/kotlin/backend/konan/ir/Ir.kt   |  7 +++-
 .../backend/konan/llvm/CodeGenerator.kt       |  4 +++
 .../kotlin/backend/konan/llvm/ContextUtils.kt |  1 +
 .../kotlin/backend/konan/llvm/IrToBitcode.kt  | 33 +++++++++++++++----
 .../backend/konan/llvm/LlvmDeclarations.kt    |  9 ++---
 .../backend/konan/lower/DelegationLowering.kt |  6 +++-
 runtime/src/main/cpp/Exceptions.h             |  1 +
 runtime/src/main/cpp/Runtime.cpp              |  7 ++++
 .../main/kotlin/kotlin/native/Annotations.kt  |  8 +++++
 .../src/main/kotlin/kotlin/native/Runtime.kt  | 17 ++++++++--
 .../kotlin/native/internal/RuntimeUtils.kt    | 10 ++++--
 .../src/main/kotlin/DecoderWorker.kt          |  1 +
 .../videoplayer/src/main/kotlin/SDLAudio.kt   |  1 +
 14 files changed, 104 insertions(+), 18 deletions(-)

diff --git a/CONCURRENCY.md b/CONCURRENCY.md
index 149da99099f..4007237dbae 100644
--- a/CONCURRENCY.md
+++ b/CONCURRENCY.md
@@ -112,3 +112,20 @@ class SharedData(rawPtr: NativePtr) : CStructVar(rawPtr) {
 ```
 So combined with the top level variable declared above, it allows seeing the same memory from different
 threads and building traditional concurrent structures with platform-specific synchronization primitives.
+
+ ## <a name="top_level"></a>Global variables and singletons
+
+  Frequently, global variables are a source of unintended concurrency issues, so _Kotlin/Native_ implements
+following mechanisms to prevent unintended sharing of state via global objects:
+
+   * global variables, unless specially marked, can be only accessed from the main thread (that is, the thread
+   _Kotlin/Native_ runtime was first initialized), if other thread access such a global, `IncorrectDereferenceException` is thrown
+   * for global variables marked with the `@kotlin.native.ThreadLocal` annotation each threads keeps thread-local copy,
+   so changes are not visible between threads
+   * for global variables marked with the `@kotlin.native.SharedImmutable` annotation value is shared, but frozen
+   before publishing, so each threads sees the same value
+   * singleton objects unless marked with `@kotlin.native.ThreadLocal` are frozen and shared, lazy values allowed,
+   unless cyclic frozen structures were attempted to be created
+   * enums are always frozen
+
+ Combined, those mechanisms allows natural race-freeze programming with code reuse across platforms in MPP projects.
\ No newline at end of file
diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/ir/Ir.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/ir/Ir.kt
index 63a79f13057..0cc6251020d 100644
--- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/ir/Ir.kt
+++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/ir/Ir.kt
@@ -370,7 +370,12 @@ internal class KonanSymbols(context: Context, val symbolTable: SymbolTable, val
     val listOfInternal = internalFunction("listOfInternal")
 
     val threadLocal =
-            context.builtIns.builtInsModule.findClassAcrossModuleDependencies(ClassId.topLevel(FqName("kotlin.native.ThreadLocal")))!!
+            context.builtIns.builtInsModule.findClassAcrossModuleDependencies(
+                    ClassId.topLevel(FqName("kotlin.native.ThreadLocal")))!!
+
+    val sharedImmutable =
+            context.builtIns.builtInsModule.findClassAcrossModuleDependencies(
+                    ClassId.topLevel(FqName("kotlin.native.SharedImmutable")))!!
 
     private fun internalFunction(name: String): IrSimpleFunctionSymbol =
             symbolTable.referenceSimpleFunction(context.getInternalFunctions(name).single())
diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt
index 5ccccd10de6..0e5d75f9937 100644
--- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt
+++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt
@@ -305,6 +305,10 @@ internal class FunctionGenerationContext(val function: LLVMValueRef,
             call(context.llvm.freezeSubgraph, listOf(value),  Lifetime.IRRELEVANT, exceptionHandler)
     }
 
+    fun checkMainThread(exceptionHandler: ExceptionHandler) {
+        call(context.llvm.checkMainThread, emptyList(), Lifetime.IRRELEVANT, exceptionHandler)
+    }
+
     private fun updateReturnRef(value: LLVMValueRef, address: LLVMValueRef) {
         call(context.llvm.updateReturnRefFunction, listOf(address, value))
     }
diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/ContextUtils.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/ContextUtils.kt
index 1e2bdd9eb69..4ad38635d25 100644
--- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/ContextUtils.kt
+++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/ContextUtils.kt
@@ -395,6 +395,7 @@ internal class Llvm(val context: Context, val llvmModule: LLVMModuleRef) {
     val initRuntimeIfNeeded = importRtFunction("Kotlin_initRuntimeIfNeeded")
     val mutationCheck = importRtFunction("MutationCheck")
     val freezeSubgraph = importRtFunction("FreezeSubgraph")
+    val checkMainThread = importRtFunction("CheckIsMainThread")
 
     val createKotlinObjCClass by lazy { importRtFunction("CreateKotlinObjCClass") }
     val getObjCKotlinTypeInfo by lazy { importRtFunction("GetObjCKotlinTypeInfo") }
diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/IrToBitcode.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/IrToBitcode.kt
index 05c163376c1..00cd8590077 100644
--- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/IrToBitcode.kt
+++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/IrToBitcode.kt
@@ -26,6 +26,7 @@ import org.jetbrains.kotlin.backend.konan.ir.*
 import org.jetbrains.kotlin.backend.konan.irasdescriptors.*
 import org.jetbrains.kotlin.backend.konan.objcexport.ObjCExport
 import org.jetbrains.kotlin.backend.konan.optimizations.*
+import org.jetbrains.kotlin.builtins.KotlinBuiltIns
 import org.jetbrains.kotlin.builtins.UnsignedType
 import org.jetbrains.kotlin.descriptors.ClassKind
 import org.jetbrains.kotlin.descriptors.Modality
@@ -49,12 +50,27 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.classId
 import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameUnsafe
 
 private val threadLocalAnnotationFqName = FqName("kotlin.native.ThreadLocal")
+private val sharedAnnotationFqName = FqName("kotlin.native.SharedImmutable")
 
+// TODO: maybe unannotated singleton objects shall be accessed from main thread only as well?
 val IrClass.objectIsShared get() =
     !descriptor.annotations.hasAnnotation(threadLocalAnnotationFqName)
 
+val IrField.isThreadLocal get() =
+    descriptor.annotations.hasAnnotation(threadLocalAnnotationFqName)
+
 val IrField.isShared get() =
-    !descriptor.annotations.hasAnnotation(threadLocalAnnotationFqName) && !descriptor.isVar
+    descriptor.annotations.hasAnnotation(sharedAnnotationFqName)
+
+val IrField.isMainOnly get() =
+    !descriptor.annotations.hasAnnotation(threadLocalAnnotationFqName) &&
+    !descriptor.annotations.hasAnnotation(sharedAnnotationFqName) &&
+    !descriptor.isDelegated
+
+val IrField.isMainOnlyNonPrimitive get() = when  {
+        KotlinBuiltIns.isPrimitiveType(descriptor.type) -> false
+        else -> isMainOnly
+    }
 
 internal fun emitLLVM(context: Context, phaser: PhaseManager) {
     val irModule = context.irModule!!
@@ -404,7 +420,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
                     context.llvm.fileInitializers
                             .forEach {
                                 if (it.initializer?.expression !is IrConst<*>?) {
-                                    if (it.isShared) {
+                                    if (!it.isThreadLocal) {
                                         val initialization = evaluateExpression(it.initializer!!.expression)
                                         val address = context.llvmDeclarations.forStaticField(it).storage
                                         freeze(initialization, currentCodeContext.exceptionHandler)
@@ -419,7 +435,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
                     context.llvm.fileInitializers
                             .forEach {
                                 if (it.initializer?.expression !is IrConst<*>?) {
-                                   if (!it.isShared) {
+                                   if (it.isThreadLocal) {
                                        val initialization = evaluateExpression(it.initializer!!.expression)
                                        val address = context.llvmDeclarations.forStaticField(it).storage
                                        storeAny(initialization, address)
@@ -432,7 +448,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
                 appendingTo(bbLocalDeinit) {
                     context.llvm.fileInitializers.forEach {
                         // Only if a subject for memory management.
-                        if (it.type.binaryTypeIsReference() && !it.isShared) {
+                        if (it.type.binaryTypeIsReference() && it.isThreadLocal) {
                             val address = context.llvmDeclarations.forStaticField(it).storage
                             storeAny(codegen.kNullObjHeaderPtr, address)
                         }
@@ -445,7 +461,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
                     context.llvm.fileInitializers
                             // Only if a subject for memory management.
                             .forEach {
-                                if (it.type.binaryTypeIsReference() && it.isShared) {
+                                if (it.type.binaryTypeIsReference() && !it.isThreadLocal) {
                                     val address = context.llvmDeclarations.forStaticField(it).storage
                                     storeAny(codegen.kNullObjHeaderPtr, address)
                                 }
@@ -1428,7 +1444,10 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
             return functionGenerationContext.loadSlot(
                     fieldPtrOfClass(thisPtr, value.symbol.owner), value.descriptor.isVar())
         } else {
-            assert (value.receiver == null)
+            assert(value.receiver == null)
+            if (context.config.threadsAreAllowed && value.symbol.owner.isMainOnlyNonPrimitive) {
+                functionGenerationContext.checkMainThread(currentCodeContext.exceptionHandler)
+            }
             val ptr = context.llvmDeclarations.forStaticField(value.symbol.owner).storage
             return functionGenerationContext.loadSlot(ptr, value.descriptor.isVar())
         }
@@ -1459,6 +1478,8 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
         } else {
             assert(value.receiver == null)
             val globalValue = context.llvmDeclarations.forStaticField(value.symbol.owner).storage
+            if (context.config.threadsAreAllowed && value.symbol.owner.isMainOnlyNonPrimitive)
+                functionGenerationContext.checkMainThread(currentCodeContext.exceptionHandler)
             if (value.symbol.owner.isShared)
                 functionGenerationContext.freeze(valueToAssign, currentCodeContext.exceptionHandler)
             functionGenerationContext.storeAny(valueToAssign, globalValue)
diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/LlvmDeclarations.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/LlvmDeclarations.kt
index e0943637999..75503de58a1 100644
--- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/LlvmDeclarations.kt
+++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/LlvmDeclarations.kt
@@ -376,21 +376,18 @@ private class DeclarationsGeneratorVisitor(override val context: Context) :
 
         val containingClass = descriptor.containingClass
         if (containingClass != null) {
-            val classDeclarations = this.classes[containingClass] ?: error(containingClass.descriptor.toString())
-
+            val classDeclarations = this.classes[containingClass] ?:
+                error(containingClass.descriptor.toString())
             val allFields = classDeclarations.fields
-
             this.fields[descriptor] = FieldLlvmDeclarations(
                     allFields.indexOf(descriptor),
                     classDeclarations.bodyType
             )
         } else {
-
             // Fields are module-private, so we use internal name:
             val name = "kvar:" + qualifyInternalName(descriptor)
-
             val storage = addGlobal(
-                    name, getLLVMType(descriptor.type), isExported = false, threadLocal = !declaration.isShared)
+                    name, getLLVMType(descriptor.type), isExported = false, threadLocal = declaration.isThreadLocal)
 
             this.staticFields[descriptor] = StaticFieldLlvmDeclarations(storage)
         }
diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/DelegationLowering.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/DelegationLowering.kt
index 08d0a371303..6f7c5fbf753 100644
--- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/DelegationLowering.kt
+++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/DelegationLowering.kt
@@ -23,7 +23,9 @@ import org.jetbrains.kotlin.backend.common.lower.irBlock
 import org.jetbrains.kotlin.backend.konan.descriptors.synthesizedName
 import org.jetbrains.kotlin.backend.konan.KonanBackendContext
 import org.jetbrains.kotlin.descriptors.*
+import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptorImpl
 import org.jetbrains.kotlin.descriptors.annotations.Annotations
+import org.jetbrains.kotlin.descriptors.annotations.AnnotationsImpl
 import org.jetbrains.kotlin.descriptors.impl.PropertyDescriptorImpl
 import org.jetbrains.kotlin.ir.IrStatement
 import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET
@@ -317,7 +319,9 @@ internal class PropertyDelegationLowering(val context: KonanBackendContext) : Fi
             IrDeclarationOriginImpl("KPROPERTIES_FOR_DELEGATION")
 
     private fun createKPropertiesFieldDescriptor(containingDeclaration: DeclarationDescriptor, fieldType: KotlinType): PropertyDescriptorImpl {
-        return PropertyDescriptorImpl.create(containingDeclaration, Annotations.EMPTY, Modality.FINAL, Visibilities.PRIVATE,
+        return PropertyDescriptorImpl.create(containingDeclaration,
+                AnnotationsImpl(listOf(AnnotationDescriptorImpl(context.ir.symbols.sharedImmutable.defaultType,
+                        emptyMap(), SourceElement.NO_SOURCE))), Modality.FINAL, Visibilities.PRIVATE,
                 false, "KPROPERTIES".synthesizedName, CallableMemberDescriptor.Kind.SYNTHESIZED, SourceElement.NO_SOURCE,
                 false, false, false, false, false, false).apply {
 
diff --git a/runtime/src/main/cpp/Exceptions.h b/runtime/src/main/cpp/Exceptions.h
index 0ee9b6bee15..7546da6efea 100644
--- a/runtime/src/main/cpp/Exceptions.h
+++ b/runtime/src/main/cpp/Exceptions.h
@@ -52,6 +52,7 @@ void RUNTIME_NORETURN ThrowNotImplementedError();
 void RUNTIME_NORETURN ThrowIllegalCharacterConversionException();
 void RUNTIME_NORETURN ThrowIllegalArgumentException();
 void RUNTIME_NORETURN ThrowInvalidMutabilityException(KConstRef where);
+void RUNTIME_NORETURN ThrowIncorrectDereferenceException();
 // Prints out mesage of Throwable.
 void PrintThrowable(KRef);
 
diff --git a/runtime/src/main/cpp/Runtime.cpp b/runtime/src/main/cpp/Runtime.cpp
index e8d3d8f167d..504ffa8c36b 100644
--- a/runtime/src/main/cpp/Runtime.cpp
+++ b/runtime/src/main/cpp/Runtime.cpp
@@ -71,6 +71,7 @@ void InitOrDeinitGlobalVariables(int initialize) {
 }
 
 THREAD_LOCAL_VARIABLE RuntimeState* runtimeState = nullptr;
+THREAD_LOCAL_VARIABLE int isMainThread = 0;
 
 int aliveRuntimesCount = 0;
 
@@ -82,6 +83,7 @@ RuntimeState* initRuntime() {
   bool firstRuntime = atomicAdd(&aliveRuntimesCount, 1) == 1;
   // Keep global variables in state as well.
   if (firstRuntime) {
+    isMainThread = 1;
     konan::consoleInit();
     InitOrDeinitGlobalVariables(INIT_GLOBALS);
   }
@@ -159,4 +161,9 @@ RuntimeState* RUNTIME_USED Kotlin_getRuntime() {
   return ::runtimeState;
 }
 
+void CheckIsMainThread() {
+  if (!isMainThread)
+    ThrowIncorrectDereferenceException();
+}
+
 }  // extern "C"
diff --git a/runtime/src/main/kotlin/kotlin/native/Annotations.kt b/runtime/src/main/kotlin/kotlin/native/Annotations.kt
index 5f61f6ec90c..00a78dfb81c 100644
--- a/runtime/src/main/kotlin/kotlin/native/Annotations.kt
+++ b/runtime/src/main/kotlin/kotlin/native/Annotations.kt
@@ -61,6 +61,14 @@ public annotation class Throws(vararg val exceptionClasses: KClass<out Throwable
 @Retention(AnnotationRetention.BINARY)
 public annotation class ThreadLocal
 
+/**
+ * Top level variable is immutable and so could be shared.
+ * PLEASE NOTE THAT THIS ANNOTATION MAY GO AWAY IN UPCOMING RELEASES.
+ */
+@Target(AnnotationTarget.FIELD)
+@Retention(AnnotationRetention.BINARY)
+public annotation class SharedImmutable
+
 /**
  * Makes top level function available from C/C++ code with the given name.
  * `fullName` controls the name of top level function, `shortName` controls the short name.
diff --git a/runtime/src/main/kotlin/kotlin/native/Runtime.kt b/runtime/src/main/kotlin/kotlin/native/Runtime.kt
index afd8af70fb9..8628c024425 100644
--- a/runtime/src/main/kotlin/kotlin/native/Runtime.kt
+++ b/runtime/src/main/kotlin/kotlin/native/Runtime.kt
@@ -15,10 +15,23 @@
  */
 package kotlin.native
 
-// Initializes Kotlin runtime for the current thread, if not inited already.
+/**
+ * Initializes Kotlin runtime for the current thread, if not inited already.
+ */
 @SymbolName("Kotlin_initRuntimeIfNeeded")
 external public fun initRuntimeIfNeeded(): Unit
 
-// Deiitializes Kotlin runtime for the current thread, if was inited.
+/**
+ * Deiitializes Kotlin runtime for the current thread, if was inited.
+ */
 @SymbolName("Kotlin_deinitRuntimeIfNeeded")
 external public fun deinitRuntimeIfNeeded(): Unit
+
+/**
+ * Exception thrown when top level variable is accessed from incorrect execution context.
+ */
+public class IncorrectDereferenceException : RuntimeException {
+    constructor() : super()
+
+    constructor(message: String) : super(message)
+}
\ No newline at end of file
diff --git a/runtime/src/main/kotlin/kotlin/native/internal/RuntimeUtils.kt b/runtime/src/main/kotlin/kotlin/native/internal/RuntimeUtils.kt
index 0b78c93ccfa..2b9f9aecaa0 100644
--- a/runtime/src/main/kotlin/kotlin/native/internal/RuntimeUtils.kt
+++ b/runtime/src/main/kotlin/kotlin/native/internal/RuntimeUtils.kt
@@ -82,12 +82,18 @@ internal fun ThrowIllegalCharacterConversionException(): Nothing {
 }
 
 @ExportForCppRuntime
-fun PrintThrowable(throwable: Throwable) {
+internal fun ThrowIncorrectDereferenceException() {
+    throw IncorrectDereferenceException(
+            "Trying to access top level value not marked as @ThreadLocal or @ImmutableShared from non-main thread")
+}
+
+@ExportForCppRuntime
+internal fun PrintThrowable(throwable: Throwable) {
     println(throwable)
 }
 
 @ExportForCppRuntime
-fun ReportUnhandledException(e: Throwable) {
+internal fun ReportUnhandledException(e: Throwable) {
     print("Uncaught Kotlin exception: ")
     e.printStackTrace()
 }
diff --git a/samples/videoplayer/src/main/kotlin/DecoderWorker.kt b/samples/videoplayer/src/main/kotlin/DecoderWorker.kt
index 2000444d369..7f70b938b09 100644
--- a/samples/videoplayer/src/main/kotlin/DecoderWorker.kt
+++ b/samples/videoplayer/src/main/kotlin/DecoderWorker.kt
@@ -20,6 +20,7 @@ import kotlinx.cinterop.*
 import platform.posix.memcpy
 
 // This global variable only set to != null value in the decoding worker.
+@ThreadLocal
 private var decoder: Decoder? = null
 
 data class VideoInfo(val size: Dimensions, val fps: Double)
diff --git a/samples/videoplayer/src/main/kotlin/SDLAudio.kt b/samples/videoplayer/src/main/kotlin/SDLAudio.kt
index 3ed95eaeae1..4cca17fb045 100644
--- a/samples/videoplayer/src/main/kotlin/SDLAudio.kt
+++ b/samples/videoplayer/src/main/kotlin/SDLAudio.kt
@@ -76,6 +76,7 @@ class SDLAudio(val player: VideoPlayer) : DisposableContainer() {
 }
 
 // Only set in the audio thread
+@ThreadLocal
 private var decoder: DecoderWorker? = null
 
 private fun audioCallback(userdata: COpaquePointer?, buffer: CPointer<Uint8Var>?, length: Int) {