diff --git a/CHANGELOG.md b/CHANGELOG.md index 84040594a6..a18600c608 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Deprecate `enableTracing` option ([#3777](https://github.com/getsentry/sentry-java/pull/3777)) - Vendor `java.util.Random` and replace `java.security.SecureRandom` usages ([#3783](https://github.com/getsentry/sentry-java/pull/3783)) +- Fix potential ANRs due to NDK scope sync ([#3754](https://github.com/getsentry/sentry-java/pull/3754)) ## 7.15.0 diff --git a/sentry-android-ndk/build.gradle.kts b/sentry-android-ndk/build.gradle.kts index f6564cd97f..f1c4873053 100644 --- a/sentry-android-ndk/build.gradle.kts +++ b/sentry-android-ndk/build.gradle.kts @@ -105,6 +105,6 @@ dependencies { testImplementation(kotlin(Config.kotlinStdLib, KotlinCompilerVersion.VERSION)) testImplementation(Config.TestLibs.kotlinTestJunit) - testImplementation(Config.TestLibs.mockitoKotlin) + testImplementation(projects.sentryTestSupport) } diff --git a/sentry-android-ndk/src/main/java/io/sentry/android/ndk/NdkScopeObserver.java b/sentry-android-ndk/src/main/java/io/sentry/android/ndk/NdkScopeObserver.java index 009bba9b81..2350c419be 100644 --- a/sentry-android-ndk/src/main/java/io/sentry/android/ndk/NdkScopeObserver.java +++ b/sentry-android-ndk/src/main/java/io/sentry/android/ndk/NdkScopeObserver.java @@ -31,12 +31,18 @@ public NdkScopeObserver(final @NotNull SentryOptions options) { @Override public void setUser(final @Nullable User user) { try { - if (user == null) { - // remove user if its null - nativeScope.removeUser(); - } else { - nativeScope.setUser(user.getId(), user.getEmail(), user.getIpAddress(), user.getUsername()); - } + options + .getExecutorService() + .submit( + () -> { + if (user == null) { + // remove user if its null + nativeScope.removeUser(); + } else { + nativeScope.setUser( + user.getId(), user.getEmail(), user.getIpAddress(), user.getUsername()); + } + }); } catch (Throwable e) { options.getLogger().log(SentryLevel.ERROR, e, "Scope sync setUser has an error."); } @@ -45,24 +51,36 @@ public void setUser(final @Nullable User user) { @Override public void addBreadcrumb(final @NotNull Breadcrumb crumb) { try { - String level = null; - if (crumb.getLevel() != null) { - level = crumb.getLevel().name().toLowerCase(Locale.ROOT); - } - final String timestamp = DateUtils.getTimestamp(crumb.getTimestamp()); + options + .getExecutorService() + .submit( + () -> { + String level = null; + if (crumb.getLevel() != null) { + level = crumb.getLevel().name().toLowerCase(Locale.ROOT); + } + final String timestamp = DateUtils.getTimestamp(crumb.getTimestamp()); - String data = null; - try { - final Map dataRef = crumb.getData(); - if (!dataRef.isEmpty()) { - data = options.getSerializer().serialize(dataRef); - } - } catch (Throwable e) { - options.getLogger().log(SentryLevel.ERROR, e, "Breadcrumb data is not serializable."); - } + String data = null; + try { + final Map dataRef = crumb.getData(); + if (!dataRef.isEmpty()) { + data = options.getSerializer().serialize(dataRef); + } + } catch (Throwable e) { + options + .getLogger() + .log(SentryLevel.ERROR, e, "Breadcrumb data is not serializable."); + } - nativeScope.addBreadcrumb( - level, crumb.getMessage(), crumb.getCategory(), crumb.getType(), timestamp, data); + nativeScope.addBreadcrumb( + level, + crumb.getMessage(), + crumb.getCategory(), + crumb.getType(), + timestamp, + data); + }); } catch (Throwable e) { options.getLogger().log(SentryLevel.ERROR, e, "Scope sync addBreadcrumb has an error."); } @@ -71,7 +89,7 @@ public void addBreadcrumb(final @NotNull Breadcrumb crumb) { @Override public void setTag(final @NotNull String key, final @NotNull String value) { try { - nativeScope.setTag(key, value); + options.getExecutorService().submit(() -> nativeScope.setTag(key, value)); } catch (Throwable e) { options.getLogger().log(SentryLevel.ERROR, e, "Scope sync setTag(%s) has an error.", key); } @@ -80,7 +98,7 @@ public void setTag(final @NotNull String key, final @NotNull String value) { @Override public void removeTag(final @NotNull String key) { try { - nativeScope.removeTag(key); + options.getExecutorService().submit(() -> nativeScope.removeTag(key)); } catch (Throwable e) { options.getLogger().log(SentryLevel.ERROR, e, "Scope sync removeTag(%s) has an error.", key); } @@ -89,7 +107,7 @@ public void removeTag(final @NotNull String key) { @Override public void setExtra(final @NotNull String key, final @NotNull String value) { try { - nativeScope.setExtra(key, value); + options.getExecutorService().submit(() -> nativeScope.setExtra(key, value)); } catch (Throwable e) { options.getLogger().log(SentryLevel.ERROR, e, "Scope sync setExtra(%s) has an error.", key); } @@ -98,7 +116,7 @@ public void setExtra(final @NotNull String key, final @NotNull String value) { @Override public void removeExtra(final @NotNull String key) { try { - nativeScope.removeExtra(key); + options.getExecutorService().submit(() -> nativeScope.removeExtra(key)); } catch (Throwable e) { options .getLogger() diff --git a/sentry-android-ndk/src/test/java/io/sentry/android/ndk/NdkScopeObserverTest.kt b/sentry-android-ndk/src/test/java/io/sentry/android/ndk/NdkScopeObserverTest.kt index 335a7679e1..110f794cf9 100644 --- a/sentry-android-ndk/src/test/java/io/sentry/android/ndk/NdkScopeObserverTest.kt +++ b/sentry-android-ndk/src/test/java/io/sentry/android/ndk/NdkScopeObserverTest.kt @@ -6,8 +6,13 @@ import io.sentry.JsonSerializer import io.sentry.SentryLevel import io.sentry.SentryOptions import io.sentry.protocol.User +import io.sentry.test.DeferredExecutorService +import io.sentry.test.ImmediateExecutorService +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.verify import kotlin.test.Test @@ -17,6 +22,7 @@ class NdkScopeObserverTest { val nativeScope = mock() val options = SentryOptions().apply { setSerializer(JsonSerializer(mock())) + executorService = ImmediateExecutorService() } fun getSut(): NdkScopeObserver { @@ -111,4 +117,36 @@ class NdkScopeObserverTest { eq(data) ) } + + @Test + fun `scope sync utilizes executor service`() { + val executorService = DeferredExecutorService() + fixture.options.executorService = executorService + val sut = fixture.getSut() + + sut.setTag("a", "b") + sut.removeTag("a") + sut.setExtra("a", "b") + sut.removeExtra("a") + sut.setUser(User()) + sut.addBreadcrumb(Breadcrumb()) + + // as long as the executor service is not run, the scope sync is not called + verify(fixture.nativeScope, never()).setTag(any(), any()) + verify(fixture.nativeScope, never()).removeTag(any()) + verify(fixture.nativeScope, never()).setExtra(any(), any()) + verify(fixture.nativeScope, never()).removeExtra(any()) + verify(fixture.nativeScope, never()).setUser(any(), any(), any(), any()) + verify(fixture.nativeScope, never()).addBreadcrumb(any(), any(), any(), any(), any(), any()) + + // when the executor service is run, the scope sync is called + executorService.runAll() + + verify(fixture.nativeScope).setTag(any(), any()) + verify(fixture.nativeScope).removeTag(any()) + verify(fixture.nativeScope).setExtra(any(), any()) + verify(fixture.nativeScope).removeExtra(any()) + verify(fixture.nativeScope).setUser(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull()) + verify(fixture.nativeScope).addBreadcrumb(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull()) + } }