Skip to content

Commit d28a94e

Browse files
markushimarkushi
authored
Merge cf412d4 into b5b093e 
2 parents b5b093e + cf412d4 commit d28a94e

File tree

4 files changed

+104
-27
lines changed

4 files changed

+104
-27
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
- If you're using code obfuscation, adjust your proguard-rules accordingly, so your custom view class name is not minified
2121
- Fix ensure Application Context is used even when SDK is initialized via Activity Context ([#3669](https://github.com/getsentry/sentry-java/pull/3669))
2222
- Fix potential ANRs due to `Calendar.getInstance` usage in Breadcrumbs constructor ([#3736](https://github.com/getsentry/sentry-java/pull/3736))
23+
- Fix potential ANRs due to NDK scope sync ([#3754](https://github.com/getsentry/sentry-java/pull/3754))
2324

2425
*Breaking changes*:
2526

‎sentry-android-ndk/build.gradle.kts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,6 @@ dependencies {
105105

106106
testImplementation(kotlin(Config.kotlinStdLib, KotlinCompilerVersion.VERSION))
107107
testImplementation(Config.TestLibs.kotlinTestJunit)
108-
109108
testImplementation(Config.TestLibs.mockitoKotlin)
109+
testImplementation(projects.sentryTestSupport)
110110
}

‎sentry-android-ndk/src/main/java/io/sentry/android/ndk/NdkScopeObserver.java

Lines changed: 64 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,18 @@ public NdkScopeObserver(final @NotNull SentryOptions options) {
3131
@Override
3232
public void setUser(final @Nullable User user) {
3333
try {
34-
if (user == null) {
35-
// remove user if its null
36-
nativeScope.removeUser();
37-
} else {
38-
nativeScope.setUser(user.getId(), user.getEmail(), user.getIpAddress(), user.getUsername());
39-
}
34+
options
35+
.getExecutorService()
36+
.submit(
37+
() -> {
38+
if (user == null) {
39+
// remove user if its null
40+
nativeScope.removeUser();
41+
} else {
42+
nativeScope.setUser(
43+
user.getId(), user.getEmail(), user.getIpAddress(), user.getUsername());
44+
}
45+
});
4046
} catch (Throwable e) {
4147
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync setUser has an error.");
4248
}
@@ -45,24 +51,36 @@ public void setUser(final @Nullable User user) {
4551
@Override
4652
public void addBreadcrumb(final @NotNull Breadcrumb crumb) {
4753
try {
48-
String level = null;
49-
if (crumb.getLevel() != null) {
50-
level = crumb.getLevel().name().toLowerCase(Locale.ROOT);
51-
}
52-
final String timestamp = DateUtils.getTimestamp(crumb.getTimestamp());
54+
options
55+
.getExecutorService()
56+
.submit(
57+
() -> {
58+
String level = null;
59+
if (crumb.getLevel() != null) {
60+
level = crumb.getLevel().name().toLowerCase(Locale.ROOT);
61+
}
62+
final String timestamp = DateUtils.getTimestamp(crumb.getTimestamp());
5363

54-
String data = null;
55-
try {
56-
final Map<String, Object> dataRef = crumb.getData();
57-
if (!dataRef.isEmpty()) {
58-
data = options.getSerializer().serialize(dataRef);
59-
}
60-
} catch (Throwable e) {
61-
options.getLogger().log(SentryLevel.ERROR, e, "Breadcrumb data is not serializable.");
62-
}
64+
String data = null;
65+
try {
66+
final Map<String, Object> dataRef = crumb.getData();
67+
if (!dataRef.isEmpty()) {
68+
data = options.getSerializer().serialize(dataRef);
69+
}
70+
} catch (Throwable e) {
71+
options
72+
.getLogger()
73+
.log(SentryLevel.ERROR, e, "Breadcrumb data is not serializable.");
74+
}
6375

64-
nativeScope.addBreadcrumb(
65-
level, crumb.getMessage(), crumb.getCategory(), crumb.getType(), timestamp, data);
76+
nativeScope.addBreadcrumb(
77+
level,
78+
crumb.getMessage(),
79+
crumb.getCategory(),
80+
crumb.getType(),
81+
timestamp,
82+
data);
83+
});
6684
} catch (Throwable e) {
6785
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync addBreadcrumb has an error.");
6886
}
@@ -71,7 +89,12 @@ public void addBreadcrumb(final @NotNull Breadcrumb crumb) {
7189
@Override
7290
public void setTag(final @NotNull String key, final @NotNull String value) {
7391
try {
74-
nativeScope.setTag(key, value);
92+
options
93+
.getExecutorService()
94+
.submit(
95+
() -> {
96+
nativeScope.setTag(key, value);
97+
});
7598
} catch (Throwable e) {
7699
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync setTag(%s) has an error.", key);
77100
}
@@ -80,7 +103,12 @@ public void setTag(final @NotNull String key, final @NotNull String value) {
80103
@Override
81104
public void removeTag(final @NotNull String key) {
82105
try {
83-
nativeScope.removeTag(key);
106+
options
107+
.getExecutorService()
108+
.submit(
109+
() -> {
110+
nativeScope.removeTag(key);
111+
});
84112
} catch (Throwable e) {
85113
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync removeTag(%s) has an error.", key);
86114
}
@@ -89,7 +117,12 @@ public void removeTag(final @NotNull String key) {
89117
@Override
90118
public void setExtra(final @NotNull String key, final @NotNull String value) {
91119
try {
92-
nativeScope.setExtra(key, value);
120+
options
121+
.getExecutorService()
122+
.submit(
123+
() -> {
124+
nativeScope.setExtra(key, value);
125+
});
93126
} catch (Throwable e) {
94127
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync setExtra(%s) has an error.", key);
95128
}
@@ -98,7 +131,12 @@ public void setExtra(final @NotNull String key, final @NotNull String value) {
98131
@Override
99132
public void removeExtra(final @NotNull String key) {
100133
try {
101-
nativeScope.removeExtra(key);
134+
options
135+
.getExecutorService()
136+
.submit(
137+
() -> {
138+
nativeScope.removeExtra(key);
139+
});
102140
} catch (Throwable e) {
103141
options
104142
.getLogger()

‎sentry-android-ndk/src/test/java/io/sentry/android/ndk/NdkScopeObserverTest.kt

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,13 @@ import io.sentry.JsonSerializer
66
import io.sentry.SentryLevel
77
import io.sentry.SentryOptions
88
import io.sentry.protocol.User
9+
import io.sentry.test.DeferredExecutorService
10+
import io.sentry.test.ImmediateExecutorService
11+
import org.mockito.kotlin.any
12+
import org.mockito.kotlin.anyOrNull
913
import org.mockito.kotlin.eq
1014
import org.mockito.kotlin.mock
15+
import org.mockito.kotlin.never
1116
import org.mockito.kotlin.verify
1217
import kotlin.test.Test
1318

@@ -17,6 +22,7 @@ class NdkScopeObserverTest {
1722
val nativeScope = mock<INativeScope>()
1823
val options = SentryOptions().apply {
1924
setSerializer(JsonSerializer(mock()))
25+
executorService = ImmediateExecutorService()
2026
}
2127

2228
fun getSut(): NdkScopeObserver {
@@ -111,4 +117,36 @@ class NdkScopeObserverTest {
111117
eq(data)
112118
)
113119
}
120+
121+
@Test
122+
fun `scope sync utilizes executor service`() {
123+
val executorService = DeferredExecutorService()
124+
fixture.options.executorService = executorService
125+
val sut = fixture.getSut()
126+
127+
sut.setTag("a", "b")
128+
sut.removeTag("a")
129+
sut.setExtra("a", "b")
130+
sut.removeExtra("a")
131+
sut.setUser(User())
132+
sut.addBreadcrumb(Breadcrumb())
133+
134+
// as long as the executor service is not run, the scope sync is not called
135+
verify(fixture.nativeScope, never()).setTag(any(), any())
136+
verify(fixture.nativeScope, never()).removeTag(any())
137+
verify(fixture.nativeScope, never()).setExtra(any(), any())
138+
verify(fixture.nativeScope, never()).removeExtra(any())
139+
verify(fixture.nativeScope, never()).setUser(any(), any(), any(), any())
140+
verify(fixture.nativeScope, never()).addBreadcrumb(any(), any(), any(), any(), any(), any())
141+
142+
// when the executor service is run, the scope sync is called
143+
executorService.runAll()
144+
145+
verify(fixture.nativeScope).setTag(any(), any())
146+
verify(fixture.nativeScope).removeTag(any())
147+
verify(fixture.nativeScope).setExtra(any(), any())
148+
verify(fixture.nativeScope).removeExtra(any())
149+
verify(fixture.nativeScope).setUser(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull())
150+
verify(fixture.nativeScope).addBreadcrumb(anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull())
151+
}
114152
}

0 commit comments

Comments
 (0)