Skip to content

Conversation

@buenaflor
Copy link
Contributor

@buenaflor buenaflor commented Oct 13, 2025

📜 Description

Part of FFI/JNI refactorings

💡 Motivation and Context

Partly completes #3204

💚 How did you test it?

Manual tests, integration test

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

cursor[bot]

This comment was marked as outdated.

Copy link
Collaborator

@denrase denrase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small comment regarding internal obj conversion. If we'll use this elsewhere, it would make sense to move to a util and add tests.

cursor[bot]

This comment was marked as outdated.

Base automatically changed from enh/app-hang-ffi-jni-refactor to main October 23, 2025 12:03
@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: JNI Object Leaks in Dart-to-Java Conversion

JNI objects created during Dart-to-Java conversion in _dartToJList and _dartToJMap are not consistently released. This includes JString keys from entry.key.toJString() in _dartToJMap and JObject results from _dartToJObject(), leading to native memory leaks.

Fix in Cursor Fix in Web

@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Silent Failures in Breadcrumb Method

The addBreadcrumb method silently returns when nativeOptions or nativeBreadcrumb are null, bypassing error logging. This behavior is inconsistent with the intent to log failures rather than failing silently.

Fix in Cursor Fix in Web

@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2025

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1240.08 ms 1250.15 ms 10.06 ms
Size 5.53 MiB 6.01 MiB 488.15 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
32914d8 1275.47 ms 1285.20 ms 9.73 ms
a10aff4 1241.67 ms 1255.02 ms 13.35 ms
e04b24b 1230.22 ms 1233.90 ms 3.67 ms
40c8f93 1234.27 ms 1261.98 ms 27.71 ms
cf443d2 1255.79 ms 1248.38 ms -7.40 ms
aeb02f2 1244.29 ms 1256.55 ms 12.26 ms
0fb3800 1256.60 ms 1266.28 ms 9.68 ms
944b773 1252.82 ms 1254.08 ms 1.27 ms
827bf09 1261.86 ms 1276.41 ms 14.55 ms
8541716 1270.18 ms 1271.80 ms 1.62 ms

App size

Revision Plain With Sentry Diff
32914d8 7.86 MiB 9.44 MiB 1.58 MiB
a10aff4 5.53 MiB 6.00 MiB 486.71 KiB
e04b24b 5.53 MiB 6.00 MiB 480.00 KiB
40c8f93 5.53 MiB 6.00 MiB 479.94 KiB
cf443d2 5.53 MiB 6.00 MiB 479.99 KiB
aeb02f2 7.86 MiB 9.44 MiB 1.58 MiB
0fb3800 7.86 MiB 9.44 MiB 1.58 MiB
944b773 5.53 MiB 6.00 MiB 479.98 KiB
827bf09 7.86 MiB 9.44 MiB 1.58 MiB
8541716 5.53 MiB 6.00 MiB 479.96 KiB

Previous results on branch: enh/ffi-jni-breadcrumbs-sync

Startup times

Revision Plain With Sentry Diff
5ad708c 1240.73 ms 1236.33 ms -4.40 ms
bfb5259 1257.88 ms 1255.78 ms -2.10 ms
0cfd7ba 1242.02 ms 1239.67 ms -2.35 ms
1e52e9f 1238.96 ms 1251.20 ms 12.24 ms
a95a359 1270.80 ms 1266.96 ms -3.84 ms
a2fda2c 1260.47 ms 1255.32 ms -5.15 ms
d5857e8 1255.33 ms 1256.29 ms 0.96 ms
3a25e45 1255.56 ms 1253.44 ms -2.12 ms
d4641f9 1248.19 ms 1257.45 ms 9.26 ms
5cc9c78 1240.42 ms 1241.86 ms 1.44 ms

App size

Revision Plain With Sentry Diff
5ad708c 5.53 MiB 6.00 MiB 485.58 KiB
bfb5259 5.53 MiB 6.01 MiB 487.41 KiB
0cfd7ba 5.53 MiB 6.00 MiB 485.59 KiB
1e52e9f 5.53 MiB 6.01 MiB 488.22 KiB
a95a359 5.53 MiB 6.01 MiB 488.13 KiB
a2fda2c 5.53 MiB 6.00 MiB 485.62 KiB
d5857e8 5.53 MiB 6.01 MiB 487.41 KiB
3a25e45 5.53 MiB 6.00 MiB 486.54 KiB
d4641f9 5.53 MiB 6.00 MiB 485.59 KiB
5cc9c78 5.53 MiB 6.00 MiB 486.52 KiB

@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2025

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 412.16 ms 401.08 ms -11.08 ms
Size 13.93 MiB 15.06 MiB 1.13 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
5b9a0da 446.94 ms 449.22 ms 2.28 ms
0929dbf 462.82 ms 492.76 ms 29.94 ms
7b21e8b 467.74 ms 466.24 ms -1.50 ms
4298701 524.40 ms 633.30 ms 108.90 ms
73dca78 476.53 ms 522.21 ms 45.68 ms
29e8ebe 389.91 ms 395.76 ms 5.84 ms
6b69699 456.06 ms 557.44 ms 101.38 ms
6ba4675 499.80 ms 632.43 ms 132.63 ms
8825ed8 447.65 ms 456.90 ms 9.25 ms
e45c0e1 447.29 ms 558.33 ms 111.04 ms

App size

Revision Plain With Sentry Diff
5b9a0da 13.93 MiB 14.93 MiB 1.00 MiB
0929dbf 6.54 MiB 7.70 MiB 1.17 MiB
7b21e8b 13.93 MiB 15.00 MiB 1.06 MiB
4298701 6.54 MiB 7.71 MiB 1.17 MiB
73dca78 6.54 MiB 7.69 MiB 1.15 MiB
29e8ebe 13.93 MiB 15.06 MiB 1.13 MiB
6b69699 6.54 MiB 7.70 MiB 1.17 MiB
6ba4675 6.54 MiB 7.53 MiB 1015.26 KiB
8825ed8 13.93 MiB 14.93 MiB 1.00 MiB
e45c0e1 6.54 MiB 7.70 MiB 1.16 MiB

Previous results on branch: enh/ffi-jni-breadcrumbs-sync

Startup times

Revision Plain With Sentry Diff
1e52e9f 376.80 ms 364.85 ms -11.95 ms
bfb5259 436.33 ms 426.84 ms -9.49 ms
a2fda2c 488.88 ms 482.12 ms -6.75 ms
d5857e8 453.85 ms 473.59 ms 19.74 ms
5ad708c 414.58 ms 446.49 ms 31.91 ms
054c6ff 416.62 ms 421.02 ms 4.41 ms
5cc9c78 415.94 ms 423.55 ms 7.61 ms
a95a359 418.08 ms 418.00 ms -0.08 ms
6bcf81b 487.10 ms 490.40 ms 3.30 ms
0cfd7ba 420.21 ms 445.85 ms 25.65 ms

App size

Revision Plain With Sentry Diff
1e52e9f 13.93 MiB 15.06 MiB 1.13 MiB
bfb5259 13.93 MiB 15.06 MiB 1.13 MiB
a2fda2c 13.93 MiB 15.06 MiB 1.13 MiB
d5857e8 13.93 MiB 15.06 MiB 1.13 MiB
5ad708c 13.93 MiB 15.06 MiB 1.13 MiB
054c6ff 13.93 MiB 15.06 MiB 1.13 MiB
5cc9c78 13.93 MiB 15.06 MiB 1.13 MiB
a95a359 13.93 MiB 15.06 MiB 1.13 MiB
6bcf81b 13.93 MiB 15.06 MiB 1.13 MiB
0cfd7ba 13.93 MiB 15.06 MiB 1.13 MiB

@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Async Methods Violate FutureOr Declaration

The addBreadcrumb and clearBreadcrumbs methods are declared as FutureOr<void> in ScopeObserver, but their NativeScopeObserver implementations use async. This forces them to always return Future<void>, contradicting the FutureOr declaration. Callers expecting a synchronous result might not await, potentially causing race conditions or unexecuted code.

Additional Locations (2)

Fix in Cursor Fix in Web

@getsentry getsentry deleted a comment from cursor bot Oct 23, 2025
@getsentry getsentry deleted a comment from cursor bot Oct 23, 2025
@buenaflor buenaflor merged commit c0dde50 into main Oct 23, 2025
120 of 121 checks passed
@buenaflor buenaflor deleted the enh/ffi-jni-breadcrumbs-sync branch October 23, 2025 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants