diff --git a/CHANGELOG.md b/CHANGELOG.md index a38d934b9a..a7ec678a4c 100644 --- 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 index 524333c14f..443bf353c6 100644 --- 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 index bd532bc4d3..9f34ac3b41 100644 --- 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 index 20bf61229c..70e9bd78f1 100644 --- 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 index 7e4d1b207b..5c4ef01616 100644 --- 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 index ed3573f3fe..a7755a2edc 100644 --- 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 index 03f01f0a3e..c0573d6a3e 100644 --- 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 index 593003ce99..6d1f1ab574 100644 --- 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 index d4001aa3e5..0bf0d49a05 100644 --- 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 index 6d8f216b71..1a5e8caf77 100644 --- 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 index 8341ec00b9..c8609a883f 100644 --- 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 index ddb2f052d5..f31154f1fd 100644 --- 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 index a293539f28..ae1db31f2a 100644 --- 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-core/src/main/java/com/bugsnag/android/BackgroundTaskService.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/BackgroundTaskService.kt similarity index 98% rename from bugsnag-android-core/src/main/java/com/bugsnag/android/BackgroundTaskService.kt rename to bugsnag-android-core/src/main/java/com/bugsnag/android/internal/BackgroundTaskService.kt index 76a92312a6..ef3394fd52 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/BackgroundTaskService.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/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 index f766b0dd2c..731e5adfc5 100644 --- 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 index 32c047bddd..160308534d 100644 --- 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 index 09601a756d..9f7291a4d1 100644 --- 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/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt index 7fd72996c2..09ca3d3ab3 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt +++ b/bugsnag-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 index 06a9e26a3a..c3b8c3d079 100644 --- 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 index c533886aeb..75f76b7fb3 100644 --- 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 index 864acadb7f..d3432f2289 100644 --- 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 index f5cc73ce74..d7f7724789 100644 --- 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 index 3978dcb8e6..bd49e955ec 100644 --- 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 index e83491ae65..cb5aab49c8 100644 --- 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 index a42bf9f824..de1cd60c5d 100644 --- 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/bugsnag-android-core/src/test/java/com/bugsnag/android/BackgroundTaskServiceTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/internal/BackgroundTaskServiceTest.kt similarity index 99% rename from bugsnag-android-core/src/test/java/com/bugsnag/android/BackgroundTaskServiceTest.kt rename to bugsnag-android-core/src/test/java/com/bugsnag/android/internal/BackgroundTaskServiceTest.kt index 2e071a13fc..b06daee347 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/BackgroundTaskServiceTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/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 index fab6d402fe..80ff9efad0 100644 --- 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 index 911af747fb..c3528ab624 100644 --- 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 index 10eada5c2f..c0dce3da34 100644 --- 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 diff --git a/bugsnag-plugin-android-ndk/src/main/jni/utils/stack_unwinder.cpp b/bugsnag-plugin-android-ndk/src/main/jni/utils/stack_unwinder.cpp index 907cb67419..3df2edb00d 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/utils/stack_unwinder.cpp +++ b/bugsnag-plugin-android-ndk/src/main/jni/utils/stack_unwinder.cpp @@ -16,7 +16,7 @@ static unwindstack::Unwinder *crash_time_unwinder; // soft lock for using the crash time unwinder - if active, return without // attempting to unwind. This isn't a "real" lock to avoid deadlocking in the // event of a crash while handling an ANR or the reverse. -static bool unwinding_crash_stack; +static std::atomic_bool unwinding_crash_stack = ATOMIC_VAR_INIT(false); // Thread-safe, reusable unwinder - uses thread-specific memory caches static unwindstack::LocalUnwinder *current_time_unwinder; @@ -114,35 +114,57 @@ static void populate_code_identifier(const unwindstack::FrameData &frame, } void bsg_unwinder_refresh(void) { - if (crash_time_unwinder == nullptr) { - return; - } + auto crash_time_maps = new unwindstack::LocalUpdatableMaps(); + if (crash_time_maps->Parse()) { + std::shared_ptr crash_time_memory( + new unwindstack::MemoryLocal); + auto new_uwinder = new unwindstack::Unwinder( + BUGSNAG_FRAMES_MAX, crash_time_maps, + unwindstack::Regs::CreateFromLocal(), crash_time_memory); + auto arch = unwindstack::Regs::CurrentArch(); + auto dexfiles_ptr = unwindstack::CreateDexFiles(arch, crash_time_memory); + new_uwinder->SetDexFiles(dexfiles_ptr.get()); - auto *crash_time_maps = dynamic_cast( - crash_time_unwinder->GetMaps()); - if (crash_time_maps != nullptr) { - crash_time_maps->Reparse(nullptr); + auto old_unwinder = crash_time_unwinder; + crash_time_unwinder = new_uwinder; + + // don't destroy an unwinder that is currently in use + if (!unwinding_crash_stack.load()) { + delete old_unwinder->GetMaps(); + delete old_unwinder; + } } } ssize_t bsg_unwind_crash_stack(bugsnag_stackframe stack[BUGSNAG_FRAMES_MAX], siginfo_t *info, void *user_context) { - if (crash_time_unwinder == nullptr || unwinding_crash_stack) { + + // we always check unwinding_crash_stack and set *before* attempting to + // retrieve the crash unwinder to avoid picking up an unwinder that is about + // to be destroyed by bsg_unwinder_refresh + static bool expected = false; + if (!std::atomic_compare_exchange_strong(&unwinding_crash_stack, &expected, + true)) { + return 0; + } + auto local_unwinder = crash_time_unwinder; + if (local_unwinder == nullptr) { + unwinding_crash_stack = false; return 0; } - unwinding_crash_stack = true; + if (user_context) { - crash_time_unwinder->SetRegs(unwindstack::Regs::CreateFromUcontext( + local_unwinder->SetRegs(unwindstack::Regs::CreateFromUcontext( unwindstack::Regs::CurrentArch(), user_context)); } else { auto regs = unwindstack::Regs::CreateFromLocal(); unwindstack::RegsGetLocal(regs); - crash_time_unwinder->SetRegs(regs); + local_unwinder->SetRegs(regs); } - crash_time_unwinder->Unwind(); + local_unwinder->Unwind(); int frame_count = 0; - for (auto &frame : crash_time_unwinder->frames()) { + for (auto &frame : local_unwinder->frames()) { auto &dst_frame = stack[frame_count]; dst_frame.frame_address = frame.pc; dst_frame.line_number = frame.rel_pc; diff --git a/examples/sdk-app-example/app/build.gradle b/examples/sdk-app-example/app/build.gradle index 2039e65123..6391bbd09f 100644 --- a/examples/sdk-app-example/app/build.gradle +++ b/examples/sdk-app-example/app/build.gradle @@ -38,7 +38,7 @@ android { } dependencies { - implementation "com.bugsnag:bugsnag-android:5.28.2" + implementation "com.bugsnag:bugsnag-android:5.28.3" implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version" implementation "androidx.appcompat:appcompat:1.4.0" implementation "com.google.android.material:material:1.4.0" diff --git a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/cpp/cxx-scenarios-bugsnag.cpp b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/cpp/cxx-scenarios-bugsnag.cpp index 7a95d11486..1e42e191a7 100644 --- a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/cpp/cxx-scenarios-bugsnag.cpp +++ b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/cpp/cxx-scenarios-bugsnag.cpp @@ -404,4 +404,19 @@ Java_com_bugsnag_android_mazerunner_scenarios_MetadataStringsTooLargeScenario_na return value / x / 8; } +static volatile int *the_value; + +int __attribute__((optnone)) get_the_null_value() { + // assume this function is very interesting + return *the_value; +} + +extern "C" +JNIEXPORT jint JNICALL +Java_com_bugsnag_android_mazerunner_scenarios_CXXRefreshSymbolTableDuringCrashScenario_activate( + JNIEnv *env, jobject thiz) { + + return get_the_null_value(); +} + } diff --git a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/java/com/bugsnag/android/mazerunner/scenarios/CXXRefreshSymbolTableDuringCrashScenario.kt b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/java/com/bugsnag/android/mazerunner/scenarios/CXXRefreshSymbolTableDuringCrashScenario.kt new file mode 100644 index 0000000000..5560c0ef99 --- /dev/null +++ b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/src/main/java/com/bugsnag/android/mazerunner/scenarios/CXXRefreshSymbolTableDuringCrashScenario.kt @@ -0,0 +1,35 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Configuration +import com.bugsnag.android.ndk.BugsnagNDK +import kotlin.concurrent.thread + +class CXXRefreshSymbolTableDuringCrashScenario( + config: Configuration, + context: Context, + eventMetadata: String? +) : Scenario(config, context, eventMetadata) { + + companion object { + init { + System.loadLibrary("bugsnag-ndk") + System.loadLibrary("cxx-scenarios-bugsnag") + } + } + + external fun activate(): Int + + override fun startScenario() { + super.startScenario() + + thread { + while (true) { + BugsnagNDK.refreshSymbolTable() + } + } + + Thread.sleep(1L) + activate() + } +} diff --git a/features/fixtures/mazerunner/cxx-scenarios/src/main/cpp/cxx-scenarios.cpp b/features/fixtures/mazerunner/cxx-scenarios/src/main/cpp/cxx-scenarios.cpp index d71da46bf1..c6fe958d2e 100644 --- a/features/fixtures/mazerunner/cxx-scenarios/src/main/cpp/cxx-scenarios.cpp +++ b/features/fixtures/mazerunner/cxx-scenarios/src/main/cpp/cxx-scenarios.cpp @@ -275,3 +275,4 @@ Java_com_bugsnag_android_mazerunner_scenarios_UnhandledNdkAutoNotifyFalseScenari } } + diff --git a/features/full_tests/native_crash_handling.feature b/features/full_tests/native_crash_handling.feature index 21ad8d64d0..69df20ae69 100644 --- a/features/full_tests/native_crash_handling.feature +++ b/features/full_tests/native_crash_handling.feature @@ -157,3 +157,18 @@ Feature: Native crash reporting And the "lineNumber" of stack frame 0 equals 0 And the first significant stack frames match: | dispatch::Handler::handle(_jobject*) | CXXCallNullFunctionPointerScenario.cpp | 9 | + + Scenario: Refresh symbol table during a crash + When I run "CXXRefreshSymbolTableDuringCrashScenario" and relaunch the crashed app + And I configure Bugsnag for "CXXRefreshSymbolTableDuringCrashScenario" + And I wait to receive an error + Then the error payload contains a completed unhandled native report + And the exception "errorClass" equals one of: + | SIGABRT | + | SIGSEGV | + And the exception "message" equals one of: + | Abort program | + | Segmentation violation (invalid memory reference) | + And the exception "type" equals "c" + And the event "severity" equals "error" + And the event "unhandled" is true diff --git a/gradle.properties b/gradle.properties index 5101b06fb0..2963133c42 100644 --- a/gradle.properties +++ b/gradle.properties @@ -11,7 +11,7 @@ org.gradle.jvmargs=-Xmx4096m # This option should only be used with decoupled projects. More details, visit # http://www.gradle.org/docs/current/userguide/multi_project_builds.html#sec:decoupled_projects org.gradle.parallel=true -VERSION_NAME=5.28.2 +VERSION_NAME=5.28.3 GROUP=com.bugsnag POM_SCM_URL=https://github.com/bugsnag/bugsnag-android POM_SCM_CONNECTION=scm:git@github.com:bugsnag/bugsnag-android.git