Merge pull request #403 from bugsnag/init-safety

Prevent Bugsnag.init from instantiating more than one client
bugsnag · Jan 4, 2019 · db5646c · db5646c
2 parents fdc3892 + d448a25
commit db5646c
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@
 * Prevent errors from leaving a self-referencing breadcrumb
  [#391](https://github.com/bugsnag/bugsnag-android/pull/391)
 
+* Prevent Bugsnag.init from instantiating more than one client
+ [#403](https://github.com/bugsnag/bugsnag-android/pull/403)
+
+
 ## 4.9.3 (2018-11-29)
 
 ### Bug fixes

diff --git a/features/bugsnag_init.feature b/features/bugsnag_init.feature
@@ -0,0 +1,7 @@
+Feature: Reporting app version
+
+Scenario: Test handled Android Exception
+    When I run "BugsnagInitScenario"
+    Then I should receive a request
+    And the request is a valid for the error reporting API
+    And the event "metaData.client.count" equals 1
diff --git a/features/fixtures/mazerunner/src/main/AndroidManifest.xml b/features/fixtures/mazerunner/src/main/AndroidManifest.xml
@@ -12,6 +12,9 @@
             </intent-filter>
         </activity>
         <activity android:name=".SecondActivity"/>
+        <meta-data
+            android:name="com.bugsnag.android.API_KEY"
+            android:value="YOUR-API-KEY-HERE" />
     </application>
 
 </manifest>
diff --git a/.../mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/BugsnagInitScenario.kt b/.../mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/BugsnagInitScenario.kt
@@ -0,0 +1,39 @@
+package com.bugsnag.android.mazerunner.scenarios
+
+import android.content.Context
+import com.bugsnag.android.Bugsnag
+import com.bugsnag.android.Client
+import com.bugsnag.android.Configuration
+import java.lang.RuntimeException
+import java.util.concurrent.Callable
+import java.util.concurrent.Executors
+
+internal class BugsnagInitScenario(
+    config: Configuration,
+    context: Context
+) : Scenario(config, context) {
+
+    init {
+        config.setAutoCaptureSessions(false)
+    }
+
+    override fun run() {
+        val threadPool = Executors.newFixedThreadPool(8)
+        val callables = mutableListOf<Callable<Client?>>()
+
+        IntRange(1, 25).forEach {
+            callables.add(Callable { Bugsnag.init(context) })
+            callables.add(Callable { Bugsnag.init(context, config.apiKey) })
+            callables.add(Callable { Bugsnag.init(context, config.apiKey, false) })
+            callables.add(Callable { Bugsnag.init(context, Configuration(config.apiKey)) })
+        }
+
+        val futures = threadPool.invokeAll(callables)
+        val uniqueClients = futures.map { it.get() }.distinct()
+
+        val bugsnag = uniqueClients.first()!!
+        bugsnag.addToTab("client", "count", uniqueClients.size)
+        bugsnag.notify(RuntimeException())
+    }
+
+}
diff --git a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java
@@ -19,7 +19,8 @@
 @SuppressWarnings("checkstyle:JavadocTagContinuationIndentation")
 public final class Bugsnag {
 
-    @Nullable
+    private static final Object lock = new Object();
+
     @SuppressLint("StaticFieldLeak")
     static Client client;
 
@@ -33,9 +34,7 @@ private Bugsnag() {
      */
     @NonNull
     public static Client init(@NonNull Context androidContext) {
-        client = new Client(androidContext);
-        NativeInterface.configureClientObservers(client);
-        return client;
+        return init(androidContext, null, true);
     }
 
     /**
@@ -46,9 +45,7 @@ public static Client init(@NonNull Context androidContext) {
      */
     @NonNull
     public static Client init(@NonNull Context androidContext, @Nullable String apiKey) {
-        client = new Client(androidContext, apiKey);
-        NativeInterface.configureClientObservers(client);
-        return client;
+        return init(androidContext, apiKey, true);
     }
 
     /**
@@ -62,9 +59,9 @@ public static Client init(@NonNull Context androidContext, @Nullable String apiK
     public static Client init(@NonNull Context androidContext,
                               @Nullable String apiKey,
                               boolean enableExceptionHandler) {
-        client = new Client(androidContext, apiKey, enableExceptionHandler);
-        NativeInterface.configureClientObservers(client);
-        return client;
+        Configuration config
+            = ConfigFactory.createNewConfiguration(androidContext, apiKey, enableExceptionHandler);
+        return init(androidContext, config);
     }
 
     /**
@@ -75,11 +72,23 @@ public static Client init(@NonNull Context androidContext,
      */
     @NonNull
     public static Client init(@NonNull Context androidContext, @NonNull Configuration config) {
-        client = new Client(androidContext, config);
-        NativeInterface.configureClientObservers(client);
+        synchronized (lock) {
+            if (client == null) {
+                client = new Client(androidContext, config);
+                NativeInterface.configureClientObservers(client);
+            } else {
+                logClientInitWarning();
+            }
+        }
         return client;
     }
 
+    private static void logClientInitWarning() {
+        Logger.warn("It appears that Bugsnag.init() was called more than once. Subsequent "
+            + "calls have no effect, but may indicate that Bugsnag is not integrated in an"
+            + " Application subclass, which can lead to undesired behaviour.");
+    }
+
     /**
      * Set the application version sent to Bugsnag. By default we'll pull this
      * from your AndroidManifest.xml