Merge pull request #2012 from DataDog/nogorodnikov/rum-4329/fix-concu…

…rrent-modification-exception-during-features-iteration

RUM-4329: Fix `ConcurrentModificationException` during features iteration

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/DatadogCore.kt

@@ -43,6 +43,7 @@ import com.datadog.android.privacy.TrackingConsent
 import com.google.gson.JsonObject
 import java.io.File
 import java.util.Locale
+import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.ExecutorService
 import java.util.concurrent.ScheduledExecutorService
 import java.util.concurrent.TimeUnit
@@ -71,7 +72,7 @@ internal class DatadogCore(
 
     private lateinit var shutdownHook: Thread
 
-    internal val features: MutableMap<String, SdkFeature> = mutableMapOf()
+    internal val features: MutableMap<String, SdkFeature> = ConcurrentHashMap()
 
     internal val context: Context = context.applicationContext
 

dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/DatadogCoreTest.kt

@@ -50,8 +50,10 @@ import fr.xgouchet.elmyr.junit5.ForgeExtension
 import org.assertj.core.api.Assertions.assertThat
 import org.assertj.core.data.Offset
 import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.AssertionFailureBuilder
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertDoesNotThrow
 import org.junit.jupiter.api.extension.ExtendWith
 import org.junit.jupiter.api.extension.Extensions
 import org.junit.jupiter.params.ParameterizedTest
@@ -72,7 +74,9 @@ import org.mockito.kotlin.verifyNoInteractions
 import org.mockito.kotlin.verifyNoMoreInteractions
 import org.mockito.kotlin.whenever
 import org.mockito.quality.Strictness
+import java.util.Collections
 import java.util.Locale
+import java.util.concurrent.CountDownLatch
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.concurrent.atomic.AtomicReference
@@ -803,6 +807,70 @@ internal class DatadogCoreTest {
         }
     }
 
+    @Test
+    fun `M allow concurrent access to features W access features when modifying their collection`(
+        @StringForgery fakeFeature: String,
+        forge: Forge
+    ) {
+        // Given
+        testedCore.features += fakeFeature to mock()
+
+        // When
+        val errorCollector = Collections.synchronizedList(mutableListOf<Throwable>())
+        val latch = CountDownLatch(2)
+        val threadA = Thread(
+            ErrorRecordingRunnable(errorCollector) {
+                latch.countDown()
+                latch.await()
+                assertDoesNotThrow {
+                    repeat(100) {
+                        testedCore.features += forge.anAlphabeticalString() to mock()
+                    }
+                }
+            }
+        ).apply { start() }
+        val threadB = Thread(
+            ErrorRecordingRunnable(errorCollector) {
+                latch.countDown()
+                latch.await()
+                assertDoesNotThrow {
+                    repeat(100) {
+                        testedCore.updateFeatureContext(fakeFeature) {
+                            // no-op
+                        }
+                    }
+                }
+            }
+        ).apply { start() }
+
+        listOf(threadA, threadB).forEach { it.join() }
+
+        // Then
+        if (errorCollector.isNotEmpty()) {
+            AssertionFailureBuilder
+                .assertionFailure()
+                .message(
+                    "Expected no errors to be thrown during the concurrent" +
+                        " access to features, but there were errors recorded. See first seen error below."
+                )
+                .cause(errorCollector.first())
+                .buildAndThrow()
+        }
+    }
+
+    class ErrorRecordingRunnable(
+        private val collector: MutableList<Throwable>,
+        private val delegate: Runnable
+    ) : Runnable {
+        override fun run() {
+            try {
+                delegate.run()
+            } catch (t: Throwable) {
+                collector += t
+            }
+        }
+    }
+
     companion object {
 
         val msToNs = TimeUnit.MILLISECONDS.toNanos(1)