Prevent ConcurrentModificationException in callbacks (#266)

sdk/src/androidTest/java/com/bugsnag/android/ConcurrentCallbackTest.java

@@ -0,0 +1,74 @@
+package com.bugsnag.android;
+
+import android.support.annotation.NonNull;
+import android.support.test.filters.SmallTest;
+import android.support.test.runner.AndroidJUnit4;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.util.Collection;
+
+/**
+ * Ensures that if a callback is added or removed during iteration, a
+ * {@link java.util.ConcurrentModificationException} is not thrown
+ */
+@RunWith(AndroidJUnit4.class)
+@SmallTest
+public class ConcurrentCallbackTest {
+
+    private Client client;
+
+    @Before
+    public void setUp() throws Exception {
+        client = BugsnagTestUtils.generateClient();
+    }
+
+    @Test
+    public void testClientNotifyModification() throws Exception {
+        final Collection<BeforeNotify> beforeNotifyTasks = client.config.getBeforeNotifyTasks();
+        client.beforeNotify(new BeforeNotify() {
+            @Override
+            public boolean run(Error error) {
+                beforeNotifyTasks.add(new BeforeNotifySkeleton());
+                // modify the Set, when iterating to the next callback this should not crash
+                return true;
+            }
+        });
+        client.beforeNotify(new BeforeNotifySkeleton());
+        client.notify(new RuntimeException());
+    }
+
+    @Test
+    public void testClientBreadcrumbModification() throws Exception {
+        final Collection<BeforeRecordBreadcrumb> breadcrumbTasks = client.config.getBeforeRecordBreadcrumbTasks();
+
+        client.beforeRecordBreadcrumb(new BeforeRecordBreadcrumb() {
+            @Override
+            public boolean shouldRecord(@NonNull Breadcrumb breadcrumb) {
+                breadcrumbTasks.add(new BeforeRecordBreadcrumbSkeleton());
+                // modify the Set, when iterating to the next callback this should not crash
+                return true;
+            }
+        });
+        client.beforeRecordBreadcrumb(new BeforeRecordBreadcrumbSkeleton());
+        client.leaveBreadcrumb("Whoops");
+        client.notify(new RuntimeException());
+    }
+
+    static class BeforeNotifySkeleton implements BeforeNotify {
+        @Override
+        public boolean run(Error error) {
+            return true;
+        }
+    }
+
+    static class BeforeRecordBreadcrumbSkeleton implements BeforeRecordBreadcrumb {
+        @Override
+        public boolean shouldRecord(@NonNull Breadcrumb breadcrumb) {
+            return true;
+        }
+    }
+
+}

sdk/src/main/java/com/bugsnag/android/Configuration.java

@@ -13,6 +13,7 @@
 import java.util.Map;
 import java.util.Observable;
 import java.util.Observer;
+import java.util.concurrent.ConcurrentLinkedQueue;
 
 /**
  * User-specified configuration storage object, contains information
@@ -50,9 +51,9 @@ public class Configuration extends Observable implements Observer {
 
     @NonNull
     private MetaData metaData;
-    private final Collection<BeforeNotify> beforeNotifyTasks = new LinkedHashSet<>();
+    private final Collection<BeforeNotify> beforeNotifyTasks = new ConcurrentLinkedQueue<>();
     private final Collection<BeforeRecordBreadcrumb> beforeRecordBreadcrumbTasks
-        = new LinkedHashSet<>();
+        = new ConcurrentLinkedQueue<>();
     private String codeBundleId;
     private String notifierType;
 
@@ -547,7 +548,9 @@ protected boolean shouldIgnoreClass(String className) {
      * @param beforeNotify the new before notify task
      */
     protected void beforeNotify(BeforeNotify beforeNotify) {
-        this.beforeNotifyTasks.add(beforeNotify);
+        if (!beforeNotifyTasks.contains(beforeNotify)) {
+            beforeNotifyTasks.add(beforeNotify);
+        }
     }
 
     /**
@@ -556,7 +559,9 @@ protected void beforeNotify(BeforeNotify beforeNotify) {
      * @param beforeRecordBreadcrumb the new before breadcrumb task
      */
     protected void beforeRecordBreadcrumb(BeforeRecordBreadcrumb beforeRecordBreadcrumb) {
-        this.beforeRecordBreadcrumbTasks.add(beforeRecordBreadcrumb);
+        if (!beforeRecordBreadcrumbTasks.contains(beforeRecordBreadcrumb)) {
+            beforeRecordBreadcrumbTasks.add(beforeRecordBreadcrumb);
+        }
     }
 
     /**