diff --git a/.buildkite/pipeline.full.yml b/.buildkite/pipeline.full.yml index ced8d4b139..7349ff0281 100644 --- a/.buildkite/pipeline.full.yml +++ b/.buildkite/pipeline.full.yml @@ -159,7 +159,9 @@ steps: - "--exclude=features/full_tests/[^a-m].*.feature" - "--app=/app/build/release/fixture-r16.apk" - "--farm=bs" - - "--device=ANDROID_6_0" + - "--device=ANDROID_6_0_MOTOROLA_MOTO_X_2ND_GEN" + - "--device=ANDROID_6_0_SAMSUNG_GALAXY_S7" + - "--device=ANDROID_6_0_GOOGLE_NEXUS_6" - "--fail-fast" concurrency: 9 concurrency_group: 'browserstack-app' @@ -181,7 +183,9 @@ steps: - "--exclude=features/full_tests/[^n-z].*.feature" - "--app=/app/build/release/fixture-r16.apk" - "--farm=bs" - - "--device=ANDROID_6_0" + - "--device=ANDROID_6_0_MOTOROLA_MOTO_X_2ND_GEN" + - "--device=ANDROID_6_0_SAMSUNG_GALAXY_S7" + - "--device=ANDROID_6_0_GOOGLE_NEXUS_6" - "--fail-fast" concurrency: 9 concurrency_group: 'browserstack-app' diff --git a/.buildkite/pipeline.quick.yml b/.buildkite/pipeline.quick.yml index 65b78326f9..c3f65c3397 100644 --- a/.buildkite/pipeline.quick.yml +++ b/.buildkite/pipeline.quick.yml @@ -37,7 +37,9 @@ steps: - "features/smoke_tests" - "--app=/app/build/release/fixture-r16.apk" - "--farm=bs" - - "--device=ANDROID_6_0" + - "--device=ANDROID_6_0_MOTOROLA_MOTO_X_2ND_GEN" + - "--device=ANDROID_6_0_SAMSUNG_GALAXY_S7" + - "--device=ANDROID_6_0_GOOGLE_NEXUS_6" - "--fail-fast" concurrency: 9 concurrency_group: 'browserstack-app' diff --git a/CHANGELOG.md b/CHANGELOG.md index bf4677bed5..87e5b74514 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## 5.20.0 (2022-03-10) + +### Enhancements + +* The number of threads reported can now be limited using `Configuration.setMaxReportedThreads` (defaulting to 200) + [#1607](https://github.com/bugsnag/bugsnag-android/pull/1607) + +* Improved the performance and stability of the NDK and ANR plugins by caching JNI references on start + [#1596](https://github.com/bugsnag/bugsnag-android/pull/1596) + [#1601](https://github.com/bugsnag/bugsnag-android/pull/1601) + ## 5.19.2 (2022-01-31) ### Bug fixes diff --git a/Gemfile b/Gemfile index 91a918faaf..7163ff5598 100644 --- a/Gemfile +++ b/Gemfile @@ -4,7 +4,7 @@ source "https://rubygems.org" #gem 'bugsnag-maze-runner', path: '../maze-runner' # Or a specific release: -gem 'bugsnag-maze-runner', git: 'https://github.com/bugsnag/maze-runner', tag: 'v6.8.0' +gem 'bugsnag-maze-runner', git: 'https://github.com/bugsnag/maze-runner', tag: 'v6.9.3' # Or follow master: #gem 'bugsnag-maze-runner', git: 'https://github.com/bugsnag/maze-runner' diff --git a/Gemfile.lock b/Gemfile.lock index 4c7cb46dea..18305c3b6f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,9 +1,9 @@ GIT remote: https://github.com/bugsnag/maze-runner - revision: fe12189f83aad154f54221ee0fcd41b483d3c0d1 - tag: v6.8.0 + revision: 3230db9009a6f5ee9d8beac07deb0d87658521ea + tag: v6.9.3 specs: - bugsnag-maze-runner (6.8.0) + bugsnag-maze-runner (6.9.3) appium_lib (~> 11.2.0) bugsnag (~> 6.24) cucumber (~> 7.1) @@ -27,7 +27,7 @@ GEM appium_lib_core (4.7.1) faye-websocket (~> 0.11.0) selenium-webdriver (~> 3.14, >= 3.14.1) - bugsnag (6.24.1) + bugsnag (6.24.2) concurrent-ruby (~> 1.0) builder (3.2.4) childprocess (3.0.0) @@ -45,10 +45,10 @@ GEM mime-types (~> 3.3, >= 3.3.1) multi_test (~> 0.1, >= 0.1.2) sys-uname (~> 1.2, >= 1.2.2) - cucumber-core (10.1.0) + cucumber-core (10.1.1) cucumber-gherkin (~> 22.0, >= 22.0.0) cucumber-messages (~> 17.1, >= 17.1.1) - cucumber-tag-expressions (~> 4.0, >= 4.0.2) + cucumber-tag-expressions (~> 4.1, >= 4.1.0) cucumber-create-meta (6.0.4) cucumber-messages (~> 17.1, >= 17.1.1) sys-uname (~> 1.2, >= 1.2.2) @@ -60,17 +60,16 @@ GEM cucumber-messages (~> 17.1, >= 17.1.0) cucumber-messages (17.1.1) cucumber-tag-expressions (4.1.0) - cucumber-wire (6.2.0) + cucumber-wire (6.2.1) cucumber-core (~> 10.1, >= 10.1.0) cucumber-cucumber-expressions (~> 14.0, >= 14.0.0) - cucumber-messages (~> 17.1, >= 17.1.1) curb (0.9.11) - diff-lcs (1.4.4) + diff-lcs (1.5.0) eventmachine (1.2.7) faye-websocket (0.11.1) eventmachine (>= 0.12.0) websocket-driver (>= 0.5.1) - ffi (1.15.4) + ffi (1.15.5) license_finder (6.15.0) bundler rubyzip (>= 1, < 3) @@ -80,9 +79,9 @@ GEM xml-simple (~> 1.1.5) mime-types (3.4.1) mime-types-data (~> 3.2015) - mime-types-data (3.2021.1115) + mime-types-data (3.2022.0105) multi_test (0.1.2) - nokogiri (1.12.5-x86_64-darwin) + nokogiri (1.13.1-x86_64-darwin) racc (~> 1.4) optimist (3.0.1) os (1.0.1) @@ -110,10 +109,11 @@ GEM PLATFORMS x86_64-darwin-19 + x86_64-darwin-20 DEPENDENCIES bugsnag-maze-runner! license_finder (~> 6.13) BUNDLED WITH - 2.2.33 + 2.3.0 diff --git a/bugsnag-android-core/detekt-baseline.xml b/bugsnag-android-core/detekt-baseline.xml index 072749065b..9070d7bbbe 100644 --- a/bugsnag-android-core/detekt-baseline.xml +++ b/bugsnag-android-core/detekt-baseline.xml @@ -17,13 +17,13 @@ LongParameterList:EventStorageModule.kt$EventStorageModule$( contextModule: ContextModule, configModule: ConfigModule, dataCollectionModule: DataCollectionModule, bgTaskService: BackgroundTaskService, trackerModule: TrackerModule, systemServiceModule: SystemServiceModule, notifier: Notifier, callbackState: CallbackState ) LongParameterList:NativeStackframe.kt$NativeStackframe$( /** * The name of the method that was being executed */ var method: String?, /** * The location of the source file */ var file: String?, /** * The line number within the source file this stackframe refers to */ var lineNumber: Number?, /** * The address of the instruction where the event occurred. */ var frameAddress: Long?, /** * The address of the function where the event occurred. */ var symbolAddress: Long?, /** * The address of the library where the event occurred. */ var loadAddress: Long?, /** * Whether this frame identifies the program counter */ var isPC: Boolean?, /** * The type of the error */ var type: ErrorType? = null ) LongParameterList:StateEvent.kt$StateEvent.Install$( @JvmField val apiKey: String, @JvmField val autoDetectNdkCrashes: Boolean, @JvmField val appVersion: String?, @JvmField val buildUuid: String?, @JvmField val releaseStage: String?, @JvmField val lastRunInfoPath: String, @JvmField val consecutiveLaunchCrashes: Int, @JvmField val sendThreads: ThreadSendPolicy ) - LongParameterList:ThreadState.kt$ThreadState$( stackTraces: MutableMap<java.lang.Thread, Array<StackTraceElement>>, currentThread: java.lang.Thread, exc: Throwable?, isUnhandled: Boolean, projectPackages: Collection<String>, logger: Logger ) + LongParameterList:ThreadState.kt$ThreadState$( allThreads: List<JavaThread>, currentThread: JavaThread, exc: Throwable?, isUnhandled: Boolean, maxThreadCount: Int, projectPackages: Collection<String>, logger: Logger ) MagicNumber:DefaultDelivery.kt$DefaultDelivery$299 MagicNumber:DefaultDelivery.kt$DefaultDelivery$429 MagicNumber:DefaultDelivery.kt$DefaultDelivery$499 MagicNumber:LastRunInfoStore.kt$LastRunInfoStore$3 MaxLineLength:LastRunInfo.kt$LastRunInfo$return "LastRunInfo(consecutiveLaunchCrashes=$consecutiveLaunchCrashes, crashed=$crashed, crashedDuringLaunch=$crashedDuringLaunch)" - MaxLineLength:ThreadState.kt$ThreadState$Thread(thread.id, thread.name, ThreadType.ANDROID, errorThread, Thread.State.forThread(thread), stacktrace, logger) + MaxLineLength:ThreadState.kt$ThreadState$"[${allThreads.size - maxThreadCount} threads omitted as the maxReportedThreads limit ($maxThreadCount) was exceeded]" ProtectedMemberInFinalClass:ConfigInternal.kt$ConfigInternal$protected val plugins = HashSet<Plugin>() ProtectedMemberInFinalClass:EventInternal.kt$EventInternal$protected fun isAnr(event: Event): Boolean ProtectedMemberInFinalClass:EventInternal.kt$EventInternal$protected fun shouldDiscardClass(): Boolean diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ThreadStateTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ThreadStateTest.kt index 56ac7546ab..0db64e674c 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ThreadStateTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/ThreadStateTest.kt @@ -20,6 +20,7 @@ class ThreadStateTest { private val threadState = ThreadState( null, true, + 1000, ThreadSendPolicy.ALWAYS, Collections.emptyList(), NoopLogger, @@ -27,6 +28,18 @@ class ThreadStateTest { ) private val json = streamableToJsonArray(threadState) + private fun allThreads(): List { + var rootGroup = Thread.currentThread().threadGroup!! + while (rootGroup.parent != null) { + rootGroup = rootGroup.parent + } + + val threadCount = rootGroup.activeCount() + val threads: Array = arrayOfNulls(threadCount) + rootGroup.enumerate(threads) + return threads.filterNotNull() + } + /** * Verifies that the current thread is serialised as an object, and that only this value * contains the errorReportingThread boolean flag @@ -50,11 +63,12 @@ class ThreadStateTest { val state = ThreadState( trace, true, + 1000, ThreadSendPolicy.ALWAYS, Collections.emptyList(), NoopLogger, otherThread, - Thread.getAllStackTraces() + allThreads() ) val json = streamableToJsonArray(state) verifyCurrentThreadStructure(json, otherThread.id) @@ -67,17 +81,20 @@ class ThreadStateTest { @Test fun testMissingCurrentThread() { val currentThread = Thread.currentThread() - val missingTraces = Thread.getAllStackTraces() - missingTraces.remove(currentThread) + val allThreads = allThreads() + val missingThreads = allThreads.filter { + it.id != currentThread.id + } val state = ThreadState( trace, true, + 1000, ThreadSendPolicy.ALWAYS, Collections.emptyList(), NoopLogger, currentThread, - missingTraces + missingThreads ) val json = streamableToJsonArray(state) @@ -92,22 +109,24 @@ class ThreadStateTest { @Test fun testHandledStacktrace() { val currentThread = Thread.currentThread() - val allStackTraces = Thread.getAllStackTraces() + val allThreads = allThreads() val state = ThreadState( trace, true, + 1000, ThreadSendPolicy.ALWAYS, Collections.emptyList(), NoopLogger, currentThread, - allStackTraces + allThreads ) val json = streamableToJsonArray(state) - // find the stack trace for the current thread that was passed as a parameter - val expectedTrace = allStackTraces.filter { - it.key.id == currentThread.id - }.map { it.value }.first() + // find the stack trace for the current thread that was passed as a parameter. + // Drop the top 3 stack elements because we're capturing the trace from a different location. + val expectedTrace = allThreads.first { + it.id == currentThread.id + }.stackTrace.drop(3) verifyCurrentThreadStructure(json, currentThread.id) { @@ -115,12 +134,13 @@ class ThreadStateTest { assertEquals(currentThread.name, it.getString("name")) assertEquals(currentThread.id, it.getLong("id")) - // stacktrace should come from the thread (check same length and line numbers) + // stacktrace should come from the thread (check same line numbers) val serialisedTrace = it.getJSONArray("stacktrace") - assertEquals(expectedTrace.size, serialisedTrace.length()) + // Only check the lower trace elements due to different trace capture locations. + val traceOffset = serialisedTrace.length() - expectedTrace.size expectedTrace.forEachIndexed { index, element -> - val jsonObject = serialisedTrace.getJSONObject(index) + val jsonObject = serialisedTrace.getJSONObject(index + traceOffset) assertEquals(element.lineNumber, jsonObject.getInt("lineNumber")) } } @@ -138,6 +158,7 @@ class ThreadStateTest { val state = ThreadState( exc, true, + 1000, ThreadSendPolicy.ALWAYS, Collections.emptyList(), NoopLogger, @@ -164,6 +185,60 @@ class ThreadStateTest { assertTrue(json.length() > 1) } + /** + * Verifies that maxReportedThreads is honored in a handled error + */ + @Test + fun testHandledStacktraceMaxReportedThreads() { + val currentThread = Thread.currentThread() + val allThreads = allThreads() + val state = ThreadState( + trace, + true, + 2, + ThreadSendPolicy.ALWAYS, + Collections.emptyList(), + NoopLogger, + currentThread, + allThreads + ) + val json = streamableToJsonArray(state) + + assertEquals(-1, json.getJSONObject(2).getInt("id")) + assert( + json.getJSONObject(2).getString("name").endsWith( + " threads omitted as the maxReportedThreads limit (2) was exceeded]", + ) + ) + } + + /** + * Verifies that maxReportedThreads is honored in an unhandled error + */ + @Test + fun testUnhandledStacktraceMaxReportedThreads() { + val currentThread = Thread.currentThread() + val exc: Throwable = RuntimeException("Whoops") + + val state = ThreadState( + exc, + true, + 4, + ThreadSendPolicy.ALWAYS, + Collections.emptyList(), + NoopLogger, + currentThread + ) + val json = streamableToJsonArray(state) + + assertEquals(-1, json.getJSONObject(4).getInt("id")) + assert( + json.getJSONObject(4).getString("name").endsWith( + " threads omitted as the maxReportedThreads limit (4) was exceeded]", + ) + ) + } + /** * Test that using [ThreadSendPolicy.NEVER] ignores any stack-traces and reports an empty * array of Threads @@ -171,15 +246,16 @@ class ThreadStateTest { @Test fun testNeverPolicyNeverSendsThreads() { val currentThread = Thread.currentThread() - val allStackTraces = Thread.getAllStackTraces() + val allThreads = allThreads() val state = ThreadState( trace, true, + 1000, ThreadSendPolicy.NEVER, Collections.emptyList(), NoopLogger, currentThread, - allStackTraces + allThreads ) val json = streamableToJsonArray(state) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt index f3ec4939f9..e85a552e1e 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/ConfigInternal.kt @@ -40,6 +40,7 @@ internal class ConfigInternal( var maxBreadcrumbs: Int = DEFAULT_MAX_BREADCRUMBS var maxPersistedEvents: Int = DEFAULT_MAX_PERSISTED_EVENTS var maxPersistedSessions: Int = DEFAULT_MAX_PERSISTED_SESSIONS + var maxReportedThreads: Int = DEFAULT_MAX_REPORTED_THREADS var context: String? = null var redactedKeys: Set @@ -99,6 +100,7 @@ internal class ConfigInternal( private const val DEFAULT_MAX_BREADCRUMBS = 50 private const val DEFAULT_MAX_PERSISTED_SESSIONS = 128 private const val DEFAULT_MAX_PERSISTED_EVENTS = 32 + private const val DEFAULT_MAX_REPORTED_THREADS = 200 private const val DEFAULT_LAUNCH_CRASH_THRESHOLD_MS: Long = 5000 @JvmStatic diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java index 3b9e6dc2a2..57bf482b00 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Configuration.java @@ -561,6 +561,32 @@ public void setMaxPersistedEvents(int maxPersistedEvents) { } } + /** + * Gets the maximum number of threads that will be reported with an event. Once the threshold is + * reached, all remaining threads will be omitted. + * + * By default, up to 200 threads are reported. + */ + public int getMaxReportedThreads() { + return impl.getMaxReportedThreads(); + } + + /** + * Sets the maximum number of threads that will be reported with an event. Once the threshold is + * reached, all remaining threads will be omitted. + * + * By default, up to 200 threads are reported. + */ + public void setMaxReportedThreads(int maxReportedThreads) { + if (maxReportedThreads >= 0) { + impl.setMaxReportedThreads(maxReportedThreads); + } else { + getLogger().e("Invalid configuration value detected. " + + "Option maxReportedThreads should be a positive integer." + + "Supplied value is " + maxReportedThreads); + } + } + /** * Sets the maximum number of persisted sessions which will be stored. Once the threshold is * reached, the oldest session will be deleted. 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 b2f08dbab0..7851ffb4c3 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.19.2", + var version: String = "5.20.0", var url: String = "https://bugsnag.com" ) : JsonStream.Streamable { diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ThreadState.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ThreadState.kt index 06e00268ac..536ed89168 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/ThreadState.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/ThreadState.kt @@ -2,25 +2,27 @@ package com.bugsnag.android import com.bugsnag.android.internal.ImmutableConfig import java.io.IOException +import java.lang.Thread as JavaThread /** * Capture and serialize the state of all threads at the time of an exception. */ -internal class ThreadState @Suppress("LongParameterList") @JvmOverloads constructor( +internal class ThreadState @Suppress("LongParameterList") constructor( exc: Throwable?, isUnhandled: Boolean, + maxThreads: Int, sendThreads: ThreadSendPolicy, projectPackages: Collection, logger: Logger, - currentThread: java.lang.Thread? = null, - stackTraces: MutableMap>? = null + currentThread: JavaThread = JavaThread.currentThread(), + allThreads: List = allThreads() ) : JsonStream.Streamable { internal constructor( exc: Throwable?, isUnhandled: Boolean, config: ImmutableConfig - ) : this(exc, isUnhandled, config.sendThreads, config.projectPackages, config.logger) + ) : this(exc, isUnhandled, config.maxReportedThreads, config.sendThreads, config.projectPackages, config.logger) val threads: MutableList @@ -30,10 +32,11 @@ internal class ThreadState @Suppress("LongParameterList") @JvmOverloads construc threads = when { recordThreads -> captureThreadTrace( - stackTraces ?: java.lang.Thread.getAllStackTraces(), - currentThread ?: java.lang.Thread.currentThread(), + allThreads, + currentThread, exc, isUnhandled, + maxThreads, projectPackages, logger ) @@ -41,37 +44,88 @@ internal class ThreadState @Suppress("LongParameterList") @JvmOverloads construc } } + companion object { + private fun rootThreadGroup(): ThreadGroup { + var group = JavaThread.currentThread().threadGroup!! + + while (group.parent != null) { + group = group.parent + } + + return group + } + + internal fun allThreads(): List { + val rootGroup = rootThreadGroup() + val threadCount = rootGroup.activeCount() + val threads: Array = arrayOfNulls(threadCount) + rootGroup.enumerate(threads) + return threads.filterNotNull() + } + } + private fun captureThreadTrace( - stackTraces: MutableMap>, - currentThread: java.lang.Thread, + allThreads: List, + currentThread: JavaThread, exc: Throwable?, isUnhandled: Boolean, + maxThreadCount: Int, projectPackages: Collection, logger: Logger ): MutableList { - // API 24/25 don't record the currentThread, add it in manually - // https://issuetracker.google.com/issues/64122757 - if (!stackTraces.containsKey(currentThread)) { - stackTraces[currentThread] = currentThread.stackTrace - } - if (exc != null && isUnhandled) { // unhandled errors use the exception trace for thread traces - stackTraces[currentThread] = exc.stackTrace + fun toBugsnagThread(thread: JavaThread): Thread { + val isErrorThread = thread.id == currentThread.id + val stackTrace = Stacktrace( + if (isErrorThread) { + if (exc != null && isUnhandled) { // unhandled errors use the exception trace for thread traces + exc.stackTrace + } else { + currentThread.stackTrace + } + } else { + thread.stackTrace + }, + projectPackages, logger + ) + + return Thread( + thread.id, + thread.name, + ThreadType.ANDROID, + isErrorThread, + Thread.State.forThread(thread), + stackTrace, + logger + ) } - val currentThreadId = currentThread.id - return stackTraces.keys - .sortedBy { it.id } - .mapNotNull { thread -> - val trace = stackTraces[thread] + // Keep the lowest ID threads (ordered). Anything after maxThreadCount is lost. + // Note: We must ensure that currentThread is always present in the final list regardless. + val keepThreads = allThreads.sortedBy { it.id }.take(maxThreadCount) - if (trace != null) { - val stacktrace = Stacktrace(trace, projectPackages, logger) - val errorThread = thread.id == currentThreadId - Thread(thread.id, thread.name, ThreadType.ANDROID, errorThread, Thread.State.forThread(thread), stacktrace, logger) - } else { - null - } - }.toMutableList() + val reportThreads = if (keepThreads.contains(currentThread)) { + keepThreads + } else { + // API 24/25 don't record the currentThread, so add it in manually + // https://issuetracker.google.com/issues/64122757 + // currentThread may also have been removed if its ID occurred after maxThreadCount + keepThreads.take(Math.max(maxThreadCount - 1, 0)).plus(currentThread).sortedBy { it.id } + }.map { toBugsnagThread(it) }.toMutableList() + + if (allThreads.size > maxThreadCount) { + reportThreads.add( + Thread( + -1, + "[${allThreads.size - maxThreadCount} threads omitted as the maxReportedThreads limit ($maxThreadCount) was exceeded]", + ThreadType.EMPTY, + false, + Thread.State.UNKNOWN, + Stacktrace(arrayOf(StackTraceElement("", "", "-", 0)), projectPackages, logger), + logger + ) + ) + } + return reportThreads } @Throws(IOException::class) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ThreadType.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ThreadType.kt index c1c3cbb5d7..60f834741c 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/ThreadType.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/ThreadType.kt @@ -5,6 +5,11 @@ package com.bugsnag.android */ enum class ThreadType(internal val desc: String) { + /** + * A thread captured from Android's JVM layer + */ + EMPTY(""), + /** * A thread captured from Android's JVM layer */ diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt index 9ace0f6686..6ccbd015f9 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt @@ -47,6 +47,7 @@ data class ImmutableConfig( val maxBreadcrumbs: Int, val maxPersistedEvents: Int, val maxPersistedSessions: Int, + val maxReportedThreads: Int, val persistenceDirectory: Lazy, val sendLaunchCrashesSynchronously: Boolean, @@ -159,6 +160,7 @@ internal fun convertToImmutableConfig( maxBreadcrumbs = config.maxBreadcrumbs, maxPersistedEvents = config.maxPersistedEvents, maxPersistedSessions = config.maxPersistedSessions, + maxReportedThreads = config.maxReportedThreads, enabledBreadcrumbTypes = config.enabledBreadcrumbTypes?.toSet(), persistenceDirectory = persistenceDir, sendLaunchCrashesSynchronously = config.sendLaunchCrashesSynchronously, diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadStateMissingTraceTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadStateMissingTraceTest.kt deleted file mode 100644 index be65725ee1..0000000000 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ThreadStateMissingTraceTest.kt +++ /dev/null @@ -1,34 +0,0 @@ -package com.bugsnag.android - -import org.junit.Assert.assertNotNull -import org.junit.Assert.assertTrue -import org.junit.Test -import java.lang.RuntimeException -import java.lang.Thread -import java.util.Collections - -class ThreadStateMissingTraceTest { - - @Test - fun handleNullThreadTraces() { - val currentThread = Thread.currentThread() - val traces = Thread.getAllStackTraces() - - // make all stacktraces null - traces.keys.forEach { - traces[it] = null - } - - val state = ThreadState( - RuntimeException(), - false, - ThreadSendPolicy.ALWAYS, - Collections.emptyList(), - NoopLogger, - currentThread, - traces - ) - assertNotNull(state) - assertTrue(state.threads.isEmpty()) - } -} diff --git a/bugsnag-plugin-android-anr/src/main/jni/anr_handler.c b/bugsnag-plugin-android-anr/src/main/jni/anr_handler.c index a4ec6d1f8f..43f7455a1d 100644 --- a/bugsnag-plugin-android-anr/src/main/jni/anr_handler.c +++ b/bugsnag-plugin-android-anr/src/main/jni/anr_handler.c @@ -12,6 +12,8 @@ #include "unwind_func.h" #include "utils/string.h" +#define JNI_VERSION JNI_VERSION_1_6 + // Lock for changing the handler configuration static pthread_mutex_t bsg_anr_handler_config = PTHREAD_MUTEX_INITIALIZER; @@ -25,7 +27,6 @@ static bool should_wait_for_semaphore = false; static sem_t watchdog_thread_semaphore; static volatile bool watchdog_thread_triggered = false; -static JavaVM *bsg_jvm = NULL; static jmethodID mthd_notify_anr_detected = NULL; static jobject obj_plugin = NULL; static jclass frame_class = NULL; @@ -41,11 +42,61 @@ static unwind_func unwind_stack_function; // bugsnag-plugin-android-ndk. Until a shared C module is available // for sharing common code when PLAT-5794 is addressed, this // duplication is a necessary evil. + +static JavaVM *jvm = NULL; +static pthread_key_t jni_cleanup_key; + +static void detach_env(void *env) { + if (jvm != NULL && env != NULL) { + (*jvm)->DetachCurrentThread(jvm); + } +} + +static JNIEnv *get_env() { + if (jvm == NULL) { + return NULL; + } + + JNIEnv *env = NULL; + switch ((*jvm)->GetEnv(jvm, (void **)&env, JNI_VERSION)) { + case JNI_OK: + return env; + case JNI_EDETACHED: + if ((*jvm)->AttachCurrentThread(jvm, &env, NULL) != JNI_OK) { + BUGSNAG_LOG("Could not attach thread to JVM"); + return NULL; + } + if (env == NULL) { + BUGSNAG_LOG("AttachCurrentThread filled a NULL JNIEnv"); + return NULL; + } + + // attach a destructor to detach the env before the thread terminates + pthread_setspecific(jni_cleanup_key, env); + + return env; + default: + BUGSNAG_LOG("Could not get JNIEnv"); + return NULL; + } +} + static bool check_and_clear_exc(JNIEnv *env) { if (env == NULL) { return false; } if ((*env)->ExceptionCheck(env)) { + BUGSNAG_LOG("BUG: JNI Native->Java call threw an exception:"); + + // Print a trace to stderr so that we can debug it + (*env)->ExceptionDescribe(env); + + // Trigger more accurate dalvik trace (this will also crash the app). + + // Code review check: THIS MUST BE COMMENTED OUT IN CHECKED IN CODE! + //(*env)->FindClass(env, NULL); + + // Clear the exception so that we don't crash. (*env)->ExceptionClear(env); return true; } @@ -53,14 +104,13 @@ static bool check_and_clear_exc(JNIEnv *env) { } static jclass safe_find_class(JNIEnv *env, const char *clz_name) { - if (env == NULL) { + if (env == NULL || clz_name == NULL) { return NULL; } - if (clz_name == NULL) { + jclass clz = (*env)->FindClass(env, clz_name); + if (check_and_clear_exc(env)) { return NULL; } - jclass clz = (*env)->FindClass(env, clz_name); - check_and_clear_exc(env); return clz; } @@ -70,7 +120,9 @@ static jmethodID safe_get_method_id(JNIEnv *env, jclass clz, const char *name, return NULL; } jmethodID methodId = (*env)->GetMethodID(env, clz, name, sig); - check_and_clear_exc(env); + if (check_and_clear_exc(env)) { + return NULL; + } return methodId; } @@ -80,49 +132,95 @@ static jfieldID safe_get_static_field_id(JNIEnv *env, jclass clz, return NULL; } jfieldID fid = (*env)->GetStaticFieldID(env, clz, name, sig); - check_and_clear_exc(env); + if (check_and_clear_exc(env)) { + return NULL; + } return fid; } static jobject safe_get_static_object_field(JNIEnv *env, jclass clz, jfieldID field) { - if (env == NULL || clz == NULL) { + if (env == NULL || clz == NULL || field == NULL) { return NULL; } jobject obj = (*env)->GetStaticObjectField(env, clz, field); - check_and_clear_exc(env); + if (check_and_clear_exc(env)) { + return NULL; + } return obj; } +static jobject safe_new_object(JNIEnv *env, jclass clz, jmethodID method, ...) { + if (env == NULL || clz == NULL) { + return NULL; + } + va_list args; + va_start(args, method); + jobject obj = (*env)->NewObjectV(env, clz, method, args); + va_end(args); + if (check_and_clear_exc(env)) { + return NULL; + } + return obj; +} + +static jstring safe_new_string_utf(JNIEnv *env, const char *str) { + if (env == NULL || str == NULL) { + return NULL; + } + jstring jstr = (*env)->NewStringUTF(env, str); + if (check_and_clear_exc(env)) { + return NULL; + } + return jstr; +} + +static void safe_delete_local_ref(JNIEnv *env, jobject obj) { + if (env != NULL && obj != NULL) { + (*env)->DeleteLocalRef(env, obj); + } +} + +// End of duplication + static bool configure_anr_jni_impl(JNIEnv *env) { - // get a global reference to the AnrPlugin class - // https://developer.android.com/training/articles/perf-jni#faq:-why-didnt-findclass-find-my-class if (env == NULL) { return false; } - int result = (*env)->GetJavaVM(env, &bsg_jvm); - if (result != 0) { - return false; - } jclass clz = safe_find_class(env, "com/bugsnag/android/AnrPlugin"); - if (check_and_clear_exc(env) || clz == NULL) { + if (clz == NULL) { return false; } mthd_notify_anr_detected = safe_get_method_id(env, clz, "notifyAnrDetected", "(Ljava/util/List;)V"); + if (mthd_notify_anr_detected == NULL) { + return false; + } // find ErrorType class jclass error_type_class = safe_find_class(env, "com/bugsnag/android/ErrorType"); + if (error_type_class == NULL) { + return false; + } jfieldID error_type_field = safe_get_static_field_id( env, error_type_class, "C", "Lcom/bugsnag/android/ErrorType;"); + if (error_type_field == NULL) { + return false; + } error_type = safe_get_static_object_field(env, error_type_class, error_type_field); + if (error_type == NULL) { + return false; + } error_type = (*env)->NewGlobalRef(env, error_type); // find NativeStackFrame class frame_class = safe_find_class(env, "com/bugsnag/android/NativeStackframe"); + if (frame_class == NULL) { + return false; + } frame_class = (*env)->NewGlobalRef(env, frame_class); // find NativeStackframe ctor @@ -131,34 +229,18 @@ static bool configure_anr_jni_impl(JNIEnv *env) { "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/Number;Ljava/lang/" "Long;Ljava/lang/Long;Ljava/lang/Long;Ljava/lang/Boolean;Lcom/bugsnag/" "android/ErrorType;)V"); - return true; -} - -static void safe_delete_local_ref(JNIEnv *env, jobject obj) { - if (env != NULL) { - (*env)->DeleteLocalRef(env, obj); + if (frame_init == NULL) { + return false; } -} -static jobject safe_new_object(JNIEnv *env, jclass clz, jmethodID method, ...) { - if (env == NULL || clz == NULL) { - return NULL; + // Initialize jvm last so that it's guaranteed NULL if any part of the init + // goes wrong. + if ((*env)->GetJavaVM(env, &jvm) != JNI_OK) { + return false; } - va_list args; - va_start(args, method); - jobject obj = (*env)->NewObjectV(env, clz, method, args); - va_end(args); - check_and_clear_exc(env); - return obj; -} -static jstring safe_new_string_utf(JNIEnv *env, const char *str) { - if (env == NULL || str == NULL) { - return NULL; - } - jstring jstr = (*env)->NewStringUTF(env, str); - check_and_clear_exc(env); - return jstr; + pthread_key_create(&jni_cleanup_key, detach_env); + return true; } static bool configure_anr_jni(JNIEnv *env) { @@ -170,39 +252,49 @@ static bool configure_anr_jni(JNIEnv *env) { } static void notify_anr_detected() { - if (!enabled) { + if (!enabled || obj_plugin == NULL) { return; } - bool should_detach = false; - JNIEnv *env; - int result = (*bsg_jvm)->GetEnv(bsg_jvm, (void **)&env, JNI_VERSION_1_4); - switch (result) { - case JNI_OK: - break; - case JNI_EDETACHED: - result = (*bsg_jvm)->AttachCurrentThread(bsg_jvm, &env, NULL); - if (result != 0) { - BUGSNAG_LOG("Failed to call JNIEnv->AttachCurrentThread(): %d", result); - return; - } - should_detach = true; - break; - default: - BUGSNAG_LOG("Failed to call JNIEnv->GetEnv(): %d", result); + JNIEnv *env = get_env(); + if (env == NULL) { return; } jclass list_class = safe_find_class(env, "java/util/LinkedList"); + if (list_class == NULL) { + return; + } jmethodID list_init = safe_get_method_id(env, list_class, "", "()V"); + if (list_init == NULL) { + return; + } jmethodID list_add = safe_get_method_id(env, list_class, "add", "(Ljava/lang/Object;)Z"); + if (list_add == NULL) { + return; + } jclass int_class = safe_find_class(env, "java/lang/Integer"); + if (int_class == NULL) { + return; + } jmethodID int_init = safe_get_method_id(env, int_class, "", "(I)V"); + if (int_init == NULL) { + return; + } jclass long_class = safe_find_class(env, "java/lang/Long"); + if (long_class == NULL) { + return; + } jmethodID long_init = safe_get_method_id(env, long_class, "", "(J)V"); + if (long_init == NULL) { + return; + } jobject jlist = safe_new_object(env, list_class, list_init); + if (jlist == NULL) { + return; + } for (ssize_t i = 0; i < anr_stacktrace_length; i++) { bugsnag_stackframe *frame = anr_stacktrace + i; jobject jmethod = safe_new_string_utf(env, frame->method); @@ -218,7 +310,7 @@ static void notify_anr_detected() { jobject jframe = safe_new_object( env, frame_class, frame_init, jmethod, jfilename, jline_number, jframe_address, jsymbol_address, jload_address, NULL, error_type); - if (jlist != NULL && list_add != NULL && jframe != NULL) { + if (jframe != NULL) { (*env)->CallBooleanMethod(env, jlist, list_add, jframe); check_and_clear_exc(env); } @@ -231,15 +323,9 @@ static void notify_anr_detected() { safe_delete_local_ref(env, jframe); } - if (obj_plugin != NULL && mthd_notify_anr_detected != NULL && jlist != NULL) { - (*env)->CallVoidMethod(env, obj_plugin, mthd_notify_anr_detected, jlist); - check_and_clear_exc(env); - } - - if (should_detach) { - (*bsg_jvm)->DetachCurrentThread( - bsg_jvm); // detach to restore initial condition - } + (*env)->CallVoidMethod(env, obj_plugin, mthd_notify_anr_detected, jlist); + check_and_clear_exc(env); + safe_delete_local_ref(env, jlist); } static inline void block_sigquit() { @@ -301,10 +387,8 @@ _Noreturn static void *sigquit_watchdog_thread_main(__unused void *_) { // Trigger Google ANR processing (occurs on a different thread). bsg_google_anr_call(); - if (enabled) { - // Do our ANR processing. - notify_anr_detected(); - } + // Trigger our ANR processing on our JNI worker thread (if enabled). + notify_anr_detected(); // Unblock SIGQUIT again so that handle_sigquit() will run again. unblock_sigquit(); diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag.c b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag.c index 9798e5b53c..da58918c6c 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag.c @@ -9,62 +9,72 @@ #include "utils/stack_unwinder.h" #include "utils/string.h" #include +#include #include #include #include -static JNIEnv *bsg_global_jni_env = NULL; - -void bugsnag_start(JNIEnv *env) { bsg_global_jni_env = env; } +void bugsnag_start(JNIEnv *env) { + if (!bsg_jni_cache_init(env)) { + BUGSNAG_LOG("Could not init JNI jni_cache."); + } +} void bugsnag_notify(const char *name, const char *message, bugsnag_severity severity) { - if (bsg_global_jni_env != NULL) { - bugsnag_notify_env(bsg_global_jni_env, name, message, severity); - } else { - BUGSNAG_LOG("Cannot bugsnag_notify before initializing with bugsnag_start"); + JNIEnv *env = bsg_jni_cache_get_env(); + if (env == NULL) { + return; } + + bugsnag_notify_env(env, name, message, severity); } void bugsnag_set_user(const char *id, const char *email, const char *name) { - if (bsg_global_jni_env != NULL) { - bugsnag_set_user_env(bsg_global_jni_env, id, email, name); - } else { - BUGSNAG_LOG( - "Cannot bugsnag_set_user before initializing with bugsnag_start"); + JNIEnv *env = bsg_jni_cache_get_env(); + if (env == NULL) { + return; } + + bugsnag_set_user_env(env, id, email, name); } void bugsnag_leave_breadcrumb(const char *message, bugsnag_breadcrumb_type type) { - if (bsg_global_jni_env != NULL) { - bugsnag_leave_breadcrumb_env(bsg_global_jni_env, message, type); - } else { - BUGSNAG_LOG("Cannot bugsnag_leave_breadcrumb_env before initializing with " - "bugsnag_start"); + JNIEnv *env = bsg_jni_cache_get_env(); + if (env == NULL) { + return; } + + bugsnag_leave_breadcrumb_env(env, message, type); } -static jfieldID parse_jseverity(JNIEnv *env, bugsnag_severity severity, - jclass severity_class) { +static jfieldID parse_jseverity(JNIEnv *env, bugsnag_severity severity) { + if (!bsg_jni_cache->initialized) { + return NULL; + } + const char *severity_sig = "Lcom/bugsnag/android/Severity;"; if (severity == BSG_SEVERITY_ERR) { - return bsg_safe_get_static_field_id(env, severity_class, "ERROR", + return bsg_safe_get_static_field_id(env, bsg_jni_cache->Severity, "ERROR", severity_sig); } else if (severity == BSG_SEVERITY_WARN) { - return bsg_safe_get_static_field_id(env, severity_class, "WARNING", + return bsg_safe_get_static_field_id(env, bsg_jni_cache->Severity, "WARNING", severity_sig); } else { - return bsg_safe_get_static_field_id(env, severity_class, "INFO", + return bsg_safe_get_static_field_id(env, bsg_jni_cache->Severity, "INFO", severity_sig); } } static void populate_notify_stacktrace(JNIEnv *env, bugsnag_stackframe *stacktrace, - ssize_t frame_count, jclass trace_class, - jmethodID trace_constructor, + ssize_t frame_count, jobjectArray trace) { + if (!bsg_jni_cache->initialized) { + return; + } + for (int i = 0; i < frame_count; i++) { bugsnag_stackframe frame = stacktrace[i]; @@ -96,8 +106,9 @@ static void populate_notify_stacktrace(JNIEnv *env, // create StackTraceElement object jobject jframe = - bsg_safe_new_object(env, trace_class, trace_constructor, class, method, - filename, frame.line_number); + bsg_safe_new_object(env, bsg_jni_cache->StackTraceElement, + bsg_jni_cache->StackTraceElement_constructor, class, + method, filename, frame.line_number); if (jframe == NULL) { goto exit; } @@ -118,10 +129,8 @@ void bugsnag_notify_env(JNIEnv *env, const char *name, const char *message, jbyteArray jname = NULL; jbyteArray jmessage = NULL; - bsg_jni_cache jni_cache; - - if (!bsg_jni_cache_init(env, &jni_cache)) { - BUGSNAG_LOG("Could not refresh JNI cache."); + if (!bsg_jni_cache->initialized) { + BUGSNAG_LOG("bugsnag_notify_env failed: JNI cache not initialized."); goto exit; } @@ -132,24 +141,22 @@ void bugsnag_notify_env(JNIEnv *env, const char *name, const char *message, // create StackTraceElement array jtrace = bsg_safe_new_object_array(env, frame_count, - jni_cache.stack_trace_element); + bsg_jni_cache->StackTraceElement); if (jtrace == NULL) { goto exit; } // populate stacktrace object - populate_notify_stacktrace(env, stacktrace, frame_count, - jni_cache.stack_trace_element, - jni_cache.ste_constructor, jtrace); + populate_notify_stacktrace(env, stacktrace, frame_count, jtrace); // get the severity field - jfieldID severity_field = parse_jseverity(env, severity, jni_cache.severity); + jfieldID severity_field = parse_jseverity(env, severity); if (severity_field == NULL) { goto exit; } // get the error severity object - jseverity = - bsg_safe_get_static_object_field(env, jni_cache.severity, severity_field); + jseverity = bsg_safe_get_static_object_field(env, bsg_jni_cache->Severity, + severity_field); if (jseverity == NULL) { goto exit; } @@ -157,9 +164,9 @@ void bugsnag_notify_env(JNIEnv *env, const char *name, const char *message, jname = bsg_byte_ary_from_string(env, name); jmessage = bsg_byte_ary_from_string(env, message); - bsg_safe_call_static_void_method(env, jni_cache.native_interface, - jni_cache.ni_notify, jname, jmessage, - jseverity, jtrace); + bsg_safe_call_static_void_method(env, bsg_jni_cache->NativeInterface, + bsg_jni_cache->NativeInterface_notify, jname, + jmessage, jseverity, jtrace); goto exit; @@ -175,10 +182,8 @@ void bugsnag_notify_env(JNIEnv *env, const char *name, const char *message, void bugsnag_set_user_env(JNIEnv *env, const char *id, const char *email, const char *name) { - bsg_jni_cache jni_cache; - - if (!bsg_jni_cache_init(env, &jni_cache)) { - BUGSNAG_LOG("Could not refresh JNI cache."); + if (!bsg_jni_cache->initialized) { + BUGSNAG_LOG("bugsnag_set_user_env failed: JNI cache not initialized."); return; } @@ -186,8 +191,9 @@ void bugsnag_set_user_env(JNIEnv *env, const char *id, const char *email, jbyteArray jemail = bsg_byte_ary_from_string(env, email); jbyteArray jname = bsg_byte_ary_from_string(env, name); - bsg_safe_call_static_void_method(env, jni_cache.native_interface, - jni_cache.ni_set_user, jid, jemail, jname); + bsg_safe_call_static_void_method(env, bsg_jni_cache->NativeInterface, + bsg_jni_cache->NativeInterface_setUser, jid, + jemail, jname); bsg_safe_release_byte_array_elements(env, jid, (jbyte *)id); bsg_safe_delete_local_ref(env, jid); @@ -198,57 +204,66 @@ void bugsnag_set_user_env(JNIEnv *env, const char *id, const char *email, } static jfieldID parse_jcrumb_type(JNIEnv *env, - const bugsnag_breadcrumb_type type, - jclass type_class) { + const bugsnag_breadcrumb_type type) { + if (!bsg_jni_cache->initialized) { + return NULL; + } + const char *type_sig = "Lcom/bugsnag/android/BreadcrumbType;"; if (type == BSG_CRUMB_USER) { - return bsg_safe_get_static_field_id(env, type_class, "USER", type_sig); + return bsg_safe_get_static_field_id(env, bsg_jni_cache->BreadcrumbType, + "USER", type_sig); } else if (type == BSG_CRUMB_ERROR) { - return bsg_safe_get_static_field_id(env, type_class, "ERROR", type_sig); + return bsg_safe_get_static_field_id(env, bsg_jni_cache->BreadcrumbType, + "ERROR", type_sig); } else if (type == BSG_CRUMB_LOG) { - return bsg_safe_get_static_field_id(env, type_class, "LOG", type_sig); + return bsg_safe_get_static_field_id(env, bsg_jni_cache->BreadcrumbType, + "LOG", type_sig); } else if (type == BSG_CRUMB_NAVIGATION) { - return bsg_safe_get_static_field_id(env, type_class, "NAVIGATION", - type_sig); + return bsg_safe_get_static_field_id(env, bsg_jni_cache->BreadcrumbType, + "NAVIGATION", type_sig); } else if (type == BSG_CRUMB_PROCESS) { - return bsg_safe_get_static_field_id(env, type_class, "PROCESS", type_sig); + return bsg_safe_get_static_field_id(env, bsg_jni_cache->BreadcrumbType, + "PROCESS", type_sig); } else if (type == BSG_CRUMB_REQUEST) { - return bsg_safe_get_static_field_id(env, type_class, "REQUEST", type_sig); + return bsg_safe_get_static_field_id(env, bsg_jni_cache->BreadcrumbType, + "REQUEST", type_sig); } else if (type == BSG_CRUMB_STATE) { - return bsg_safe_get_static_field_id(env, type_class, "STATE", type_sig); + return bsg_safe_get_static_field_id(env, bsg_jni_cache->BreadcrumbType, + "STATE", type_sig); } else { // MANUAL is the default type - return bsg_safe_get_static_field_id(env, type_class, "MANUAL", type_sig); + return bsg_safe_get_static_field_id(env, bsg_jni_cache->BreadcrumbType, + "MANUAL", type_sig); } } void bugsnag_leave_breadcrumb_env(JNIEnv *env, const char *message, const bugsnag_breadcrumb_type type) { - bsg_jni_cache jni_cache; - jbyteArray jmessage = NULL; jobject jtype = NULL; - if (!bsg_jni_cache_init(env, &jni_cache)) { - BUGSNAG_LOG("Could not refresh JNI cache."); + if (!bsg_jni_cache->initialized) { + BUGSNAG_LOG( + "bugsnag_leave_breadcrumb_env failed: JNI cache not initialized."); goto exit; } // get breadcrumb type fieldID - jfieldID crumb_type = parse_jcrumb_type(env, type, jni_cache.breadcrumb_type); + jfieldID crumb_type = parse_jcrumb_type(env, type); if (crumb_type == NULL) { goto exit; } // get the breadcrumb type - jtype = bsg_safe_get_static_object_field(env, jni_cache.breadcrumb_type, + jtype = bsg_safe_get_static_object_field(env, bsg_jni_cache->BreadcrumbType, crumb_type); if (jtype == NULL) { goto exit; } jmessage = bsg_byte_ary_from_string(env, message); - bsg_safe_call_static_void_method(env, jni_cache.native_interface, - jni_cache.ni_leave_breadcrumb, jmessage, - jtype); + bsg_safe_call_static_void_method( + env, bsg_jni_cache->NativeInterface, + bsg_jni_cache->NativeInterface_leaveBreadcrumb, jmessage, jtype); goto exit; 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 c98e3a7640..5bb8a749f7 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c @@ -150,10 +150,8 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install( jboolean auto_detect_ndk_crashes, jint _api_level, jboolean is32bit, jint send_threads) { - bsg_jni_cache jni_cache; - - if (!bsg_jni_cache_init(env, &jni_cache)) { - BUGSNAG_LOG("Could not refresh JNI jni_cache."); + if (!bsg_jni_cache_init(env)) { + BUGSNAG_LOG("Could not init JNI jni_cache."); } bsg_environment *bugsnag_env = calloc(1, sizeof(bsg_environment)); @@ -191,7 +189,7 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install( } // populate metadata from Java layer - bsg_populate_event(env, &jni_cache, &bugsnag_env->next_event); + bsg_populate_event(env, &bugsnag_env->next_event); time(&bugsnag_env->start_time); if (bugsnag_env->next_event.app.in_foreground) { bugsnag_env->foreground_start_time = bugsnag_env->start_time; @@ -226,8 +224,6 @@ Java_com_bugsnag_android_ndk_NativeBridge_deliverReportAtPath( static pthread_mutex_t bsg_native_delivery_mutex = PTHREAD_MUTEX_INITIALIZER; pthread_mutex_lock(&bsg_native_delivery_mutex); - bsg_jni_cache jni_cache; - const char *event_path = NULL; bugsnag_event *event = NULL; jbyteArray jpayload = NULL; @@ -235,8 +231,8 @@ Java_com_bugsnag_android_ndk_NativeBridge_deliverReportAtPath( char *payload = NULL; jstring japi_key = NULL; - if (!bsg_jni_cache_init(env, &jni_cache)) { - BUGSNAG_LOG("Could not refresh JNI cache."); + if (!bsg_jni_cache->initialized) { + BUGSNAG_LOG("deliverReportAtPath failed: JNI cache not initialized."); goto exit; } @@ -277,9 +273,10 @@ Java_com_bugsnag_android_ndk_NativeBridge_deliverReportAtPath( japi_key = bsg_safe_new_string_utf(env, event->api_key); if (japi_key != NULL) { bool is_launching = event->app.is_launching; - bsg_safe_call_static_void_method(env, jni_cache.native_interface, - jni_cache.ni_deliver_report, jstage, - jpayload, japi_key, is_launching); + bsg_safe_call_static_void_method( + env, bsg_jni_cache->NativeInterface, + bsg_jni_cache->NativeInterface_deliverReport, jstage, jpayload, + japi_key, is_launching); } exit: @@ -362,10 +359,8 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addBreadcrumb( JNIEnv *env, jobject _this, jstring name_, jstring crumb_type, jstring timestamp_, jobject metadata) { - bsg_jni_cache jni_cache; - - if (!bsg_jni_cache_init(env, &jni_cache)) { - BUGSNAG_LOG("Could not refresh JNI cache."); + if (!bsg_jni_cache->initialized) { + BUGSNAG_LOG("addBreadcrumb failed: JNI cache not initialized."); return; } const char *name = bsg_safe_get_string_utf_chars(env, name_); @@ -394,7 +389,7 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_addBreadcrumb( crumb->type = BSG_CRUMB_MANUAL; } - bsg_populate_crumb_metadata(env, &jni_cache, crumb, metadata); + bsg_populate_crumb_metadata(env, crumb, metadata); request_env_write_lock(); bugsnag_event_add_breadcrumb(&bsg_global_env->next_event, crumb); release_env_write_lock(); @@ -717,14 +712,12 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_updateMetadata( return; } - bsg_jni_cache jni_cache; - if (!bsg_jni_cache_init(env, &jni_cache)) { - BUGSNAG_LOG("Could not refresh JNI cache."); + if (!bsg_jni_cache->initialized) { + BUGSNAG_LOG("updateMetadata failed: JNI cache not initialized."); return; } request_env_write_lock(); - bsg_populate_metadata(env, &jni_cache, &bsg_global_env->next_event.metadata, - metadata); + bsg_populate_metadata(env, &bsg_global_env->next_event.metadata, metadata); release_env_write_lock(); } diff --git a/bugsnag-plugin-android-ndk/src/main/jni/jni_cache.c b/bugsnag-plugin-android-ndk/src/main/jni/jni_cache.c index bf47221a34..99c7a26772 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/jni_cache.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/jni_cache.c @@ -4,275 +4,164 @@ #include "jni_cache.h" #include "safejni.h" -#include +#include "utils/logger.h" +#include -#include -#ifndef BUGSNAG_LOG -#define BUGSNAG_LOG(fmt, ...) \ - __android_log_print(ANDROID_LOG_WARN, "BugsnagNDK", fmt, ##__VA_ARGS__) -#endif +#define JNI_VERSION JNI_VERSION_1_6 -#define report_contents(VALUE, TYPECODE) \ - BUGSNAG_LOG(#VALUE " == " TYPECODE, VALUE) +static bsg_jni_cache_t jni_cache; +bsg_jni_cache_t *const bsg_jni_cache = &jni_cache; -bool bsg_jni_cache_init(JNIEnv *env, bsg_jni_cache *cache) { - if (cache == NULL) { - return false; - } - - // All objects here are local references, and MUST be refreshed on every - // transition from Java to native. - - // Classes - - cache->integer = bsg_safe_find_class(env, "java/lang/Integer"); - if (cache->integer == NULL) { - report_contents(cache->integer, "%p"); - goto failed; - } - - cache->boolean = bsg_safe_find_class(env, "java/lang/Boolean"); - if (cache->boolean == NULL) { - report_contents(cache->boolean, "%p"); - goto failed; - } - - cache->long_class = bsg_safe_find_class(env, "java/lang/Long"); - if (cache->long_class == NULL) { - report_contents(cache->long_class, "%p"); - goto failed; - } - - cache->float_class = bsg_safe_find_class(env, "java/lang/Float"); - if (cache->float_class == NULL) { - report_contents(cache->float_class, "%p"); - goto failed; - } - - cache->number = bsg_safe_find_class(env, "java/lang/Number"); - if (cache->number == NULL) { - report_contents(cache->number, "%p"); - goto failed; - } - - cache->string = bsg_safe_find_class(env, "java/lang/String"); - if (cache->string == NULL) { - report_contents(cache->string, "%p"); - goto failed; - } - - // Methods - - cache->arraylist = bsg_safe_find_class(env, "java/util/ArrayList"); - if (cache->arraylist == NULL) { - report_contents(cache->arraylist, "%p"); - goto failed; - } - - cache->hash_map = bsg_safe_find_class(env, "java/util/HashMap"); - if (cache->hash_map == NULL) { - report_contents(cache->hash_map, "%p"); - goto failed; - } - - cache->map = bsg_safe_find_class(env, "java/util/Map"); - if (cache->map == NULL) { - report_contents(cache->map, "%p"); - goto failed; - } - - cache->native_interface = - bsg_safe_find_class(env, "com/bugsnag/android/NativeInterface"); - if (cache->native_interface == NULL) { - report_contents(cache->native_interface, "%p"); - goto failed; - } +static pthread_key_t jni_cleanup_key; - cache->stack_trace_element = - bsg_safe_find_class(env, "java/lang/StackTraceElement"); - if (cache->stack_trace_element == NULL) { - report_contents(cache->stack_trace_element, "%p"); - goto failed; - } - - cache->severity = bsg_safe_find_class(env, "com/bugsnag/android/Severity"); - if (cache->severity == NULL) { - report_contents(cache->severity, "%p"); - goto failed; - } - - cache->breadcrumb_type = - bsg_safe_find_class(env, "com/bugsnag/android/BreadcrumbType"); - if (cache->breadcrumb_type == NULL) { - report_contents(cache->breadcrumb_type, "%p"); - goto failed; - } - - cache->integer_int_value = - bsg_safe_get_method_id(env, cache->integer, "intValue", "()I"); - if (cache->integer_int_value == NULL) { - report_contents(cache->integer_int_value, "%p"); - goto failed; - } - - cache->float_float_value = - bsg_safe_get_method_id(env, cache->float_class, "floatValue", "()F"); - if (cache->float_float_value == NULL) { - report_contents(cache->float_float_value, "%p"); - goto failed; - } - - cache->number_double_value = - bsg_safe_get_method_id(env, cache->number, "doubleValue", "()D"); - if (cache->number_double_value == NULL) { - report_contents(cache->number_double_value, "%p"); - goto failed; - } - - cache->long_long_value = - bsg_safe_get_method_id(env, cache->integer, "longValue", "()J"); - if (cache->long_long_value == NULL) { - report_contents(cache->long_long_value, "%p"); - goto failed; - } - - cache->boolean_bool_value = - bsg_safe_get_method_id(env, cache->boolean, "booleanValue", "()Z"); - if (cache->boolean_bool_value == NULL) { - report_contents(cache->boolean_bool_value, "%p"); - goto failed; - } - - cache->arraylist_init_with_obj = bsg_safe_get_method_id( - env, cache->arraylist, "", "(Ljava/util/Collection;)V"); - if (cache->arraylist_init_with_obj == NULL) { - report_contents(cache->arraylist_init_with_obj, "%p"); - goto failed; - } - - cache->arraylist_get = bsg_safe_get_method_id(env, cache->arraylist, "get", - "(I)Ljava/lang/Object;"); - if (cache->arraylist_get == NULL) { - report_contents(cache->arraylist_get, "%p"); - goto failed; - } - - cache->hash_map_key_set = bsg_safe_get_method_id( - env, cache->hash_map, "keySet", "()Ljava/util/Set;"); - if (cache->hash_map_key_set == NULL) { - report_contents(cache->hash_map_key_set, "%p"); - goto failed; - } - - cache->hash_map_size = - bsg_safe_get_method_id(env, cache->hash_map, "size", "()I"); - if (cache->hash_map_size == NULL) { - report_contents(cache->hash_map_size, "%p"); - goto failed; - } - - cache->hash_map_get = bsg_safe_get_method_id( - env, cache->hash_map, "get", "(Ljava/lang/Object;)Ljava/lang/Object;"); - if (cache->hash_map_get == NULL) { - report_contents(cache->hash_map_get, "%p"); - goto failed; - } - - cache->map_key_set = - bsg_safe_get_method_id(env, cache->map, "keySet", "()Ljava/util/Set;"); - if (cache->map_key_set == NULL) { - report_contents(cache->map_key_set, "%p"); - goto failed; - } - - cache->map_size = bsg_safe_get_method_id(env, cache->map, "size", "()I"); - if (cache->map_size == NULL) { - report_contents(cache->map_size, "%p"); - goto failed; - } - - cache->map_get = bsg_safe_get_method_id( - env, cache->map, "get", "(Ljava/lang/Object;)Ljava/lang/Object;"); - if (cache->map_get == NULL) { - report_contents(cache->map_get, "%p"); - goto failed; - } - - cache->ni_get_app = bsg_safe_get_static_method_id( - env, cache->native_interface, "getApp", "()Ljava/util/Map;"); - if (cache->ni_get_app == NULL) { - report_contents(cache->ni_get_app, "%p"); - goto failed; - } - - cache->ni_get_device = bsg_safe_get_static_method_id( - env, cache->native_interface, "getDevice", "()Ljava/util/Map;"); - if (cache->ni_get_device == NULL) { - report_contents(cache->ni_get_device, "%p"); - goto failed; - } - - cache->ni_get_user = bsg_safe_get_static_method_id( - env, cache->native_interface, "getUser", "()Ljava/util/Map;"); - if (cache->ni_get_user == NULL) { - report_contents(cache->ni_get_user, "%p"); - goto failed; - } - - cache->ni_set_user = bsg_safe_get_static_method_id( - env, cache->native_interface, "setUser", "([B[B[B)V"); - if (cache->ni_set_user == NULL) { - report_contents(cache->ni_set_user, "%p"); - goto failed; - } - - cache->ni_get_metadata = bsg_safe_get_static_method_id( - env, cache->native_interface, "getMetadata", "()Ljava/util/Map;"); - if (cache->ni_get_metadata == NULL) { - report_contents(cache->ni_get_metadata, "%p"); - goto failed; +static void detach_java_env(void *env) { + if (bsg_jni_cache->initialized && env != NULL) { + (*bsg_jni_cache->jvm)->DetachCurrentThread(bsg_jni_cache->jvm); } +} - // lookup NativeInterface.getContext() - cache->ni_get_context = bsg_safe_get_static_method_id( - env, cache->native_interface, "getContext", "()Ljava/lang/String;"); - if (cache->ni_get_context == NULL) { - report_contents(cache->ni_get_context, "%p"); - goto failed; +JNIEnv *bsg_jni_cache_get_env() { + if (!bsg_jni_cache->initialized) { + return NULL; + } + + JNIEnv *env = NULL; + switch ((*bsg_jni_cache->jvm) + ->GetEnv(bsg_jni_cache->jvm, (void **)&env, JNI_VERSION)) { + case JNI_OK: + return env; + case JNI_EDETACHED: + if ((*bsg_jni_cache->jvm) + ->AttachCurrentThread(bsg_jni_cache->jvm, &env, NULL) != JNI_OK) { + BUGSNAG_LOG("Could not attach thread to JVM"); + return NULL; + } + if (env == NULL) { + BUGSNAG_LOG("AttachCurrentThread filled a NULL JNIEnv"); + return NULL; + } + + // attach a destructor to detach the env before the thread terminates + pthread_setspecific(jni_cleanup_key, env); + + return env; + default: + BUGSNAG_LOG("Could not get JNIEnv"); + return NULL; } +} - cache->ni_notify = bsg_safe_get_static_method_id( - env, cache->native_interface, "notify", +// All classes must be cached as global refs +#define CACHE_CLASS(CLASS, PATH) \ + do { \ + jclass cls = bsg_safe_find_class(env, PATH); \ + if (cls == NULL) { \ + BUGSNAG_LOG("JNI Cache Init Error: JNI class ref " #CLASS \ + " (%s) is NULL", \ + PATH); \ + goto failed; \ + } \ + bsg_jni_cache->CLASS = (*env)->NewGlobalRef(env, cls); \ + } while (0) + +// Methods are IDs, which remain valid as long as their class ref remains valid. +#define CACHE_METHOD(CLASS, METHOD, NAME, PARAMS) \ + do { \ + jmethodID mtd = \ + bsg_safe_get_method_id(env, bsg_jni_cache->CLASS, NAME, PARAMS); \ + if (mtd == NULL) { \ + BUGSNAG_LOG("JNI Cache Init Error: JNI method ref " #CLASS "." #METHOD \ + " (%s%s) is NULL", \ + NAME, PARAMS); \ + goto failed; \ + } \ + bsg_jni_cache->METHOD = mtd; \ + } while (0) + +#define CACHE_STATIC_METHOD(CLASS, METHOD, NAME, PARAMS) \ + do { \ + jmethodID mtd = bsg_safe_get_static_method_id(env, bsg_jni_cache->CLASS, \ + NAME, PARAMS); \ + if (mtd == NULL) { \ + BUGSNAG_LOG("JNI Cache Init Error: JNI method ref " #CLASS "." #METHOD \ + " (%s%s) is NULL", \ + NAME, PARAMS); \ + goto failed; \ + } \ + bsg_jni_cache->METHOD = mtd; \ + } while (0) + +bool bsg_jni_cache_init(JNIEnv *env) { + if (bsg_jni_cache->initialized) { + return true; + } + + (*env)->GetJavaVM(env, &bsg_jni_cache->jvm); + if (bsg_jni_cache->jvm == NULL) { + BUGSNAG_LOG("JNI Cache Init Error: Could not get global JavaVM"); + goto failed; + } + + CACHE_CLASS(Boolean, "java/lang/Boolean"); + CACHE_METHOD(Boolean, Boolean_booleanValue, "booleanValue", "()Z"); + + CACHE_CLASS(Float, "java/lang/Float"); + CACHE_METHOD(Float, Float_floatValue, "floatValue", "()F"); + + CACHE_CLASS(number, "java/lang/Number"); + CACHE_METHOD(number, number_double_value, "doubleValue", "()D"); + + CACHE_CLASS(String, "java/lang/String"); + + CACHE_CLASS(ArrayList, "java/util/ArrayList"); + CACHE_METHOD(ArrayList, ArrayList_constructor, "", + "(Ljava/util/Collection;)V"); + CACHE_METHOD(ArrayList, ArrayList_get, "get", "(I)Ljava/lang/Object;"); + + CACHE_CLASS(Map, "java/util/Map"); + CACHE_METHOD(Map, Map_keySet, "keySet", "()Ljava/util/Set;"); + CACHE_METHOD(Map, Map_size, "size", "()I"); + CACHE_METHOD(Map, Map_get, "get", "(Ljava/lang/Object;)Ljava/lang/Object;"); + + CACHE_CLASS(HashMap, "java/util/HashMap"); + CACHE_METHOD(HashMap, HashMap_keySet, "keySet", "()Ljava/util/Set;"); + CACHE_METHOD(HashMap, HashMap_size, "size", "()I"); + CACHE_METHOD(HashMap, HashMap_get, "get", + "(Ljava/lang/Object;)Ljava/lang/Object;"); + + CACHE_CLASS(NativeInterface, "com/bugsnag/android/NativeInterface"); + CACHE_STATIC_METHOD(NativeInterface, NativeInterface_getApp, "getApp", + "()Ljava/util/Map;"); + CACHE_STATIC_METHOD(NativeInterface, NativeInterface_getDevice, "getDevice", + "()Ljava/util/Map;"); + CACHE_STATIC_METHOD(NativeInterface, NativeInterface_getUser, "getUser", + "()Ljava/util/Map;"); + CACHE_STATIC_METHOD(NativeInterface, NativeInterface_setUser, "setUser", + "([B[B[B)V"); + CACHE_STATIC_METHOD(NativeInterface, NativeInterface_getMetadata, + "getMetadata", "()Ljava/util/Map;"); + CACHE_STATIC_METHOD(NativeInterface, NativeInterface_getContext, "getContext", + "()Ljava/lang/String;"); + CACHE_STATIC_METHOD( + NativeInterface, NativeInterface_notify, "notify", "([B[BLcom/bugsnag/android/Severity;[Ljava/lang/StackTraceElement;)V"); - if (cache->ni_notify == NULL) { - report_contents(cache->ni_notify, "%p"); - goto failed; - } + CACHE_STATIC_METHOD(NativeInterface, NativeInterface_deliverReport, + "deliverReport", "([B[BLjava/lang/String;Z)V"); + CACHE_STATIC_METHOD(NativeInterface, NativeInterface_leaveBreadcrumb, + "leaveBreadcrumb", + "([BLcom/bugsnag/android/BreadcrumbType;)V"); - cache->ni_deliver_report = bsg_safe_get_static_method_id( - env, cache->native_interface, "deliverReport", - "([B[BLjava/lang/String;Z)V"); - if (cache->ni_deliver_report == NULL) { - report_contents(cache->ni_deliver_report, "%p"); - goto failed; - } + CACHE_CLASS(StackTraceElement, "java/lang/StackTraceElement"); + CACHE_METHOD(StackTraceElement, StackTraceElement_constructor, "", + "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;I)V"); - cache->ni_leave_breadcrumb = bsg_safe_get_static_method_id( - env, cache->native_interface, "leaveBreadcrumb", - "([BLcom/bugsnag/android/BreadcrumbType;)V"); - if (cache->ni_leave_breadcrumb == NULL) { - report_contents(cache->ni_leave_breadcrumb, "%p"); - goto failed; - } + CACHE_CLASS(Severity, "com/bugsnag/android/Severity"); - cache->ste_constructor = bsg_safe_get_method_id( - env, cache->stack_trace_element, "", - "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;I)V"); - if (cache->ste_constructor == NULL) { - report_contents(cache->ste_constructor, "%p"); - goto failed; - } + CACHE_CLASS(BreadcrumbType, "com/bugsnag/android/BreadcrumbType"); + + pthread_key_create(&jni_cleanup_key, detach_java_env); + bsg_jni_cache->initialized = true; return true; failed: diff --git a/bugsnag-plugin-android-ndk/src/main/jni/jni_cache.h b/bugsnag-plugin-android-ndk/src/main/jni/jni_cache.h index c72e0b3059..410d7f7d60 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/jni_cache.h +++ b/bugsnag-plugin-android-ndk/src/main/jni/jni_cache.h @@ -13,55 +13,71 @@ extern "C" { #endif typedef struct { - jclass hash_map; - jclass map; - jclass arraylist; - jclass integer; - jclass boolean; - jclass metadata; - jclass native_interface; - jclass long_class; - jclass float_class; + bool initialized; + + JavaVM *jvm; + + jclass Boolean; + jmethodID Boolean_booleanValue; + + jclass Float; + jmethodID Float_floatValue; + jclass number; - jclass string; - jclass stack_trace_element; - jclass severity; - jclass breadcrumb_type; - jmethodID integer_int_value; - jmethodID long_long_value; - jmethodID float_float_value; - jmethodID boolean_bool_value; jmethodID number_double_value; - jmethodID hash_map_get; - jmethodID hash_map_size; - jmethodID hash_map_key_set; - jmethodID map_get; - jmethodID map_size; - jmethodID map_key_set; - jmethodID arraylist_init_with_obj; - jmethodID arraylist_get; - jmethodID ni_get_app; - jmethodID ni_get_device; - jmethodID ni_get_user; - jmethodID ni_set_user; - jmethodID ni_get_metadata; - jmethodID ni_get_context; - jmethodID ni_notify; - jmethodID ni_leave_breadcrumb; - jmethodID ni_deliver_report; - jmethodID ste_constructor; -} bsg_jni_cache; + + jclass String; + + jclass Map; + jmethodID Map_get; + jmethodID Map_size; + jmethodID Map_keySet; + + jclass HashMap; + jmethodID HashMap_get; + jmethodID HashMap_size; + jmethodID HashMap_keySet; + + jclass ArrayList; + jmethodID ArrayList_constructor; + jmethodID ArrayList_get; + + jclass NativeInterface; + jmethodID NativeInterface_getApp; + jmethodID NativeInterface_getDevice; + jmethodID NativeInterface_getUser; + jmethodID NativeInterface_setUser; + jmethodID NativeInterface_getMetadata; + jmethodID NativeInterface_getContext; + jmethodID NativeInterface_notify; + jmethodID NativeInterface_leaveBreadcrumb; + jmethodID NativeInterface_deliverReport; + + jclass StackTraceElement; + jmethodID StackTraceElement_constructor; + + jclass Severity; + + jclass BreadcrumbType; +} bsg_jni_cache_t; + +extern bsg_jni_cache_t *const bsg_jni_cache; /** * Populate all references in the JNI cache. - * This MUST be called on every Java-to-native call to ensure that references - * remain bound to the correct JNIEnv. * * @param env The JNI env - * @param cache The cache to refresh * @return false if an error occurs, in which case the cache is unusable. */ -bool bsg_jni_cache_init(JNIEnv *env, bsg_jni_cache *cache); +bool bsg_jni_cache_init(JNIEnv *env); + +/** + * Get the current JNI environment, attaching if necessary. + * The environment will be detached automatically on thread termination. + * @return The current JNI environment, or NULL in case of JNI error (which will + * be logged). + */ +JNIEnv *bsg_jni_cache_get_env(); #ifdef __cplusplus } diff --git a/bugsnag-plugin-android-ndk/src/main/jni/metadata.c b/bugsnag-plugin-android-ndk/src/main/jni/metadata.c index de90a3ad94..5894703753 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/metadata.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/metadata.c @@ -5,12 +5,11 @@ #include #include -static jobject get_map_value_obj(JNIEnv *env, bsg_jni_cache *jni_cache, - jobject map, const char *_key) { +static jobject get_map_value_obj(JNIEnv *env, jobject map, const char *_key) { jobject obj = NULL; jstring key = NULL; - if (jni_cache == NULL) { + if (!bsg_jni_cache->initialized) { goto exit; } @@ -19,17 +18,16 @@ static jobject get_map_value_obj(JNIEnv *env, bsg_jni_cache *jni_cache, goto exit; } - obj = bsg_safe_call_object_method(env, map, jni_cache->hash_map_get, key); + obj = bsg_safe_call_object_method(env, map, bsg_jni_cache->HashMap_get, key); exit: bsg_safe_delete_local_ref(env, key); return obj; } -static void copy_map_value_string(JNIEnv *env, bsg_jni_cache *jni_cache, - jobject map, const char *_key, char *dest, - int len) { - jobject _value = get_map_value_obj(env, jni_cache, map, _key); +static void copy_map_value_string(JNIEnv *env, jobject map, const char *_key, + char *dest, int len) { + jobject _value = get_map_value_obj(env, map, _key); if (_value == NULL) { return; @@ -45,78 +43,76 @@ static void copy_map_value_string(JNIEnv *env, bsg_jni_cache *jni_cache, bsg_safe_delete_local_ref(env, _value); } -static long get_map_value_long(JNIEnv *env, bsg_jni_cache *jni_cache, - jobject map, const char *_key) { +static long get_map_value_long(JNIEnv *env, jobject map, const char *_key) { jobject _value = NULL; long value = 0; - if (jni_cache == NULL) { + if (!bsg_jni_cache->initialized) { goto exit; } - _value = get_map_value_obj(env, jni_cache, map, _key); + _value = get_map_value_obj(env, map, _key); if (_value == NULL) { goto exit; } - value = - bsg_safe_call_double_method(env, _value, jni_cache->number_double_value); + value = bsg_safe_call_double_method(env, _value, + bsg_jni_cache->number_double_value); exit: bsg_safe_delete_local_ref(env, _value); return value; } -static float get_map_value_float(JNIEnv *env, bsg_jni_cache *jni_cache, - jobject map, const char *_key) { +static float get_map_value_float(JNIEnv *env, jobject map, const char *_key) { jobject _value = NULL; float value = 0; - if (jni_cache == NULL) { + if (!bsg_jni_cache->initialized) { goto exit; } - _value = get_map_value_obj(env, jni_cache, map, _key); + _value = get_map_value_obj(env, map, _key); if (_value == NULL) { goto exit; } - value = bsg_safe_call_float_method(env, _value, jni_cache->float_float_value); + value = + bsg_safe_call_float_method(env, _value, bsg_jni_cache->Float_floatValue); exit: bsg_safe_delete_local_ref(env, _value); return value; } -static bool get_map_value_bool(JNIEnv *env, bsg_jni_cache *jni_cache, - jobject map, const char *_key) { +static bool get_map_value_bool(JNIEnv *env, jobject map, const char *_key) { jobject _value = NULL; bool value = 0; - if (jni_cache == NULL) { + if (!bsg_jni_cache->initialized) { goto exit; } - _value = get_map_value_obj(env, jni_cache, map, _key); + _value = get_map_value_obj(env, map, _key); if (_value == NULL) { goto exit; } - value = - bsg_safe_call_boolean_method(env, _value, jni_cache->boolean_bool_value); + value = bsg_safe_call_boolean_method(env, _value, + bsg_jni_cache->Boolean_booleanValue); exit: bsg_safe_delete_local_ref(env, _value); return value; } -static int populate_cpu_abi_from_map(JNIEnv *env, bsg_jni_cache *jni_cache, - jobject map, bsg_device_info *device) { +static int populate_cpu_abi_from_map(JNIEnv *env, jobject map, + bsg_device_info *device) { jstring key = NULL; jobjectArray _value = NULL; int count = 0; - if (jni_cache == NULL) { + if (!bsg_jni_cache->initialized) { goto exit; } @@ -125,7 +121,8 @@ static int populate_cpu_abi_from_map(JNIEnv *env, bsg_jni_cache *jni_cache, goto exit; } - _value = bsg_safe_call_object_method(env, map, jni_cache->hash_map_get, key); + _value = + bsg_safe_call_object_method(env, map, bsg_jni_cache->HashMap_get, key); if (_value == NULL) { goto exit; } @@ -155,49 +152,43 @@ static int populate_cpu_abi_from_map(JNIEnv *env, bsg_jni_cache *jni_cache, return count; } -static void populate_app_data(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_event *event) { - if (jni_cache == NULL) { +static void populate_app_data(JNIEnv *env, bugsnag_event *event) { + if (!bsg_jni_cache->initialized) { return; } - jobject data = bsg_safe_call_static_object_method( - env, jni_cache->native_interface, jni_cache->ni_get_app); + jobject data = + bsg_safe_call_static_object_method(env, bsg_jni_cache->NativeInterface, + bsg_jni_cache->NativeInterface_getApp); if (data == NULL) { return; } - copy_map_value_string(env, jni_cache, data, "binaryArch", - event->app.binary_arch, sizeof(event->app.binary_arch)); - copy_map_value_string(env, jni_cache, data, "buildUUID", - event->app.build_uuid, sizeof(event->app.build_uuid)); - event->app.duration_ms_offset = - get_map_value_long(env, jni_cache, data, "duration"); + copy_map_value_string(env, data, "binaryArch", event->app.binary_arch, + sizeof(event->app.binary_arch)); + copy_map_value_string(env, data, "buildUUID", event->app.build_uuid, + sizeof(event->app.build_uuid)); + event->app.duration_ms_offset = get_map_value_long(env, data, "duration"); event->app.duration_in_foreground_ms_offset = - get_map_value_long(env, jni_cache, data, "durationInForeground"); + get_map_value_long(env, data, "durationInForeground"); - copy_map_value_string(env, jni_cache, data, "id", event->app.id, - sizeof(event->app.id)); - event->app.in_foreground = - get_map_value_bool(env, jni_cache, data, "inForeground"); + copy_map_value_string(env, data, "id", event->app.id, sizeof(event->app.id)); + event->app.in_foreground = get_map_value_bool(env, data, "inForeground"); event->app.is_launching = true; char name[64]; - copy_map_value_string(env, jni_cache, data, "name", name, sizeof(name)); + copy_map_value_string(env, data, "name", name, sizeof(name)); bugsnag_event_add_metadata_string(event, "app", "name", name); - copy_map_value_string(env, jni_cache, data, "releaseStage", - event->app.release_stage, + copy_map_value_string(env, data, "releaseStage", event->app.release_stage, sizeof(event->app.release_stage)); - copy_map_value_string(env, jni_cache, data, "type", event->app.type, + copy_map_value_string(env, data, "type", event->app.type, sizeof(event->app.type)); - copy_map_value_string(env, jni_cache, data, "version", event->app.version, + copy_map_value_string(env, data, "version", event->app.version, sizeof(event->app.version)); - event->app.version_code = - get_map_value_long(env, jni_cache, data, "versionCode"); + event->app.version_code = get_map_value_long(env, data, "versionCode"); - bool restricted = - get_map_value_bool(env, jni_cache, data, "backgroundWorkRestricted"); + bool restricted = get_map_value_bool(env, data, "backgroundWorkRestricted"); if (restricted) { bugsnag_event_add_metadata_bool(event, "app", "backgroundWorkRestricted", @@ -205,140 +196,133 @@ static void populate_app_data(JNIEnv *env, bsg_jni_cache *jni_cache, } char process_name[64]; - copy_map_value_string(env, jni_cache, data, "processName", process_name, + copy_map_value_string(env, data, "processName", process_name, sizeof(process_name)); bugsnag_event_add_metadata_string(event, "app", "processName", process_name); - long total_memory = get_map_value_long(env, jni_cache, data, "memoryLimit"); + long total_memory = get_map_value_long(env, data, "memoryLimit"); bugsnag_event_add_metadata_double(event, "app", "memoryLimit", (double)total_memory); bsg_safe_delete_local_ref(env, data); } -static void populate_device_metadata(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_event *event, void *data) { +static void populate_device_metadata(JNIEnv *env, bugsnag_event *event, + void *data) { char brand[64]; - copy_map_value_string(env, jni_cache, data, "brand", brand, sizeof(brand)); + copy_map_value_string(env, data, "brand", brand, sizeof(brand)); bugsnag_event_add_metadata_string(event, "device", "brand", brand); - bugsnag_event_add_metadata_double( - event, "device", "dpi", get_map_value_long(env, jni_cache, data, "dpi")); - bugsnag_event_add_metadata_bool( - event, "device", "emulator", - get_map_value_bool(env, jni_cache, data, "emulator")); + bugsnag_event_add_metadata_double(event, "device", "dpi", + get_map_value_long(env, data, "dpi")); + bugsnag_event_add_metadata_bool(event, "device", "emulator", + get_map_value_bool(env, data, "emulator")); char location_status[32]; - copy_map_value_string(env, jni_cache, data, "locationStatus", location_status, + copy_map_value_string(env, data, "locationStatus", location_status, sizeof(location_status)); bugsnag_event_add_metadata_string(event, "device", "locationStatus", location_status); char network_access[64]; - copy_map_value_string(env, jni_cache, data, "networkAccess", network_access, + copy_map_value_string(env, data, "networkAccess", network_access, sizeof(network_access)); bugsnag_event_add_metadata_string(event, "device", "networkAccess", network_access); bugsnag_event_add_metadata_double( event, "device", "screenDensity", - get_map_value_float(env, jni_cache, data, "screenDensity")); + get_map_value_float(env, data, "screenDensity")); char screen_resolution[32]; - copy_map_value_string(env, jni_cache, data, "screenResolution", - screen_resolution, sizeof(screen_resolution)); + copy_map_value_string(env, data, "screenResolution", screen_resolution, + sizeof(screen_resolution)); bugsnag_event_add_metadata_string(event, "device", "screenResolution", screen_resolution); } -static void populate_device_data(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_event *event) { - if (jni_cache == NULL) { +static void populate_device_data(JNIEnv *env, bugsnag_event *event) { + if (!bsg_jni_cache->initialized) { return; } jobject data = bsg_safe_call_static_object_method( - env, jni_cache->native_interface, jni_cache->ni_get_device); + env, bsg_jni_cache->NativeInterface, + bsg_jni_cache->NativeInterface_getDevice); if (data == NULL) { return; } - populate_cpu_abi_from_map(env, jni_cache, data, &event->device); + populate_cpu_abi_from_map(env, data, &event->device); - copy_map_value_string(env, jni_cache, data, "id", event->device.id, + copy_map_value_string(env, data, "id", event->device.id, sizeof(event->device.id)); - event->device.jailbroken = - get_map_value_bool(env, jni_cache, data, "jailbroken"); + event->device.jailbroken = get_map_value_bool(env, data, "jailbroken"); - copy_map_value_string(env, jni_cache, data, "locale", event->device.locale, + copy_map_value_string(env, data, "locale", event->device.locale, sizeof(event->device.locale)); - copy_map_value_string(env, jni_cache, data, "manufacturer", - event->device.manufacturer, + copy_map_value_string(env, data, "manufacturer", event->device.manufacturer, sizeof(event->device.manufacturer)); - copy_map_value_string(env, jni_cache, data, "model", event->device.model, + copy_map_value_string(env, data, "model", event->device.model, sizeof(event->device.model)); - copy_map_value_string(env, jni_cache, data, "orientation", - event->device.orientation, + copy_map_value_string(env, data, "orientation", event->device.orientation, sizeof(event->device.orientation)); bsg_strncpy(event->device.os_name, bsg_os_name(), sizeof(event->device.os_name)); - copy_map_value_string(env, jni_cache, data, "osVersion", - event->device.os_version, + copy_map_value_string(env, data, "osVersion", event->device.os_version, sizeof(event->device.os_version)); - jobject _runtime_versions = - get_map_value_obj(env, jni_cache, data, "runtimeVersions"); + jobject _runtime_versions = get_map_value_obj(env, data, "runtimeVersions"); if (_runtime_versions != NULL) { - copy_map_value_string(env, jni_cache, _runtime_versions, "osBuild", + copy_map_value_string(env, _runtime_versions, "osBuild", event->device.os_build, sizeof(event->device.os_build)); char api_level[8]; - copy_map_value_string(env, jni_cache, _runtime_versions, "androidApiLevel", - api_level, sizeof(api_level)); + copy_map_value_string(env, _runtime_versions, "androidApiLevel", api_level, + sizeof(api_level)); event->device.api_level = strtol(api_level, NULL, 10); } - event->device.total_memory = - get_map_value_long(env, jni_cache, data, "totalMemory"); + event->device.total_memory = get_map_value_long(env, data, "totalMemory"); // add fields to device metadata - populate_device_metadata(env, jni_cache, event, data); + populate_device_metadata(env, event, data); bsg_safe_delete_local_ref(env, data); bsg_safe_delete_local_ref(env, _runtime_versions); } -static void populate_user_data(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_event *event) { - if (jni_cache == NULL) { +static void populate_user_data(JNIEnv *env, bugsnag_event *event) { + if (!bsg_jni_cache->initialized) { return; } jobject data = bsg_safe_call_static_object_method( - env, jni_cache->native_interface, jni_cache->ni_get_user); + env, bsg_jni_cache->NativeInterface, + bsg_jni_cache->NativeInterface_getUser); if (data == NULL) { return; } - copy_map_value_string(env, jni_cache, data, "id", event->user.id, + copy_map_value_string(env, data, "id", event->user.id, sizeof(event->user.id)); - copy_map_value_string(env, jni_cache, data, "name", event->user.name, + copy_map_value_string(env, data, "name", event->user.name, sizeof(event->user.name)); - copy_map_value_string(env, jni_cache, data, "email", event->user.email, + copy_map_value_string(env, data, "email", event->user.email, sizeof(event->user.email)); bsg_safe_delete_local_ref(env, data); } -static void populate_context(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_event *event) { - if (jni_cache == NULL) { +static void populate_context(JNIEnv *env, bugsnag_event *event) { + if (!bsg_jni_cache->initialized) { return; } jstring _context = bsg_safe_call_static_object_method( - env, jni_cache->native_interface, jni_cache->ni_get_context); + env, bsg_jni_cache->NativeInterface, + bsg_jni_cache->NativeInterface_getContext); if (_context != NULL) { const char *value = bsg_safe_get_string_utf_chars(env, (jstring)_context); if (value != NULL) { @@ -352,24 +336,24 @@ static void populate_context(JNIEnv *env, bsg_jni_cache *jni_cache, bsg_safe_delete_local_ref(env, _context); } -static void populate_metadata_value(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_metadata *dst, const char *section, - const char *name, jobject _value) { - if (jni_cache == NULL) { +static void populate_metadata_value(JNIEnv *env, bugsnag_metadata *dst, + const char *section, const char *name, + jobject _value) { + if (!bsg_jni_cache->initialized) { return; } - if (bsg_safe_is_instance_of(env, _value, jni_cache->number)) { + if (bsg_safe_is_instance_of(env, _value, bsg_jni_cache->number)) { // add a double metadata value - double value = bsg_safe_call_double_method(env, _value, - jni_cache->number_double_value); + double value = bsg_safe_call_double_method( + env, _value, bsg_jni_cache->number_double_value); bsg_add_metadata_value_double(dst, section, name, value); - } else if (bsg_safe_is_instance_of(env, _value, jni_cache->boolean)) { + } else if (bsg_safe_is_instance_of(env, _value, bsg_jni_cache->Boolean)) { // add a boolean metadata value - bool value = bsg_safe_call_boolean_method(env, _value, - jni_cache->boolean_bool_value); + bool value = bsg_safe_call_boolean_method( + env, _value, bsg_jni_cache->Boolean_booleanValue); bsg_add_metadata_value_bool(dst, section, name, value); - } else if (bsg_safe_is_instance_of(env, _value, jni_cache->string)) { + } else if (bsg_safe_is_instance_of(env, _value, bsg_jni_cache->String)) { const char *value = bsg_safe_get_string_utf_chars(env, _value); if (value != NULL) { bsg_add_metadata_value_str(dst, section, name, value); @@ -377,31 +361,31 @@ static void populate_metadata_value(JNIEnv *env, bsg_jni_cache *jni_cache, } } -static void populate_metadata_obj(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_metadata *dst, jobject section, - jobject section_keylist, int index) { +static void populate_metadata_obj(JNIEnv *env, bugsnag_metadata *dst, + jobject section, jobject section_keylist, + int index) { jstring section_key = NULL; const char *name = NULL; jobject _value = NULL; - if (jni_cache == NULL) { + if (!bsg_jni_cache->initialized) { goto exit; } section_key = bsg_safe_call_object_method( - env, section_keylist, jni_cache->arraylist_get, (jint)index); + env, section_keylist, bsg_jni_cache->ArrayList_get, (jint)index); if (section_key == NULL) { goto exit; } - _value = bsg_safe_call_object_method(env, section, jni_cache->map_get, + _value = bsg_safe_call_object_method(env, section, bsg_jni_cache->Map_get, section_key); name = bsg_safe_get_string_utf_chars(env, section_key); if (name == NULL) { goto exit; } - populate_metadata_value(env, jni_cache, dst, section, name, _value); + populate_metadata_value(env, dst, section, name, _value); exit: bsg_safe_release_string_utf_chars(env, section_key, name); @@ -409,16 +393,20 @@ static void populate_metadata_obj(JNIEnv *env, bsg_jni_cache *jni_cache, bsg_safe_delete_local_ref(env, _value); } -static void populate_metadata_section(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_metadata *dst, jobject metadata, - jobject keylist, int i) { +static void populate_metadata_section(JNIEnv *env, bugsnag_metadata *dst, + jobject metadata, jobject keylist, + int i) { jstring _key = NULL; const char *section = NULL; jobject _section = NULL; jobject section_keyset = NULL; jobject section_keylist = NULL; - _key = bsg_safe_call_object_method(env, keylist, jni_cache->arraylist_get, + if (!bsg_jni_cache->initialized) { + goto exit; + } + + _key = bsg_safe_call_object_method(env, keylist, bsg_jni_cache->ArrayList_get, (jint)i); if (_key == NULL) { goto exit; @@ -428,29 +416,29 @@ static void populate_metadata_section(JNIEnv *env, bsg_jni_cache *jni_cache, goto exit; } _section = - bsg_safe_call_object_method(env, metadata, jni_cache->map_get, _key); + bsg_safe_call_object_method(env, metadata, bsg_jni_cache->Map_get, _key); if (_section == NULL) { goto exit; } jint section_size = - bsg_safe_call_int_method(env, _section, jni_cache->map_size); + bsg_safe_call_int_method(env, _section, bsg_jni_cache->Map_size); if (section_size == -1) { goto exit; } section_keyset = - bsg_safe_call_object_method(env, _section, jni_cache->map_key_set); + bsg_safe_call_object_method(env, _section, bsg_jni_cache->Map_keySet); if (section_keyset == NULL) { goto exit; } section_keylist = - bsg_safe_new_object(env, jni_cache->arraylist, - jni_cache->arraylist_init_with_obj, section_keyset); + bsg_safe_new_object(env, bsg_jni_cache->ArrayList, + bsg_jni_cache->ArrayList_constructor, section_keyset); if (section_keylist == NULL) { goto exit; } for (int j = 0; j < section_size; j++) { - populate_metadata_obj(env, jni_cache, dst, _section, section_keylist, j); + populate_metadata_obj(env, dst, _section, section_keylist, j); } goto exit; @@ -464,18 +452,20 @@ static void populate_metadata_section(JNIEnv *env, bsg_jni_cache *jni_cache, // Internal API -void bsg_populate_metadata(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_metadata *dst, jobject metadata) { +void bsg_populate_metadata(JNIEnv *env, bugsnag_metadata *dst, + jobject metadata) { jobject _metadata = NULL; jobject keyset = NULL; jobject keylist = NULL; - if (jni_cache == NULL) { + if (!bsg_jni_cache->initialized) { goto exit; } + if (metadata == NULL) { _metadata = bsg_safe_call_static_object_method( - env, jni_cache->native_interface, jni_cache->ni_get_metadata); + env, bsg_jni_cache->NativeInterface, + bsg_jni_cache->NativeInterface_getMetadata); metadata = _metadata; } @@ -484,25 +474,25 @@ void bsg_populate_metadata(JNIEnv *env, bsg_jni_cache *jni_cache, goto exit; } - int size = bsg_safe_call_int_method(env, metadata, jni_cache->map_size); + int size = bsg_safe_call_int_method(env, metadata, bsg_jni_cache->Map_size); if (size == -1) { goto exit; } // create a list of metadata keys - keyset = - bsg_safe_call_static_object_method(env, metadata, jni_cache->map_key_set); + keyset = bsg_safe_call_static_object_method(env, metadata, + bsg_jni_cache->Map_keySet); if (keyset == NULL) { goto exit; } - keylist = bsg_safe_new_object(env, jni_cache->arraylist, - jni_cache->arraylist_init_with_obj, keyset); + keylist = bsg_safe_new_object(env, bsg_jni_cache->ArrayList, + bsg_jni_cache->ArrayList_constructor, keyset); if (keylist == NULL) { goto exit; } for (int i = 0; i < size; i++) { - populate_metadata_section(env, jni_cache, dst, metadata, keylist, i); + populate_metadata_section(env, dst, metadata, keylist, i); } exit: @@ -511,46 +501,47 @@ void bsg_populate_metadata(JNIEnv *env, bsg_jni_cache *jni_cache, bsg_safe_delete_local_ref(env, keylist); } -void bsg_populate_crumb_metadata(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_breadcrumb *crumb, jobject metadata) { +void bsg_populate_crumb_metadata(JNIEnv *env, bugsnag_breadcrumb *crumb, + jobject metadata) { jobject keyset = NULL; jobject keylist = NULL; if (metadata == NULL) { goto exit; } - if (jni_cache == NULL) { + if (!bsg_jni_cache->initialized) { goto exit; } // get size of metadata map - jint map_size = bsg_safe_call_int_method(env, metadata, jni_cache->map_size); + jint map_size = + bsg_safe_call_int_method(env, metadata, bsg_jni_cache->Map_size); if (map_size == -1) { goto exit; } // create a list of metadata keys - keyset = bsg_safe_call_object_method(env, metadata, jni_cache->map_key_set); + keyset = + bsg_safe_call_object_method(env, metadata, bsg_jni_cache->Map_keySet); if (keyset == NULL) { goto exit; } - keylist = bsg_safe_new_object(env, jni_cache->arraylist, - jni_cache->arraylist_init_with_obj, keyset); + keylist = bsg_safe_new_object(env, bsg_jni_cache->ArrayList, + bsg_jni_cache->ArrayList_constructor, keyset); if (keylist == NULL) { goto exit; } for (int i = 0; i < map_size; i++) { jstring _key = bsg_safe_call_object_method( - env, keylist, jni_cache->arraylist_get, (jint)i); - jobject _value = - bsg_safe_call_object_method(env, metadata, jni_cache->map_get, _key); + env, keylist, bsg_jni_cache->ArrayList_get, (jint)i); + jobject _value = bsg_safe_call_object_method(env, metadata, + bsg_jni_cache->Map_get, _key); if (_key != NULL && _value != NULL) { const char *key = bsg_safe_get_string_utf_chars(env, _key); if (key != NULL) { - populate_metadata_value(env, jni_cache, &crumb->metadata, "metaData", - key, _value); + populate_metadata_value(env, &crumb->metadata, "metaData", key, _value); bsg_safe_release_string_utf_chars(env, _key, key); } } @@ -563,15 +554,14 @@ void bsg_populate_crumb_metadata(JNIEnv *env, bsg_jni_cache *jni_cache, bsg_safe_delete_local_ref(env, keylist); } -void bsg_populate_event(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_event *event) { - if (jni_cache == NULL) { +void bsg_populate_event(JNIEnv *env, bugsnag_event *event) { + if (!bsg_jni_cache->initialized) { return; } - populate_context(env, jni_cache, event); - populate_app_data(env, jni_cache, event); - populate_device_data(env, jni_cache, event); - populate_user_data(env, jni_cache, event); + populate_context(env, event); + populate_app_data(env, event); + populate_device_data(env, event); + populate_user_data(env, event); } const char *bsg_os_name() { return "android"; } diff --git a/bugsnag-plugin-android-ndk/src/main/jni/metadata.h b/bugsnag-plugin-android-ndk/src/main/jni/metadata.h index fa5429b8a1..5998aaf686 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/metadata.h +++ b/bugsnag-plugin-android-ndk/src/main/jni/metadata.h @@ -9,21 +9,20 @@ * Load all app, device, user, and custom metadata from NativeInterface into a * event */ -void bsg_populate_event(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_event *event); +void bsg_populate_event(JNIEnv *env, bugsnag_event *event); /** * Load custom metadata from NativeInterface into a native metadata struct, * optionally from an object. If metadata is not provided, load from * NativeInterface */ -void bsg_populate_metadata(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_metadata *dst, jobject metadata); +void bsg_populate_metadata(JNIEnv *env, bugsnag_metadata *dst, + jobject metadata); /** * Parse as java.util.Map to populate crumb metadata */ -void bsg_populate_crumb_metadata(JNIEnv *env, bsg_jni_cache *jni_cache, - bugsnag_breadcrumb *crumb, jobject metadata); +void bsg_populate_crumb_metadata(JNIEnv *env, bugsnag_breadcrumb *crumb, + jobject metadata); const char *bsg_os_name(); diff --git a/bugsnag-plugin-android-ndk/src/main/jni/safejni.c b/bugsnag-plugin-android-ndk/src/main/jni/safejni.c index b68224e435..730608ceb5 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/safejni.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/safejni.c @@ -1,5 +1,9 @@ #include "safejni.h" +#include "bugsnag_ndk.h" +#include "utils/logger.h" +#include "utils/stack_unwinder.h" #include +#include #include bool bsg_check_and_clear_exc(JNIEnv *env) { @@ -7,6 +11,17 @@ bool bsg_check_and_clear_exc(JNIEnv *env) { return false; } if ((*env)->ExceptionCheck(env)) { + BUGSNAG_LOG("BUG: JNI Native->Java call threw an exception:"); + + // Print a trace to stderr so that we can debug it + (*env)->ExceptionDescribe(env); + + // Trigger more accurate dalvik trace (this will also crash the app). + + // Code review check: THIS MUST BE COMMENTED OUT IN CHECKED IN CODE! + //(*env)->FindClass(env, NULL); + + // Clear the exception so that we don't crash. (*env)->ExceptionClear(env); return true; } @@ -18,7 +33,9 @@ jclass bsg_safe_find_class(JNIEnv *env, const char *clz_name) { return NULL; } jclass clz = (*env)->FindClass(env, clz_name); - bsg_check_and_clear_exc(env); + if (bsg_check_and_clear_exc(env)) { + return NULL; + } return clz; } @@ -28,7 +45,9 @@ jmethodID bsg_safe_get_method_id(JNIEnv *env, jclass clz, const char *name, return NULL; } jmethodID methodId = (*env)->GetMethodID(env, clz, name, sig); - bsg_check_and_clear_exc(env); + if (bsg_check_and_clear_exc(env)) { + return NULL; + } return methodId; } @@ -38,7 +57,9 @@ jmethodID bsg_safe_get_static_method_id(JNIEnv *env, jclass clz, return NULL; } jmethodID methodId = (*env)->GetStaticMethodID(env, clz, name, sig); - bsg_check_and_clear_exc(env); + if (bsg_check_and_clear_exc(env)) { + return NULL; + } return methodId; } @@ -47,7 +68,9 @@ jstring bsg_safe_new_string_utf(JNIEnv *env, const char *str) { return NULL; } jstring jstr = (*env)->NewStringUTF(env, str); - bsg_check_and_clear_exc(env); + if (bsg_check_and_clear_exc(env)) { + return NULL; + } return jstr; } @@ -103,7 +126,9 @@ jlong bsg_safe_call_long_method(JNIEnv *env, jobject _value, jmethodID method) { return 0; } jlong value = (*env)->CallLongMethod(env, _value, method); - bsg_check_and_clear_exc(env); + if (bsg_check_and_clear_exc(env)) { + return -1; // default to -1 + } return value; } @@ -112,7 +137,9 @@ jobjectArray bsg_safe_new_object_array(JNIEnv *env, jsize size, jclass clz) { return NULL; } jobjectArray trace = (*env)->NewObjectArray(env, size, clz, NULL); - bsg_check_and_clear_exc(env); + if (bsg_check_and_clear_exc(env)) { + return NULL; + } return trace; } @@ -122,7 +149,9 @@ jobject bsg_safe_get_object_array_element(JNIEnv *env, jobjectArray array, return NULL; } jobject obj = (*env)->GetObjectArrayElement(env, array, size); - bsg_check_and_clear_exc(env); + if (bsg_check_and_clear_exc(env)) { + return NULL; + } return obj; } @@ -141,7 +170,9 @@ jfieldID bsg_safe_get_static_field_id(JNIEnv *env, jclass clz, const char *name, return NULL; } jfieldID field_id = (*env)->GetStaticFieldID(env, clz, name, sig); - bsg_check_and_clear_exc(env); + if (bsg_check_and_clear_exc(env)) { + return NULL; + } return field_id; } @@ -151,7 +182,9 @@ jobject bsg_safe_get_static_object_field(JNIEnv *env, jclass clz, return NULL; } jobject obj = (*env)->GetStaticObjectField(env, clz, field); - bsg_check_and_clear_exc(env); + if (bsg_check_and_clear_exc(env)) { + return NULL; + } return obj; } @@ -163,7 +196,9 @@ jobject bsg_safe_new_object(JNIEnv *env, jclass clz, jmethodID method, ...) { va_start(args, method); jobject obj = (*env)->NewObjectV(env, clz, method, args); va_end(args); - bsg_check_and_clear_exc(env); + if (bsg_check_and_clear_exc(env)) { + return NULL; + } return obj; } @@ -176,7 +211,9 @@ jobject bsg_safe_call_object_method(JNIEnv *env, jobject _value, va_start(args, method); jobject value = (*env)->CallObjectMethodV(env, _value, method, args); va_end(args); - bsg_check_and_clear_exc(env); + if (bsg_check_and_clear_exc(env)) { + return NULL; + } return value; } @@ -201,7 +238,9 @@ jobject bsg_safe_call_static_object_method(JNIEnv *env, jclass clz, va_start(args, method); jobject obj = (*env)->CallStaticObjectMethodV(env, clz, method, args); va_end(args); - bsg_check_and_clear_exc(env); + if (bsg_check_and_clear_exc(env)) { + return NULL; + } return obj; } diff --git a/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java b/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java index 26b06feb2b..7405e969e5 100644 --- a/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java +++ b/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java @@ -37,6 +37,7 @@ static ImmutableConfig generateConfig() throws IOException { 22, 32, 32, + 1000, LazyKt.lazy(new Function0() { @Override public File invoke() { diff --git a/features/fixtures/mazerunner/jvm-scenarios/detekt-baseline.xml b/features/fixtures/mazerunner/jvm-scenarios/detekt-baseline.xml index 44227c2619..1720b6a473 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/detekt-baseline.xml +++ b/features/fixtures/mazerunner/jvm-scenarios/detekt-baseline.xml @@ -7,6 +7,7 @@ MagicNumber:AutoDetectAnrsFalseScenario.kt$AutoDetectAnrsFalseScenario$100000 MagicNumber:AutoDetectAnrsTrueScenario.kt$AutoDetectAnrsTrueScenario$100000 MagicNumber:BugsnagInitScenario.kt$BugsnagInitScenario$25 + MagicNumber:HandledExceptionMaxThreadsScenario.kt$HandledExceptionMaxThreadsScenario$3 MagicNumber:HandledKotlinSmokeScenario.kt$HandledKotlinSmokeScenario$999 MagicNumber:JvmAnrSleepScenario.kt$JvmAnrSleepScenario$100000 MagicNumber:LoadConfigurationKotlinScenario.kt$LoadConfigurationKotlinScenario$10000 diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledExceptionMaxThreadsScenario.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledExceptionMaxThreadsScenario.kt new file mode 100644 index 0000000000..ea4dc113c5 --- /dev/null +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/HandledExceptionMaxThreadsScenario.kt @@ -0,0 +1,24 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Bugsnag +import com.bugsnag.android.Configuration + +/** + * Sends a handled exception to Bugsnag, which does not include session data. + */ +internal class HandledExceptionMaxThreadsScenario( + config: Configuration, + context: Context, + eventMetadata: String +) : Scenario(config, context, eventMetadata) { + + init { + config.maxReportedThreads = 3 + } + + override fun startScenario() { + super.startScenario() + Bugsnag.notify(generateException()) + } +} diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledExceptionMaxThreadsScenario.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledExceptionMaxThreadsScenario.kt new file mode 100644 index 0000000000..1896cb19df --- /dev/null +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/UnhandledExceptionMaxThreadsScenario.kt @@ -0,0 +1,23 @@ +package com.bugsnag.android.mazerunner.scenarios + +import android.content.Context +import com.bugsnag.android.Configuration + +/** + * Sends an unhandled exception to Bugsnag. + */ +internal class UnhandledExceptionMaxThreadsScenario( + config: Configuration, + context: Context, + eventMetadata: String +) : Scenario(config, context, eventMetadata) { + + init { + config.maxReportedThreads = 2 + } + + override fun startScenario() { + super.startScenario() + throw generateException() + } +} diff --git a/features/full_tests/max_reported_threads.feature b/features/full_tests/max_reported_threads.feature new file mode 100644 index 0000000000..d42e942d82 --- /dev/null +++ b/features/full_tests/max_reported_threads.feature @@ -0,0 +1,25 @@ +Feature: Reporting with other exception handlers installed + +Scenario: Unhandled exception with max threads set + When I run "UnhandledExceptionMaxThreadsScenario" and relaunch the app + And I configure Bugsnag for "UnhandledExceptionMaxThreadsScenario" + And I wait to receive an error + Then the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + And the error payload field "events" is an array with 1 elements + And the exception "errorClass" equals "java.lang.RuntimeException" + And the exception "message" equals "UnhandledExceptionMaxThreadsScenario" + And the error payload field "events.0.threads" is an array with 3 elements + And the error payload field "events.0.threads.2.id" equals -1 + And the error payload field "events.0.threads.2.name" ends with "threads omitted as the maxReportedThreads limit (2) was exceeded]" + +Scenario: Handled exception with max threads set + When I run "HandledExceptionMaxThreadsScenario" and relaunch the app + And I configure Bugsnag for "HandledExceptionMaxThreadsScenario" + And I wait to receive an error + Then the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier + And the error payload field "events" is an array with 1 elements + And the exception "errorClass" equals "java.lang.RuntimeException" + And the exception "message" equals "HandledExceptionMaxThreadsScenario" + And the error payload field "events.0.threads" is an array with 4 elements + And the error payload field "events.0.threads.3.id" equals -1 + And the error payload field "events.0.threads.3.name" ends with "threads omitted as the maxReportedThreads limit (3) was exceeded]" diff --git a/features/steps/android_steps.rb b/features/steps/android_steps.rb index 95c0c87153..c89ae15da2 100644 --- a/features/steps/android_steps.rb +++ b/features/steps/android_steps.rb @@ -1,5 +1,3 @@ -require 'minitest/spec' - # Waits 5s for an element to be present. If it isn't assume a system error dialog is # blocking its view and dismiss it before trying once more. # diff --git a/gradle.properties b/gradle.properties index 14d8c10d1d..13efc4cbdf 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.19.2 +VERSION_NAME=5.20.0 GROUP=com.bugsnag POM_SCM_URL=https://github.com/bugsnag/bugsnag-android POM_SCM_CONNECTION=scm:git@github.com:bugsnag/bugsnag-android.git