Introduce the notion of @BeningDataRace (Kotlin#3873)

* Mark BufferedChannelIterator.continuation as @BeningDataRace to address potential UB and OoTA on K/N
* Explain benign data-race on SelectImplementation.clauses and mark it with @BenignDataRace
* Explain benign data-race on SelectImplementation.internalResult and mark it with @BenignDataRace


Fixes Kotlin#3834
Fixes Kotlin#3843

Author: qwwdfsad

kotlinx-coroutines-core/common/src/channels/BufferedChannel.kt

@@ -1582,7 +1582,12 @@ internal open class BufferedChannel<E>(
          * When [hasNext] suspends, this field stores the corresponding
          * continuation. The [tryResumeHasNext] and [tryResumeHasNextOnClosedChannel]
          * function resume this continuation when the [hasNext] invocation should complete.
+         *
+         * This property is the subject to bening data race:
+         * It is nulled-out on both completion and cancellation paths that
+         * could happen concurrently.
          */
+        @BenignDataRace
         private var continuation: CancellableContinuationImpl<Boolean>? = null
 
         // `hasNext()` is just a special receive operation.

kotlinx-coroutines-core/common/src/internal/Concurrent.common.kt

@@ -12,3 +12,15 @@ internal expect class ReentrantLock() {
 internal expect inline fun <T> ReentrantLock.withLock(action: () -> T): T
 
 internal expect fun <E> identitySet(expectedSize: Int): MutableSet<E>
+
+/**
+ * Annotation indicating that the marked property is the subject of benign data race.
+ * LLVM does not support this notion, so on K/N platforms we alias it into `@Volatile` to prevent potential OoTA.
+ *
+ * The purpose of this annotation is not to save an extra-volatile on JVM platform, but rather to explicitly emphasize
+ * that data-race is benign.
+ */
+@OptionalExpectation
+@Target(AnnotationTarget.FIELD)
+@OptIn(ExperimentalMultiplatform::class)
+internal expect annotation class BenignDataRace()

kotlinx-coroutines-core/common/src/selects/Select.kt

@@ -372,7 +372,12 @@ internal open class SelectImplementation<R>(
 
     /**
      * List of clauses waiting on this `select` instance.
+     *
+     * This property is the subject to bening data race: concurrent cancellation might null-out this property
+     * while [trySelect] operation reads it and iterates over its content.
+     * A logical race is resolved by the consensus on [state] property.
      */
+    @BenignDataRace
     private var clauses: MutableList<ClauseData>? = ArrayList(2)
 
     /**
@@ -407,7 +412,13 @@ internal open class SelectImplementation<R>(
      * one that stores either result when the clause is successfully registered ([inRegistrationPhase] is `true`),
      * or [DisposableHandle] instance when the clause is completed during registration ([inRegistrationPhase] is `false`).
      * Yet, this optimization is omitted for code simplicity.
+     *
+     * This property is the subject to benign data race:
+     * [Cleanup][cleanup] procedure can be invoked both as part of the completion sequence
+     * and as a cancellation handler triggered by an external cancellation.
+     * In both scenarios, [NO_RESULT] is written to this property via race.
      */
+    @BenignDataRace
     private var internalResult: Any? = NO_RESULT
 
     /**
@@ -621,9 +632,8 @@ internal open class SelectImplementation<R>(
                         // try to resume the continuation.
                         this.internalResult = internalResult
                         if (cont.tryResume(onCancellation)) return TRY_SELECT_SUCCESSFUL
-                        // If the resumption failed, we need to clean
-                        // the [result] field to avoid memory leaks.
-                        this.internalResult = null
+                        // If the resumption failed, we need to clean the [result] field to avoid memory leaks.
+                        this.internalResult = NO_RESULT
                         return TRY_SELECT_CANCELLED
                     }
                 }

kotlinx-coroutines-core/native/src/internal/Concurrent.kt

@@ -29,3 +29,5 @@ internal open class SuppressSupportingThrowableImpl : Throwable() {
     }
 }
 
+@Suppress("ACTUAL_WITHOUT_EXPECT") // This suppress can be removed in 2.0: KT-59355
+internal actual typealias BenignDataRace = kotlin.concurrent.Volatile