Prevent ConcurrentModificationException thrown from Metadata class

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## TBD
+
+### Bug fixes
+
+* Prevent ConcurrentModificationException thrown from Metadata class
+  [#935](https://github.com/bugsnag/bugsnag-android/pull/935)
+
 ## 5.1.0 (2020-09-08)
 
 ### Enhancements

bugsnag-android-core/src/main/java/com/bugsnag/android/Metadata.kt

@@ -3,7 +3,6 @@
 package com.bugsnag.android
 
 import java.io.IOException
-import java.util.HashMap
 import java.util.HashSet
 import java.util.concurrent.ConcurrentHashMap
 
@@ -14,7 +13,7 @@ import java.util.concurrent.ConcurrentHashMap
  * Diagnostic information is presented on your Bugsnag dashboard in tabs.
  */
 internal data class Metadata @JvmOverloads constructor(
-    internal val store: MutableMap<String, Any> = ConcurrentHashMap(),
+    internal val store: ConcurrentHashMap<String, Any> = ConcurrentHashMap(),
     val jsonStreamer: ObjectJsonStreamer = ObjectJsonStreamer(),
     val redactedKeys: Set<String> = jsonStreamer.redactedKeys
 ) : JsonStream.Streamable, MetadataAware {
@@ -79,8 +78,8 @@ internal data class Metadata @JvmOverloads constructor(
         }
     }
 
-    fun toMap(): Map<String, Any> {
-        val hashMap = HashMap(store)
+    fun toMap(): ConcurrentHashMap<String, Any> {
+        val hashMap = ConcurrentHashMap(store)
 
         // deep copy each section
         store.entries.forEach {
@@ -106,7 +105,7 @@ internal data class Metadata @JvmOverloads constructor(
             return newMeta
         }
 
-        internal fun mergeMaps(data: List<Map<String, Any>>): MutableMap<String, Any> {
+        internal fun mergeMaps(data: List<Map<String, Any>>): ConcurrentHashMap<String, Any> {
             val keys = data.flatMap { it.keys }.toSet()
             val result = ConcurrentHashMap<String, Any>()
 
@@ -144,7 +143,7 @@ internal data class Metadata @JvmOverloads constructor(
     }
 
     fun copy() = this.copy(
-        store = toMap().toMutableMap(),
+        store = toMap(),
         jsonStreamer = jsonStreamer,
         redactedKeys = redactedKeys
     )

bugsnag-android-core/src/test/java/com/bugsnag/android/MetadataConcurrentModificationTest.kt

@@ -0,0 +1,21 @@
+package com.bugsnag.android
+
+import org.junit.Assert.assertNotNull
+import org.junit.Test
+import java.util.concurrent.Executors
+
+internal class MetadataConcurrentModificationTest {
+
+    @Test()
+    fun testHandlesConcurrentModification() {
+        val metadata = Metadata().copy()
+        val executor = Executors.newSingleThreadExecutor()
+
+        repeat(100) { count ->
+            assertNotNull(metadata.toMap())
+            executor.execute {
+                metadata.store["$count"] = count
+            }
+        }
+    }
+}

bugsnag-android-core/src/test/resources/event_redaction.json

@@ -3,10 +3,10 @@
     "app": {
       "password": "[REDACTED]"
     },
-    "device": {
+    "baz": {
       "password": "[REDACTED]"
     },
-    "baz": {
+    "device": {
       "password": "[REDACTED]"
     }
   },
@@ -48,4 +48,4 @@
     }
   ],
   "threads": []
-}
+}

bugsnag-android-core/src/test/resources/event_serialization_6.json

@@ -3,11 +3,11 @@
     "app": {
       "foo": 55
     },
-    "device": {
-      "bar": true
-    },
     "wham": {
       "some_key": "Avalue"
+    },
+    "device": {
+      "bar": true
     }
   },
   "severity": "warning",

bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/MetadataDeserializer.java

@@ -1,10 +1,12 @@
 package com.bugsnag.android;
 
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 class MetadataDeserializer implements MapDeserializer<Metadata> {
     @Override
     public Metadata deserialize(Map<String, Object> map) {
-        return new Metadata(map);
+        ConcurrentHashMap<String, Object> store = new ConcurrentHashMap<>(map);
+        return new Metadata(store);
     }
 }