From 1d1e3424b0c1989f3b84816e2a94677f3fed2f39 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Wed, 19 Dec 2018 16:54:18 +0000 Subject: [PATCH 1/6] test: add mazerunner scenario which verifies bugsnag.init only instantiates one client --- features/bugsnag_init.feature | 7 ++++ .../mazerunner/src/main/AndroidManifest.xml | 3 ++ .../scenarios/BugsnagInitScenario.kt | 39 +++++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 features/bugsnag_init.feature create mode 100644 features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/BugsnagInitScenario.kt diff --git a/features/bugsnag_init.feature b/features/bugsnag_init.feature new file mode 100644 index 0000000000..3c53423a3f --- /dev/null +++ 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 index ef3c038299..62bfe8b769 100644 --- a/features/fixtures/mazerunner/src/main/AndroidManifest.xml +++ b/features/fixtures/mazerunner/src/main/AndroidManifest.xml @@ -12,6 +12,9 @@ + diff --git a/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/BugsnagInitScenario.kt b/features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/BugsnagInitScenario.kt new file mode 100644 index 0000000000..32c11f9746 --- /dev/null +++ b/features/fixtures/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>() + + 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()) + } + +} From c353cb2584dbd37a43e74ed0db053abfcb3d76e4 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Wed, 19 Dec 2018 17:04:12 +0000 Subject: [PATCH 2/6] fix: only allow Bugsnag.init to be called once --- .../java/com/bugsnag/android/Bugsnag.java | 51 +++++++++++++++---- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java index b656f37f43..cd508eb730 100644 --- a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java +++ b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java @@ -19,9 +19,10 @@ @SuppressWarnings("checkstyle:JavadocTagContinuationIndentation") public final class Bugsnag { - @Nullable + private static final Object lock = new Object(); + @SuppressLint("StaticFieldLeak") - static Client client; + static volatile Client client; private Bugsnag() { } @@ -33,8 +34,14 @@ private Bugsnag() { */ @NonNull public static Client init(@NonNull Context androidContext) { - client = new Client(androidContext); - NativeInterface.configureClientObservers(client); + synchronized (lock) { + if (client == null) { + client = new Client(androidContext); + NativeInterface.configureClientObservers(client); + } else { + logClientInitWarning(); + } + } return client; } @@ -46,8 +53,14 @@ 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); + synchronized (lock) { + if (client == null) { + client = new Client(androidContext, apiKey); + NativeInterface.configureClientObservers(client); + } else { + logClientInitWarning(); + } + } return client; } @@ -62,8 +75,14 @@ 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); + synchronized (lock) { + if (client == null) { + client = new Client(androidContext, apiKey, enableExceptionHandler); + NativeInterface.configureClientObservers(client); + } else { + logClientInitWarning(); + } + } return client; } @@ -75,11 +94,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 From 5b84d4408f819e7ba5240de4aee64f619fde9279 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 20 Dec 2018 09:09:55 +0000 Subject: [PATCH 3/6] perf: remove unnecessary volatile modifier on client --- sdk/src/main/java/com/bugsnag/android/Bugsnag.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java index cd508eb730..d9bc56b7c4 100644 --- a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java +++ b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java @@ -22,7 +22,7 @@ public final class Bugsnag { private static final Object lock = new Object(); @SuppressLint("StaticFieldLeak") - static volatile Client client; + static Client client; private Bugsnag() { } From 6514efbc34cf980ac0e7a7e4b71be9c7843faa00 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 20 Dec 2018 09:26:00 +0000 Subject: [PATCH 4/6] style: fix style --- sdk/src/main/java/com/bugsnag/android/Bugsnag.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java index d9bc56b7c4..4b6e3d77f2 100644 --- a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java +++ b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java @@ -106,9 +106,9 @@ public static Client init(@NonNull Context androidContext, @NonNull Configuratio } 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."); + 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."); } /** From dd455830e44740edfe77845cf5ddf6f8589ffb61 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 20 Dec 2018 09:36:17 +0000 Subject: [PATCH 5/6] docs: add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bda7dd06f9..cc31ee8cf3 100644 --- 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 From d448a2546140578687bd6b124f7fa92e7b13ef63 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Thu, 3 Jan 2019 11:10:41 +0000 Subject: [PATCH 6/6] refactor: reduce duplication in bugsnag.init calls --- .../java/com/bugsnag/android/Bugsnag.java | 32 +++---------------- 1 file changed, 5 insertions(+), 27 deletions(-) diff --git a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java index 4b6e3d77f2..34063c18b9 100644 --- a/sdk/src/main/java/com/bugsnag/android/Bugsnag.java +++ b/sdk/src/main/java/com/bugsnag/android/Bugsnag.java @@ -34,15 +34,7 @@ private Bugsnag() { */ @NonNull public static Client init(@NonNull Context androidContext) { - synchronized (lock) { - if (client == null) { - client = new Client(androidContext); - NativeInterface.configureClientObservers(client); - } else { - logClientInitWarning(); - } - } - return client; + return init(androidContext, null, true); } /** @@ -53,15 +45,7 @@ public static Client init(@NonNull Context androidContext) { */ @NonNull public static Client init(@NonNull Context androidContext, @Nullable String apiKey) { - synchronized (lock) { - if (client == null) { - client = new Client(androidContext, apiKey); - NativeInterface.configureClientObservers(client); - } else { - logClientInitWarning(); - } - } - return client; + return init(androidContext, apiKey, true); } /** @@ -75,15 +59,9 @@ public static Client init(@NonNull Context androidContext, @Nullable String apiK public static Client init(@NonNull Context androidContext, @Nullable String apiKey, boolean enableExceptionHandler) { - synchronized (lock) { - if (client == null) { - client = new Client(androidContext, apiKey, enableExceptionHandler); - NativeInterface.configureClientObservers(client); - } else { - logClientInitWarning(); - } - } - return client; + Configuration config + = ConfigFactory.createNewConfiguration(androidContext, apiKey, enableExceptionHandler); + return init(androidContext, config); } /**