From 451064d477d663e6d1fe53a43b722b2f1db454a6 Mon Sep 17 00:00:00 2001 From: jason Date: Fri, 8 Nov 2024 11:26:38 +0000 Subject: [PATCH] feat(exitinfo): ExitInfoPluginStore keeps data in memory to avoid re-loading constantly --- .../android/BugsnagExitInfoPluginStoreTest.kt | 68 +++++++------------ .../bugsnag/android/BugsnagExitInfoPlugin.kt | 5 +- .../bugsnag/android/ExitInfoPluginStore.kt | 51 ++++++++++---- 3 files changed, 65 insertions(+), 59 deletions(-) diff --git a/bugsnag-plugin-android-exitinfo/src/androidTest/java/com/bugsnag/android/BugsnagExitInfoPluginStoreTest.kt b/bugsnag-plugin-android-exitinfo/src/androidTest/java/com/bugsnag/android/BugsnagExitInfoPluginStoreTest.kt index a9252572f4..8320df8199 100644 --- a/bugsnag-plugin-android-exitinfo/src/androidTest/java/com/bugsnag/android/BugsnagExitInfoPluginStoreTest.kt +++ b/bugsnag-plugin-android-exitinfo/src/androidTest/java/com/bugsnag/android/BugsnagExitInfoPluginStoreTest.kt @@ -4,7 +4,6 @@ import android.content.Context import androidx.test.core.app.ApplicationProvider import com.bugsnag.android.internal.ImmutableConfig import org.junit.Assert.assertEquals -import org.junit.Assert.assertNull import org.junit.Before import org.junit.Test import java.io.File @@ -24,7 +23,6 @@ internal class BugsnagExitInfoPluginStoreTest { ) file = File(immutableConfig.persistenceDirectory.value, "bugsnag-exit-reasons") file.delete() - exitInfoPluginStore = ExitInfoPluginStore(immutableConfig) } /** @@ -32,8 +30,10 @@ internal class BugsnagExitInfoPluginStoreTest { */ @Test fun readNonExistentFile() { - val expectedResult = null to emptySet() - assertEquals(expectedResult, exitInfoPluginStore.load()) + exitInfoPluginStore = ExitInfoPluginStore(immutableConfig) + assertEquals(0, exitInfoPluginStore.currentPid) + assertEquals(0, exitInfoPluginStore.previousPid) + assertEquals(emptySet(), exitInfoPluginStore.exitInfoKeys) } /** @@ -42,8 +42,11 @@ internal class BugsnagExitInfoPluginStoreTest { @Test fun readEmptyFile() { file.createNewFile() - val expectedResult = null to emptySet() - assertEquals(expectedResult, exitInfoPluginStore.load()) + exitInfoPluginStore = ExitInfoPluginStore(immutableConfig) + + assertEquals(0, exitInfoPluginStore.currentPid) + assertEquals(0, exitInfoPluginStore.previousPid) + assertEquals(emptySet(), exitInfoPluginStore.exitInfoKeys) } /** @@ -52,8 +55,11 @@ internal class BugsnagExitInfoPluginStoreTest { @Test fun readInvalidFileContents() { file.writeText("{\"hamster\": 2}") - val expectedResult = null to emptySet() - assertEquals(expectedResult, exitInfoPluginStore.load()) + exitInfoPluginStore = ExitInfoPluginStore(immutableConfig) + + assertEquals(0, exitInfoPluginStore.currentPid) + assertEquals(0, exitInfoPluginStore.previousPid) + assertEquals(emptySet(), exitInfoPluginStore.exitInfoKeys) } /** @@ -63,49 +69,27 @@ internal class BugsnagExitInfoPluginStoreTest { fun readLegacyFileContents() { file.writeText("12345") exitInfoPluginStore = ExitInfoPluginStore(immutableConfig) - val info = requireNotNull(exitInfoPluginStore.load()) - assertEquals(12345, info.first) + assertEquals(12345, exitInfoPluginStore.previousPid) + assertEquals(0, exitInfoPluginStore.currentPid) } @Test - fun writableFileWithEmptyExitInfo() { - exitInfoPluginStore.persist(12345, emptySet()) - val firstPid = exitInfoPluginStore.load().first - assertNull(firstPid) + fun addExitInfoKey() { + file.writeText("12345") exitInfoPluginStore = ExitInfoPluginStore(immutableConfig) - val (storedPid, storageExitInfoKeys) = exitInfoPluginStore.load() - assertEquals(12345, storedPid) - assertEquals(emptySet(), storageExitInfoKeys) - } - - @Test - fun writableFile() { - val expectedPid = 12345 - val expectedExitInfoKeys = setOf(ExitInfoKey(111, 100L)) - exitInfoPluginStore.persist(expectedPid, expectedExitInfoKeys) + exitInfoPluginStore.currentPid = 54321 - val (storedPid, storageExitInfoKeys) = exitInfoPluginStore.load() - assertNull(storedPid) - assertEquals(emptySet(), storageExitInfoKeys) + assertEquals(12345, exitInfoPluginStore.previousPid) - exitInfoPluginStore = ExitInfoPluginStore(immutableConfig) - val (storedPid2, storageExitInfoKeys2) = exitInfoPluginStore.load() - assertEquals(expectedPid, storedPid2) - assertEquals(expectedExitInfoKeys, storageExitInfoKeys2) - } + val expectedExitInfoKey = ExitInfoKey(111, 100L) + exitInfoPluginStore.addExitInfoKey(expectedExitInfoKey) - @Test - fun addExitInfoKeyToFileContents() { - file.writeText("12345") + // reload the ExitInfoPluginStore exitInfoPluginStore = ExitInfoPluginStore(immutableConfig) - val expectedPid1 = requireNotNull(exitInfoPluginStore.load()) - assertEquals(12345, expectedPid1.first) - val expectedExitInfoKey = ExitInfoKey(111, 100L) - exitInfoPluginStore.addExitInfoKey(expectedExitInfoKey) - val (expectedPid2, storageExitInfoKeys2) = requireNotNull(exitInfoPluginStore.load()) - assertEquals(expectedPid1.first, expectedPid2) - assertEquals(setOf(expectedExitInfoKey), storageExitInfoKeys2) + assertEquals(54321, exitInfoPluginStore.previousPid) + assertEquals(0, exitInfoPluginStore.currentPid) + assertEquals(setOf(expectedExitInfoKey), exitInfoPluginStore.exitInfoKeys) } private fun generateConfiguration(): Configuration { diff --git a/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/BugsnagExitInfoPlugin.kt b/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/BugsnagExitInfoPlugin.kt index aefd531dbd..2427fadfca 100644 --- a/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/BugsnagExitInfoPlugin.kt +++ b/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/BugsnagExitInfoPlugin.kt @@ -25,12 +25,11 @@ class BugsnagExitInfoPlugin @JvmOverloads constructor( val exitInfoPluginStore = ExitInfoPluginStore(client.immutableConfig) - val (oldPid, exitInfoKeys) = exitInfoPluginStore.load() - exitInfoPluginStore.persist(android.os.Process.myPid(), exitInfoKeys) + exitInfoPluginStore.currentPid = android.os.Process.myPid() val exitInfoCallback = ExitInfoCallback( client.appContext, - oldPid, + exitInfoPluginStore.previousPid, TombstoneEventEnhancer( client.logger, configuration.listOpenFds, diff --git a/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/ExitInfoPluginStore.kt b/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/ExitInfoPluginStore.kt index 493ec38650..8b74888792 100644 --- a/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/ExitInfoPluginStore.kt +++ b/bugsnag-plugin-android-exitinfo/src/main/java/com/bugsnag/android/ExitInfoPluginStore.kt @@ -10,9 +10,40 @@ internal class ExitInfoPluginStore(config: ImmutableConfig) { private val file: File = File(config.persistenceDirectory.value, "bugsnag-exit-reasons") private val logger: Logger = config.logger private val lock = ReentrantReadWriteLock() - private val isFirstRun: Boolean = !file.exists() - fun persist(currentPid: Int, exitInfoKeys: Set) { + var previousPid: Int = 0 + private set + + var currentPid: Int = 0 + + private var _exitInfoKeys = HashSet() + val exitInfoKeys: Set get() = _exitInfoKeys + + init { + load() + } + + private fun load() { + lock.readLock().withLock { + val json = tryLoadJson() + if (json != null) { + if (previousPid == 0) { + previousPid = json.first ?: 0 + } else { + currentPid = json.first ?: 0 + } + + _exitInfoKeys = json.second + } else { + val legacy = tryLoadLegacy() + if (legacy != null) { + previousPid = legacy + } + } + } + } + + fun persist() { lock.writeLock().withLock { try { file.writer().buffered().use { writer -> @@ -30,19 +61,12 @@ internal class ExitInfoPluginStore(config: ImmutableConfig) { } } - fun load(): Pair> { - if (isFirstRun) { - return null to emptySet() - } - return tryLoadJson() ?: (tryLoadLegacy() to emptySet()) - } - - private fun tryLoadJson(): Pair>? { + private fun tryLoadJson(): Pair>? { try { val fileContents = file.readText() val jsonObject = JSONObject(fileContents) val currentPid = jsonObject.getInt("pid") - val exitInfoKeys = mutableSetOf() + val exitInfoKeys = HashSet() val jsonArray = jsonObject.getJSONArray("exitInfoKeys") for (i in 0 until jsonArray.length()) { val exitInfoKeyObject = jsonArray.getJSONObject(i) @@ -74,8 +98,7 @@ internal class ExitInfoPluginStore(config: ImmutableConfig) { } fun addExitInfoKey(exitInfoKey: ExitInfoKey) { - val (oldPid, exitInfoKeys) = load() - val newExitInfoKeys = exitInfoKeys.toMutableSet().plus(exitInfoKey) - oldPid?.let { persist(it, newExitInfoKeys) } + _exitInfoKeys.add(exitInfoKey) + persist() } }