[QA] Move NDK scope sync to background thread (#3754)

* Move NDK scope sync to background thread

* Update Changelog

* Cleanup code

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
 

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)
 }

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<String, Object> 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<String, Object> 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()

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<INativeScope>()
         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())
+    }
 }