Skip to content

Commit cdf371b

Browse files
authored
fix potential segfault: dont use Companion in JNI calls and improve JNI ref releasing (#3354)
* Dont use Companion in JNI * Dont use Companion in JNI * Update CHANGELOG * Fix JNI ref release * Styling change * Refactor privacy options handling in Sentry init
1 parent ef47ff6 commit cdf371b

File tree

4 files changed

+53
-46
lines changed

4 files changed

+53
-46
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22

33
## Unreleased
44

5+
### Fixes
6+
7+
- Dont use `Companion` in JNI calls and properly release JNI refs ([#3354](https://github.com/getsentry/sentry-dart/pull/3354))
8+
- This potentially fixes segfault crashes related to JNI
9+
510
### Enhancements
611

712
- Flush logs if client/hub/sdk is closed ([#3335](https://github.com/getsentry/sentry-dart/pull/3335)

packages/flutter/lib/src/native/java/android_replay_recorder.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ class _AndroidReplayHandler extends WorkerHandler {
9999
late final native.ReplayIntegration _nativeReplay;
100100

101101
_AndroidReplayHandler(this._config) {
102-
_nativeReplay = native.SentryFlutterPlugin.Companion
103-
.privateSentryGetReplayIntegration()!;
102+
_nativeReplay =
103+
native.SentryFlutterPlugin.privateSentryGetReplayIntegration()!;
104104
}
105105

106106
@override

packages/flutter/lib/src/native/java/sentry_native_java.dart

Lines changed: 40 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ class SentryNativeJava extends SentryNativeChannel {
7474

7575
// NOTE: when instructionAddressSet is empty, loadDebugImagesAsBytes will return
7676
// all debug images as fallback.
77-
imagesUtf8JsonBytes = native.SentryFlutterPlugin.Companion
78-
.loadDebugImagesAsBytes(instructionAddressSet);
77+
imagesUtf8JsonBytes = native.SentryFlutterPlugin.loadDebugImagesAsBytes(
78+
instructionAddressSet);
7979
if (imagesUtf8JsonBytes == null) return null;
8080

8181
final byteRange =
@@ -112,8 +112,7 @@ class SentryNativeJava extends SentryNativeChannel {
112112
// is significantly faster because contexts can be large and contain many nested
113113
// objects. Local benchmarks show this method is ~4x faster than the alternative
114114
// approach of converting JNI objects to Dart objects one by one.
115-
contextsUtf8JsonBytes =
116-
native.SentryFlutterPlugin.Companion.loadContextsAsBytes();
115+
contextsUtf8JsonBytes = native.SentryFlutterPlugin.loadContextsAsBytes();
117116
if (contextsUtf8JsonBytes == null) return null;
118117

119118
final byteRange =
@@ -136,8 +135,7 @@ class SentryNativeJava extends SentryNativeChannel {
136135

137136
@override
138137
int? displayRefreshRate() => tryCatchSync('displayRefreshRate', () {
139-
return native.SentryFlutterPlugin.Companion
140-
.getDisplayRefreshRate()
138+
return native.SentryFlutterPlugin.getDisplayRefreshRate()
141139
?.intValue(releaseOriginal: true);
142140
});
143141

@@ -150,7 +148,7 @@ class SentryNativeJava extends SentryNativeChannel {
150148
return null;
151149
}
152150
appStartUtf8JsonBytes =
153-
native.SentryFlutterPlugin.Companion.fetchNativeAppStartAsBytes();
151+
native.SentryFlutterPlugin.fetchNativeAppStartAsBytes();
154152
if (appStartUtf8JsonBytes == null) return null;
155153

156154
final byteRange =
@@ -166,7 +164,7 @@ class SentryNativeJava extends SentryNativeChannel {
166164

167165
@override
168166
void nativeCrash() {
169-
native.SentryFlutterPlugin.Companion.crash();
167+
native.SentryFlutterPlugin.crash();
170168
}
171169

172170
@override
@@ -194,10 +192,12 @@ class SentryNativeJava extends SentryNativeChannel {
194192
final nativeOptions = native.ScopesAdapter.getInstance()?.getOptions()
195193
?..releasedBy(arena);
196194
if (nativeOptions == null) return;
197-
final jMap = _dartToJMap(breadcrumb.toJson(), arena);
195+
final jMap = _dartToJMap(breadcrumb.toJson());
198196
final nativeBreadcrumb =
199197
native.Breadcrumb.fromMap(jMap, nativeOptions)
200198
?..releasedBy(arena);
199+
// release jMap directly after use
200+
jMap.release();
201201
if (nativeBreadcrumb == null) return;
202202
native.Sentry.addBreadcrumb$1(nativeBreadcrumb);
203203
});
@@ -219,9 +219,11 @@ class SentryNativeJava extends SentryNativeChannel {
219219
?..releasedBy(arena);
220220
if (nativeOptions == null) return;
221221

222-
final nativeUser = native.User.fromMap(
223-
_dartToJMap(user.toJson(), arena), nativeOptions)
222+
final jMap = _dartToJMap(user.toJson());
223+
final nativeUser = native.User.fromMap(jMap, nativeOptions)
224224
?..releasedBy(arena);
225+
// release jMap directly after use
226+
jMap.release();
225227
if (nativeUser == null) return;
226228

227229
native.Sentry.setUser(nativeUser);
@@ -237,7 +239,7 @@ class SentryNativeJava extends SentryNativeChannel {
237239
run: (iScope) {
238240
using((arena) {
239241
final jKey = key.toJString()..releasedBy(arena);
240-
final jVal = _dartToJObject(value, arena);
242+
final jVal = _dartToJObject(value)?..releasedBy(arena);
241243

242244
if (jVal == null) return;
243245

@@ -306,8 +308,8 @@ class SentryNativeJava extends SentryNativeChannel {
306308
SentryId captureReplay() {
307309
final id = tryCatchSync<SentryId>('captureReplay', () {
308310
return using((arena) {
309-
_nativeReplay ??= native.SentryFlutterPlugin.Companion
310-
.privateSentryGetReplayIntegration();
311+
_nativeReplay ??=
312+
native.SentryFlutterPlugin.privateSentryGetReplayIntegration();
311313
// The passed parameter is `isTerminating`
312314
_nativeReplay?.captureReplay(false.toJBoolean()..releasedBy(arena));
313315

@@ -380,46 +382,45 @@ class SentryNativeJava extends SentryNativeChannel {
380382
0, // bitRate is currently not used
381383
);
382384

383-
_nativeReplay ??= native.SentryFlutterPlugin.Companion
384-
.privateSentryGetReplayIntegration();
385+
_nativeReplay ??=
386+
native.SentryFlutterPlugin.privateSentryGetReplayIntegration();
385387
_nativeReplay?.onConfigurationChanged(replayConfig);
386388

387389
replayConfig.release();
388390
});
389391
}
390392

391-
JObject? _dartToJObject(Object? value, Arena arena) => switch (value) {
393+
JObject? _dartToJObject(Object? value) => switch (value) {
392394
null => null,
393-
String s => s.toJString()..releasedBy(arena),
394-
bool b => b.toJBoolean()..releasedBy(arena),
395-
int i => i.toJLong()..releasedBy(arena),
396-
double d => d.toJDouble()..releasedBy(arena),
397-
List<dynamic> l => _dartToJList(l, arena),
398-
Map<String, dynamic> m => _dartToJMap(m, arena),
395+
String s => s.toJString(),
396+
bool b => b.toJBoolean(),
397+
int i => i.toJLong(),
398+
double d => d.toJDouble(),
399+
List<dynamic> l => _dartToJList(l),
400+
Map<String, dynamic> m => _dartToJMap(m),
399401
_ => null
400402
};
401403

402-
JList<JObject?> _dartToJList(List<dynamic> values, Arena arena) {
403-
final jlist = JList.array(JObject.nullableType)..releasedBy(arena);
404-
405-
for (final value in values) {
406-
final jObj = _dartToJObject(value, arena);
407-
jlist.add(jObj);
404+
JList<JObject?> _dartToJList(List<dynamic> values) {
405+
final jList = JList.array(JObject.nullableType);
406+
for (final v in values) {
407+
final j = _dartToJObject(v);
408+
jList.add(j);
409+
j?.release();
408410
}
409-
410-
return jlist;
411+
return jList;
411412
}
412413

413-
JMap<JString, JObject?> _dartToJMap(Map<String, dynamic> json, Arena arena) {
414-
final jmap = JMap.hash(JString.type, JObject.nullableType)..releasedBy(arena);
415-
414+
JMap<JString, JObject?> _dartToJMap(Map<String, dynamic> json) {
415+
final jMap = JMap.hash(JString.type, JObject.nullableType);
416416
for (final entry in json.entries) {
417-
final key = entry.key.toJString()..releasedBy(arena);
418-
final value = _dartToJObject(entry.value, arena);
419-
jmap[key] = value;
417+
final jk = entry.key.toJString();
418+
final jv = _dartToJObject(entry.value);
419+
jMap[jk] = jv;
420+
jk.release();
421+
jv?.release();
420422
}
421-
422-
return jmap;
423+
return jMap;
423424
}
424425

425426
const _videoBlockSize = 16;

packages/flutter/lib/src/native/java/sentry_native_java_init.dart

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ void initSentryAndroid({
4646
);
4747

4848
replayCallbacks.use((cb) {
49-
native.SentryFlutterPlugin.Companion
50-
.setupReplay(androidOptions, cb);
49+
native.SentryFlutterPlugin.setupReplay(androidOptions, cb);
5150
});
5251
},
5352
),
@@ -126,7 +125,9 @@ native.SentryOptions$BeforeSendReplayCallback createBeforeSendReplayCallback(
126125
return shouldRemove;
127126
});
128127

129-
payload?.addAll(_dartToJMap(options.privacy.toJson(), arena));
128+
final jMap = _dartToJMap(options.privacy.toJson());
129+
payload?.addAll(jMap);
130+
jMap.release();
130131
}
131132
});
132133
return sentryReplayEvent;
@@ -151,8 +152,8 @@ native.ReplayRecorderCallbacks? createReplayRecorderCallbacks({
151152
SentryId.fromId(replayIdString.toDartString(releaseOriginal: true));
152153

153154
owner._replayId = replayId;
154-
owner._nativeReplay = native.SentryFlutterPlugin.Companion
155-
.privateSentryGetReplayIntegration();
155+
owner._nativeReplay =
156+
native.SentryFlutterPlugin.privateSentryGetReplayIntegration();
156157
owner._replayRecorder = AndroidReplayRecorder.factory(options);
157158
await owner._replayRecorder!.start();
158159
hub.configureScope((s) {

0 commit comments

Comments
 (0)