Merge pull request #1785 from bugsnag/release/v5.28.3

Release v5.28.3
bugsnag · Nov 16, 2022 · 7ebe519 · 7ebe519
2 parents c288805 + 17cbed4
commit 7ebe519
Show file tree

Hide file tree

Showing 36 changed files with 163 additions and 37 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,12 @@
 # Changelog
 
+## 5.28.3 (2022-11-16)
+
+### Bug fixes
+
+* Fixed a very rare race-condition in refreshSymbolTable that could lead to empty native stack traces being reported
+  [#1781](https://github.com/bugsnag/bugsnag-android/pull/1781)
+
 ## 5.28.2 (2022-11-08)
 
 ### Bug fixes

diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DataCollectorTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DataCollectorTest.kt
@@ -3,6 +3,7 @@ package com.bugsnag.android
 import android.content.Context
 import android.content.res.Configuration
 import android.content.res.Resources
+import com.bugsnag.android.internal.BackgroundTaskService
 import org.junit.Ignore
 import org.junit.Test
 import org.junit.runner.RunWith

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java
@@ -2,11 +2,13 @@
 
 import static com.bugsnag.android.SeverityReason.REASON_HANDLED_EXCEPTION;
 
+import com.bugsnag.android.internal.BackgroundTaskService;
 import com.bugsnag.android.internal.ImmutableConfig;
 import com.bugsnag.android.internal.InternalMetrics;
 import com.bugsnag.android.internal.InternalMetricsImpl;
 import com.bugsnag.android.internal.InternalMetricsNoop;
 import com.bugsnag.android.internal.StateObserver;
+import com.bugsnag.android.internal.TaskType;
 import com.bugsnag.android.internal.dag.ConfigModule;
 import com.bugsnag.android.internal.dag.ContextModule;
 import com.bugsnag.android.internal.dag.SystemServiceModule;

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt
@@ -1,6 +1,7 @@
 package com.bugsnag.android
 
 import android.os.Environment
+import com.bugsnag.android.internal.BackgroundTaskService
 import com.bugsnag.android.internal.dag.ConfigModule
 import com.bugsnag.android.internal.dag.ContextModule
 import com.bugsnag.android.internal.dag.DependencyModule

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeliveryDelegate.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeliveryDelegate.java
@@ -2,7 +2,9 @@
 
 import static com.bugsnag.android.SeverityReason.REASON_PROMISE_REJECTION;
 
+import com.bugsnag.android.internal.BackgroundTaskService;
 import com.bugsnag.android.internal.ImmutableConfig;
+import com.bugsnag.android.internal.TaskType;
 
 import androidx.annotation.NonNull;
 import androidx.annotation.VisibleForTesting;

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt
@@ -11,6 +11,8 @@ import android.content.res.Resources
 import android.os.BatteryManager
 import android.os.Build
 import android.provider.Settings
+import com.bugsnag.android.internal.BackgroundTaskService
+import com.bugsnag.android.internal.TaskType
 import java.io.File
 import java.util.Date
 import java.util.Locale

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt
@@ -1,5 +1,6 @@
 package com.bugsnag.android
 
+import com.bugsnag.android.internal.BackgroundTaskService
 import com.bugsnag.android.internal.dag.ConfigModule
 import com.bugsnag.android.internal.dag.ContextModule
 import com.bugsnag.android.internal.dag.DependencyModule

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStore.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStore.java
@@ -1,6 +1,8 @@
 package com.bugsnag.android;
 
+import com.bugsnag.android.internal.BackgroundTaskService;
 import com.bugsnag.android.internal.ImmutableConfig;
+import com.bugsnag.android.internal.TaskType;
 
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java
@@ -3,8 +3,10 @@
 import static com.bugsnag.android.DeliveryHeadersKt.HEADER_INTERNAL_ERROR;
 import static com.bugsnag.android.SeverityReason.REASON_UNHANDLED_EXCEPTION;
 
+import com.bugsnag.android.internal.BackgroundTaskService;
 import com.bugsnag.android.internal.ImmutableConfig;
 import com.bugsnag.android.internal.JsonHelper;
+import com.bugsnag.android.internal.TaskType;
 
 import android.annotation.SuppressLint;
 import android.content.Context;

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/LibraryLoader.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/LibraryLoader.java
@@ -1,5 +1,7 @@
 package com.bugsnag.android;
 
+import com.bugsnag.android.internal.TaskType;
+
 import java.util.concurrent.atomic.AtomicBoolean;
 
 class LibraryLoader {

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Notifier.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/Notifier.kt
@@ -7,7 +7,7 @@ import java.io.IOException
  */
 class Notifier @JvmOverloads constructor(
     var name: String = "Android Bugsnag Notifier",
-    var version: String = "5.28.2",
+    var version: String = "5.28.3",
     var url: String = "https://bugsnag.com"
 ) : JsonStream.Streamable {
 

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionTracker.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionTracker.java
@@ -1,7 +1,9 @@
 package com.bugsnag.android;
 
+import com.bugsnag.android.internal.BackgroundTaskService;
 import com.bugsnag.android.internal.DateUtils;
 import com.bugsnag.android.internal.ImmutableConfig;
+import com.bugsnag.android.internal.TaskType;
 
 import android.os.SystemClock;
 
@@ -18,7 +20,6 @@
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.atomic.AtomicReference;
 
 class SessionTracker extends BaseObservable {
 

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/TrackerModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/TrackerModule.kt
@@ -1,5 +1,6 @@
 package com.bugsnag.android
 
+import com.bugsnag.android.internal.BackgroundTaskService
 import com.bugsnag.android.internal.dag.ConfigModule
 import com.bugsnag.android.internal.dag.DependencyModule
 

diff --git a/.../bugsnag/android/BackgroundTaskService.kt → ...android/internal/BackgroundTaskService.kt b/.../bugsnag/android/BackgroundTaskService.kt → ...android/internal/BackgroundTaskService.kt
@@ -1,4 +1,4 @@
-package com.bugsnag.android
+package com.bugsnag.android.internal
 
 import androidx.annotation.VisibleForTesting
 import java.util.concurrent.BlockingQueue
@@ -18,7 +18,7 @@ import java.lang.Thread as JThread
  * The type of task which is being submitted. This determines which execution queue
  * the task will be added to.
  */
-internal enum class TaskType {
+enum class TaskType {
 
     /**
      * A task that sends an error request. Any filesystem operations
@@ -91,7 +91,7 @@ internal fun createExecutor(name: String, type: TaskType, keepAlive: Boolean): E
  * It also avoids short-running operations being held up by long-running operations submitted
  * to the same executor.
  */
-internal class BackgroundTaskService(
+class BackgroundTaskService(
     // these executors must remain single-threaded - the SDK makes assumptions
     // about synchronization based on this.
     @get:VisibleForTesting

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt
@@ -1,7 +1,7 @@
 package com.bugsnag.android.internal.dag
 
-import com.bugsnag.android.BackgroundTaskService
-import com.bugsnag.android.TaskType
+import com.bugsnag.android.internal.BackgroundTaskService
+import com.bugsnag.android.internal.TaskType
 
 internal abstract class DependencyModule {
 

diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt
@@ -3,6 +3,7 @@ package com.bugsnag.android
 import android.content.Context
 import android.content.res.Configuration
 import android.content.res.Resources
+import com.bugsnag.android.internal.BackgroundTaskService
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNull

diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryDelegateTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryDelegateTest.kt
@@ -1,6 +1,7 @@
 package com.bugsnag.android
 
 import com.bugsnag.android.BugsnagTestUtils.generateImmutableConfig
+import com.bugsnag.android.internal.BackgroundTaskService
 import com.bugsnag.android.internal.StateObserver
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotNull

diff --git a/...ag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt b/...ag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt
@@ -6,6 +6,7 @@ import android.content.res.Configuration
 import android.content.res.Resources
 import android.util.DisplayMetrics
 import com.bugsnag.android.BugsnagTestUtils.generateDeviceBuildInfo
+import com.bugsnag.android.internal.BackgroundTaskService
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.Parameterized

diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt
@@ -6,6 +6,7 @@ import android.content.res.Configuration
 import android.content.res.Resources
 import android.util.DisplayMetrics
 import com.bugsnag.android.BugsnagTestUtils.generateDeviceBuildInfo
+import com.bugsnag.android.internal.BackgroundTaskService
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.Parameterized

diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/EventFilenameTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/EventFilenameTest.kt
@@ -1,6 +1,7 @@
 package com.bugsnag.android
 
 import com.bugsnag.android.EventStore.EVENT_COMPARATOR
+import com.bugsnag.android.internal.BackgroundTaskService
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNull

diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/EventStoreMaxLimitTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/EventStoreMaxLimitTest.kt
@@ -2,6 +2,7 @@ package com.bugsnag.android
 
 import com.bugsnag.android.BugsnagTestUtils.generateConfiguration
 import com.bugsnag.android.BugsnagTestUtils.generateEvent
+import com.bugsnag.android.internal.BackgroundTaskService
 import com.bugsnag.android.internal.ImmutableConfig
 import com.bugsnag.android.internal.convertToImmutableConfig
 import org.junit.After

diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt
@@ -5,6 +5,7 @@ import android.os.storage.StorageManager
 import com.bugsnag.android.BugsnagTestUtils.generateAppWithState
 import com.bugsnag.android.BugsnagTestUtils.generateDeviceWithState
 import com.bugsnag.android.BugsnagTestUtils.generateImmutableConfig
+import com.bugsnag.android.internal.BackgroundTaskService
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotNull
 import org.junit.Test

diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/LaunchCrashDeliveryTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/LaunchCrashDeliveryTest.kt
@@ -2,6 +2,7 @@ package com.bugsnag.android
 
 import com.bugsnag.android.BugsnagTestUtils.generateConfiguration
 import com.bugsnag.android.BugsnagTestUtils.generateEvent
+import com.bugsnag.android.internal.BackgroundTaskService
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue

diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionTrackerPauseResumeTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionTrackerPauseResumeTest.kt
@@ -4,6 +4,7 @@ import android.app.ActivityManager
 import android.content.Context
 import com.bugsnag.android.BugsnagTestUtils.generateConfiguration
 import com.bugsnag.android.BugsnagTestUtils.generateDevice
+import com.bugsnag.android.internal.BackgroundTaskService
 import com.bugsnag.android.internal.ImmutableConfig
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse

diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionTrackerTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionTrackerTest.java
@@ -7,6 +7,7 @@
 import static org.junit.Assert.assertNull;
 import static org.mockito.Mockito.when;
 
+import com.bugsnag.android.internal.BackgroundTaskService;
 import com.bugsnag.android.internal.ImmutableConfig;
 
 import android.app.ActivityManager;

diff --git a/...snag/android/BackgroundTaskServiceTest.kt → ...oid/internal/BackgroundTaskServiceTest.kt b/...snag/android/BackgroundTaskServiceTest.kt → ...oid/internal/BackgroundTaskServiceTest.kt
@@ -1,4 +1,4 @@
-package com.bugsnag.android
+package com.bugsnag.android.internal
 
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull

diff --git a/bugsnag-plugin-android-ndk/src/main/java/com/bugsnag/android/NdkPlugin.kt b/bugsnag-plugin-android-ndk/src/main/java/com/bugsnag/android/NdkPlugin.kt
@@ -25,7 +25,7 @@ internal class NdkPlugin : Plugin {
         private set
 
     private fun initNativeBridge(client: Client): NativeBridge {
-        val nativeBridge = NativeBridge()
+        val nativeBridge = NativeBridge(client.bgTaskService)
         client.addObserver(nativeBridge)
         client.setupNdkPlugin()
         return nativeBridge

diff --git a/bugsnag-plugin-android-ndk/src/main/java/com/bugsnag/android/ndk/NativeBridge.kt b/bugsnag-plugin-android-ndk/src/main/java/com/bugsnag/android/ndk/NativeBridge.kt
@@ -17,18 +17,21 @@ import com.bugsnag.android.StateEvent.UpdateContext
 import com.bugsnag.android.StateEvent.UpdateInForeground
 import com.bugsnag.android.StateEvent.UpdateOrientation
 import com.bugsnag.android.StateEvent.UpdateUser
+import com.bugsnag.android.internal.BackgroundTaskService
 import com.bugsnag.android.internal.StateObserver
+import com.bugsnag.android.internal.TaskType
 import java.io.File
 import java.io.FileFilter
 import java.nio.charset.Charset
 import java.util.UUID
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.concurrent.locks.ReentrantLock
+import kotlin.concurrent.withLock
 
 /**
  * Observes changes in the Bugsnag environment, propagating them to the native layer
  */
-class NativeBridge : StateObserver {
+class NativeBridge(private val bgTaskService: BackgroundTaskService) : StateObserver {
 
     private val lock = ReentrantLock()
     private val installed = AtomicBoolean(false)
@@ -125,7 +128,14 @@ class NativeBridge : StateObserver {
                 makeSafe(event.contextActivity ?: "")
             )
             is StateEvent.UpdateLastRunInfo -> updateLastRunInfo(event.consecutiveLaunchCrashes)
-            is StateEvent.UpdateIsLaunching -> updateIsLaunching(event.isLaunching)
+            is StateEvent.UpdateIsLaunching -> {
+                updateIsLaunching(event.isLaunching)
+
+                if (!event.isLaunching) {
+                    // we refreshSymbolTable on the background to avoid holding up the main thread
+                    bgTaskService.submitTask(TaskType.DEFAULT, this::refreshSymbolTable)
+                }
+            }
             is UpdateOrientation -> updateOrientation(event.orientation ?: "")
             is UpdateUser -> {
                 updateUserId(makeSafe(event.user.id ?: ""))
@@ -164,12 +174,13 @@ class NativeBridge : StateObserver {
     }
 
     private fun deliverPendingReports() {
-        lock.lock()
         val filenameRegex = """.*\.crash$""".toRegex()
+        lock.lock()
         try {
             val outDir = reportDirectory
             if (outDir.exists()) {
-                val fileList = outDir.listFiles(FileFilter { filenameRegex.containsMatchIn(it.name) })
+                val fileList =
+                    outDir.listFiles(FileFilter { filenameRegex.containsMatchIn(it.name) })
                 if (fileList != null) {
                     for (file in fileList) {
                         deliverReportAtPath(file.absolutePath)
@@ -186,8 +197,7 @@ class NativeBridge : StateObserver {
     }
 
     private fun handleInstallMessage(arg: Install) {
-        lock.lock()
-        try {
+        lock.withLock {
             if (installed.get()) {
                 logger.w("Received duplicate setup message with arg: $arg")
             } else {
@@ -204,8 +214,6 @@ class NativeBridge : StateObserver {
                 )
                 installed.set(true)
             }
-        } finally {
-            lock.unlock()
         }
     }
 

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c
@@ -517,10 +517,6 @@ Java_com_bugsnag_android_ndk_NativeBridge_updateIsLaunching(
   bugsnag_app_set_is_launching(&bsg_global_env->next_event, new_value);
   bsg_update_next_run_info(bsg_global_env);
   release_env_write_lock();
-
-  if (!new_value) {
-    bugsnag_refresh_symbol_table();
-  }
 }
 
 JNIEXPORT void JNICALL