diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ConcurrentCallbackTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ConcurrentCallbackTest.java new file mode 100644 index 0000000000..7183865b6a --- /dev/null +++ b/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 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 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; + } + } + +} diff --git a/sdk/src/main/java/com/bugsnag/android/Configuration.java b/sdk/src/main/java/com/bugsnag/android/Configuration.java index 2c9a9780d5..eff55eafce 100644 --- a/sdk/src/main/java/com/bugsnag/android/Configuration.java +++ b/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 beforeNotifyTasks = new LinkedHashSet<>(); + private final Collection beforeNotifyTasks = new ConcurrentLinkedQueue<>(); private final Collection 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); + } } /**