fix(android): Preserve NDK stack on tombstone merge (JAVA-645) [on hold]#5771
Draft
runningcode wants to merge 2 commits into
Draft
fix(android): Preserve NDK stack on tombstone merge (JAVA-645) [on hold]#5771runningcode wants to merge 2 commits into
runningcode wants to merge 2 commits into
Conversation
When a native crash is also captured via the ApplicationExitInfo tombstone, mergeNativeCrashes overwrote the Native SDK event's exceptions, threads and debug-meta with the tombstone's. For JNI-raised crashes the tombstone's crashing-thread stack is the ART-unwound managed (OAT/VDEX) stack, which cannot be symbolicated or ProGuard-deobfuscated, so the merged event became unreadable. Keep the Native SDK's crashing-thread stack, threads and debug-meta (symbolicated against uploaded debug files) and only enrich the exception with the tombstone's signal metadata. Fall back to the tombstone data when the Native SDK event carries no exception. Fixes JAVA-645 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EmE8hdaj9H9K61opK2PZ6U
📲 Install BuildsAndroid
|
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b193867 | 319.59 ms | 403.09 ms | 83.50 ms |
| 2124a46 | 319.19 ms | 415.04 ms | 95.85 ms |
| a5ab36f | 316.83 ms | 394.54 ms | 77.71 ms |
| dba088c | 320.59 ms | 361.29 ms | 40.70 ms |
| 4e3e79d | 328.10 ms | 395.64 ms | 67.54 ms |
| abfcc92 | 309.54 ms | 380.32 ms | 70.78 ms |
| 539ca63 | 313.51 ms | 355.43 ms | 41.92 ms |
| 3d205d0 | 352.15 ms | 432.53 ms | 80.38 ms |
| bb0ff41 | 312.86 ms | 363.78 ms | 50.92 ms |
| 2195398 | 319.02 ms | 342.38 ms | 23.36 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b193867 | 1.58 MiB | 2.19 MiB | 620.00 KiB |
| 2124a46 | 1.58 MiB | 2.12 MiB | 551.51 KiB |
| a5ab36f | 1.58 MiB | 2.12 MiB | 555.26 KiB |
| dba088c | 1.58 MiB | 2.13 MiB | 558.99 KiB |
| 4e3e79d | 0 B | 0 B | 0 B |
| abfcc92 | 1.58 MiB | 2.13 MiB | 557.31 KiB |
| 539ca63 | 1.58 MiB | 2.12 MiB | 551.41 KiB |
| 3d205d0 | 1.58 MiB | 2.10 MiB | 532.97 KiB |
| bb0ff41 | 0 B | 0 B | 0 B |
| 2195398 | 0 B | 0 B | 0 B |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning
On hold — premise corrected by a control experiment. A genuine native fault (null-deref inside
libnative-sample.so) symbolicates tonative-sample.cpp:13onmainwithout this change, and also with it. Native symbolication / uploaded symbols were never broken — the original "unreadable native crash" was an artifact of the sample'sraise(SIGSEGV)-from-JNI crash (a managed-origin crash whose tombstone stack is the ART/OAT managed chain).What this change actually does for a real native crash:
mainkeeps the tombstone's full mixed stack (symbolicated native crash frame + the obfuscated managed call chain); this PR uses sentry-native's native-only crashing thread, which drops that call-chain context. So it is neutral-to-worse for genuine native crashes, not the win described below.The higher-value fix is to deobfuscate the managed frames in the merged native event (they stay obfuscated only because the event
platform == "native"). See JAVA-645 for the reframed analysis. Holding this PR pending that decision.📜 Description
When
io.sentry.tombstone.enable=true, a native crash that is also captured by sentry-native is reported through theApplicationExitInfotombstone path (mechanism: TombstoneMerged).TombstoneIntegration.mergeNativeCrasheskept the Native SDK event's contexts but overwrote itsexceptions,threads, anddebugMetawith the tombstone's. For JNI-raised crashes the tombstone's crashing-thread stack is the ART-unwound managed stack (OAT/VDEX frames), which cannot be symbolicated (native) or ProGuard-deobfuscated, so the merged event became unreadable.This change keeps the Native SDK's crashing-thread stack-trace, threads and debug-meta — which are symbolicated against the uploaded debug files — and only enriches the Native SDK exception with the tombstone's signal metadata (and flags the merge via the mechanism type). It falls back to the tombstone's data when the Native SDK event carries no exception.
💡 Motivation and Context
Native crashes on affected builds were effectively unreadable: obfuscated managed frames plus a couple of system frames, with no app source lines. Uploading native symbols and the ProGuard mapping did not help, because the frames that would consume them were discarded/replaced.
Evidence from a raw
TombstoneMergedevent JSON:libsentry.soshowsdebug_status: "found", and sentry-native's own worker threads symbolicate to source (sentry_backend_inproc.c:1238,sentry_batcher.c:339).base.vdex/base.odex/boot.oatwithsymbolicator_status: "unknown_image", retaining obfuscated names;libnative-sample.sowasdebug_status: "unused".Reproduced on Android 12, 16 and 17 (Pixel 3, Galaxy A55, Pixel 10) and SDK 8.48.0 / 8.49.0. The
sentrymonolith andsymbolicatorbehave correctly — the fix is entirely client-side.Fixes JAVA-645
💚 How did you test it?
Manually, with the
sentry-samples-androidrelease build (SAGP enabled, native symbols + ProGuard mapping uploaded):io.sentry.tombstone.enable=true→TombstoneMerged, unsymbolicated / obfuscated crashing stack.io.sentry.tombstone.enable=false→mechanism: signalhandler, and the pipeline symbolicates — confirming the merge was the cause.Automated tests for
mergeNativeCrashesstill need to be added/updated (see Next steps) — this is a draft.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
TombstoneIntegrationTestcoverage asserting the Native SDK crashing-thread stack/threads/debug-meta are preserved and only the mechanism/signal metadata is merged.platform: javaso ProGuard deobfuscation applies.