Skip to content

Commit 57a3722

Browse files
markushimarkushi
authored
Merge 9f71414 into f79c9c1 
2 parents f79c9c1 + 9f71414 commit 57a3722

File tree

4 files changed

+84
-27
lines changed

4 files changed

+84
-27
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
### Fixes
66

77
- Deprecate `enableTracing` option ([#3777](https://github.com/getsentry/sentry-java/pull/3777))
8+
- Fix potential ANRs due to NDK scope sync ([#3754](https://github.com/getsentry/sentry-java/pull/3754))
89

910
## 7.15.0
1011

‎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: 44 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,7 @@ 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.getExecutorService().submit(() -> nativeScope.setTag(key, value));
7593
} catch (Throwable e) {
7694
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync setTag(%s) has an error.", key);
7795
}
@@ -80,7 +98,7 @@ public void setTag(final @NotNull String key, final @NotNull String value) {
8098
@Override
8199
public void removeTag(final @NotNull String key) {
82100
try {
83-
nativeScope.removeTag(key);
101+
options.getExecutorService().submit(() -> nativeScope.removeTag(key));
84102
} catch (Throwable e) {
85103
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync removeTag(%s) has an error.", key);
86104
}
@@ -89,7 +107,7 @@ public void removeTag(final @NotNull String key) {
89107
@Override
90108
public void setExtra(final @NotNull String key, final @NotNull String value) {
91109
try {
92-
nativeScope.setExtra(key, value);
110+
options.getExecutorService().submit(() -> nativeScope.setExtra(key, value));
93111
} catch (Throwable e) {
94112
options.getLogger().log(SentryLevel.ERROR, e, "Scope sync setExtra(%s) has an error.", key);
95113
}
@@ -98,7 +116,7 @@ public void setExtra(final @NotNull String key, final @NotNull String value) {
98116
@Override
99117
public void removeExtra(final @NotNull String key) {
100118
try {
101-
nativeScope.removeExtra(key);
119+
options.getExecutorService().submit(() -> nativeScope.removeExtra(key));
102120
} catch (Throwable e) {
103121
options
104122
.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)