Merge pull request #149 from bugsnag/fix-concurrent-modification-exc

Prevent potential ConcurrentModificationException when adding callback

.travis.yml

@@ -1,4 +1,5 @@
 sudo: required
+dist: trusty
 language: java
 before_cache:
 - rm -f  $HOME/.gradle/caches/modules-2/modules-2.lock

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## TBD
+
+* Prevent potential ConcurrentModificationException when adding callback
+  [#149](https://github.com/bugsnag/bugsnag-java/pull/149)
+
 ## 3.6.0 (2019-07-08)
 
 * Allow a BugsnagAppender to be created from an existing client

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

@@ -13,13 +13,13 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 @SuppressWarnings("visibilitymodifier")
@@ -44,7 +44,7 @@ public class Configuration {
     public String releaseStage;
     public boolean sendThreads = false;
 
-    Collection<Callback> callbacks = new ArrayList<Callback>();
+    Collection<Callback> callbacks = new ConcurrentLinkedQueue<Callback>();
     Serializer serializer = new Serializer();
     private final AtomicBoolean autoCaptureSessions = new AtomicBoolean(true);
     private final AtomicBoolean sendUncaughtExceptions = new AtomicBoolean(true);
@@ -81,7 +81,9 @@ boolean shouldIgnoreClass(String className) {
     }
 
     void addCallback(Callback callback) {
-        callbacks.add(callback);
+        if (!callbacks.contains(callback)) {
+            callbacks.add(callback);
+        }
     }
 
     boolean inProject(String className) {

bugsnag/src/test/java/com/bugsnag/ConcurrentCallbackTest.java

@@ -0,0 +1,44 @@
+package com.bugsnag;
+
+import com.bugsnag.callbacks.Callback;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Ensures that if a callback is added or removed during iteration, a
+ * {@link java.util.ConcurrentModificationException} is not thrown
+ */
+public class ConcurrentCallbackTest {
+
+    private Bugsnag bugsnag;
+
+    @Before
+    public void initBugsnag() {
+        bugsnag = new Bugsnag("apikey");
+    }
+
+    @After
+    public void closeBugsnag() {
+        bugsnag.close();
+    }
+
+    @Test
+    public void testClientNotifyModification() {
+        final Configuration config = bugsnag.getConfig();
+
+        config.addCallback(new Callback() {
+            @Override
+            public void beforeNotify(Report report) {
+                // modify the callback collection, when iterating to the next callback this should not crash
+                config.addCallback(new Callback() {
+                    @Override
+                    public void beforeNotify(Report report) {
+                    }
+                });
+            }
+        });
+        bugsnag.notify(new RuntimeException());
+    }
+}