From c25cd047395bfed037a0a6fa2e152459aeb8be46 Mon Sep 17 00:00:00 2001 From: Karl Stenerud Date: Fri, 6 May 2022 14:03:34 +0200 Subject: [PATCH] Fixed concurrency bug that could be triggered via the React Native plugin --- CHANGELOG.md | 7 +++ .../com/bugsnag/android/DataCollectorTest.kt | 49 +++++++++++++++++++ .../bugsnag/android/BackgroundTaskService.kt | 4 +- .../com/bugsnag/android/DeviceBuildInfo.kt | 2 +- .../bugsnag/android/DeviceDataCollector.kt | 7 ++- .../java/com/bugsnag/android/RootDetector.kt | 2 +- 6 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DataCollectorTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index bd796c25c0..ff1b3abd74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## TBD + +### Bug fixes + +* Fixed concurrency bug that could be triggered via the React Native plugin + [#1679](https://github.com/bugsnag/bugsnag-android/pull/1679) + ## 5.22.2 (2022-05-04) ### 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 new file mode 100644 index 0000000000..9704c3763b --- /dev/null +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DataCollectorTest.kt @@ -0,0 +1,49 @@ +package com.bugsnag.android + +import android.content.Context +import android.content.res.Configuration +import android.content.res.Resources +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito +import org.mockito.junit.MockitoJUnitRunner +import java.io.File +import kotlin.concurrent.thread + +@RunWith(MockitoJUnitRunner::class) +class DataCollectorTest { + + @Test + fun testConcurretAccess() { + val res = Mockito.mock(Resources::class.java) + Mockito.`when`(res.configuration).thenReturn(Configuration()) + + val collector = DeviceDataCollector( + Mockito.mock(Connectivity::class.java), + Mockito.mock(Context::class.java), + res, + "fakeDevice", + Mockito.mock(DeviceBuildInfo::class.java), + File("/tmp/javatest"), + Mockito.mock(RootDetector::class.java), + Mockito.mock(BackgroundTaskService::class.java), + Mockito.mock(Logger::class.java) + ) + + repeat(10) { index -> + collector.addRuntimeVersionInfo("key" + index, "value" + index) + } + + val count = 500 + + thread { + repeat(count) { index -> + collector.addRuntimeVersionInfo("key" + index, "value" + index) + } + } + + repeat(count) { + collector.generateDevice() + } + } +} diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/BackgroundTaskService.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/BackgroundTaskService.kt index c171c23d50..9f741f91df 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/BackgroundTaskService.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/BackgroundTaskService.kt @@ -83,7 +83,7 @@ internal fun createExecutor(name: String, keepAlive: Boolean): ThreadPoolExecuto * It also avoids short-running operations being held up by long-running operations submitted * to the same executor. */ -internal class BackgroundTaskService( +internal open class BackgroundTaskService( // these executors must remain single-threaded - the SDK makes assumptions // about synchronization based on this. @VisibleForTesting @@ -137,7 +137,7 @@ internal class BackgroundTaskService( * @see [submitTask] */ @Throws(RejectedExecutionException::class) - fun submitTask(taskType: TaskType, callable: Callable): Future { + open fun submitTask(taskType: TaskType, callable: Callable): Future { return when (taskType) { TaskType.ERROR_REQUEST -> errorExecutor.submit(callable) TaskType.SESSION_REQUEST -> sessionExecutor.submit(callable) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceBuildInfo.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceBuildInfo.kt index 39ed0fa205..0628fc6e77 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceBuildInfo.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceBuildInfo.kt @@ -2,7 +2,7 @@ package com.bugsnag.android import android.os.Build -internal class DeviceBuildInfo( +internal open class DeviceBuildInfo( val manufacturer: String?, val model: String?, val osVersion: String?, 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 69c4063e97..490d7fcca7 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 @@ -42,7 +42,7 @@ internal class DeviceDataCollector( private val screenResolution = getScreenResolution() private val locale = Locale.getDefault().toString() private val cpuAbi = getCpuAbi() - private val runtimeVersions: MutableMap + private var runtimeVersions: MutableMap private val rootedFuture: Future? private val totalMemoryFuture: Future? = retrieveTotalDeviceMemory() private var orientation = AtomicInteger(resources.configuration.orientation) @@ -293,6 +293,9 @@ internal class DeviceDataCollector( } fun addRuntimeVersionInfo(key: String, value: String) { - runtimeVersions[key] = value + // Use copy-on-write to avoid a ConcurrentModificationException in generateDeviceWithState + val newRuntimeVersions = runtimeVersions.toMutableMap() + newRuntimeVersions[key] = value + runtimeVersions = newRuntimeVersions } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt index 5e9c7a7563..0026ef2ec9 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt @@ -14,7 +14,7 @@ import java.util.concurrent.atomic.AtomicBoolean * possible to manipulate Java return values & native library loading, it will always be possible * for a determined application to defeat these root checks. */ -internal class RootDetector @JvmOverloads constructor( +internal open class RootDetector @JvmOverloads constructor( private val deviceBuildInfo: DeviceBuildInfo = DeviceBuildInfo.defaultInfo(), private val rootBinaryLocations: List = ROOT_INDICATORS, private val buildProps: File = BUILD_PROP_FILE,