fix(android): Stop Hermes profiler on React instance teardown#6035
Open
fix(android): Stop Hermes profiler on React instance teardown#6035
Conversation
#5441) HermesSamplingProfiler.enable() starts a global timer thread that fires pthread_kill(jsThread, SIGPROF) every ~10ms to sample stacks. Before this change, nothing stopped it when the ReactInstance was torn down (Metro reload, orderly Activity destroy, CodePush swap), so the sampler could call pthread_kill on a pthread that was already joined by RN's teardown sequence, resulting in SIGABRT in libhermes.so at hermes::vm::sampling_profiler::Sampler::platformSuspendVMAndWalkStack. Hermes's own Sampler::disable() synchronously joins the timer thread before returning. Calling it from the module's invalidate() hook closes the window before the JS thread is destroyed. - Override invalidate() in both new-arch and old-arch wrappers to call a cleanup hook on RNSentryModuleImpl. - Defensive HermesSamplingProfiler.disable() before enable() in startProfiling() to flush any leaked registration from a prior run. - Track isProfiling as volatile and snatch the AndroidProfiler reference locally in invalidate() to avoid racing with a concurrent stopProfiling on the JS thread. - Delete the android profiler trace file in invalidate() to avoid leaking it in cacheDir when the profile is being discarded. Upstream race: facebook/hermes#1853. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
antonis
commented
Apr 23, 2026
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…stopProfiling TOCTOU Apply review feedback from Sentry Warden and Cursor Bugbot: - Replace volatile fields with AtomicBoolean/AtomicReference so PMD's AvoidUsingVolatile rule passes and to make the cross-thread gating explicit. startProfiling/stopProfiling run on the JS thread; invalidate runs on the NativeModules teardown thread. - startProfiling now sets isProfiling=true immediately after HermesSamplingProfiler.enable() succeeds, before androidProfiler.start(). The catch block unwinds via getAndSet(false) + disable() so we can't leave the Hermes sampler running while the flag reads false. - stopProfiling snapshots androidProfiler.get() into a local, closing the TOCTOU where a concurrent invalidate() could null the field between the null-check and endAndCollect(). - invalidate gates on isProfiling.getAndSet(false) and atomically snatches the profiler reference with androidProfiler.getAndSet(null). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… invalidate Address two new review findings: - High severity (Cursor Bugbot): the previous invalidate() shared a single try/catch with endAndCollect(). If endAndCollect threw, the HermesSamplingProfiler.disable() call on the line below was skipped, leaving the sampler's timer thread alive — defeating the entire purpose of invalidate(). Reorder so the crash-critical Hermes disable runs first in its own try/catch; the Android platform profiler cleanup follows in an independent try/catch. - Medium severity (Cursor Bugbot / reviewer): stopProfiling could race with invalidate on the same AndroidProfiler instance (both calling endAndCollect concurrently). Gate stopProfiling on isProfiling.compareAndSet(true, false) and take ownership of the profiler reference via androidProfiler.getAndSet(null), mirroring invalidate's pattern. Also add two small idempotency tests that verify invalidate() is a no-op on a fresh module without crossing the Hermes JNI boundary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1c3a4b9. Configure here.
Contributor
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7ac3378+dirty | 404.78 ms | 439.84 ms | 35.06 ms |
| 3d377b5+dirty | 406.18 ms | 453.52 ms | 47.34 ms |
| 04207c4+dirty | 459.19 ms | 518.54 ms | 59.35 ms |
| 0d9949d+dirty | 403.57 ms | 437.00 ms | 33.43 ms |
| 3ce5254+dirty | 410.57 ms | 448.48 ms | 37.91 ms |
| 890d145+dirty | 504.54 ms | 491.55 ms | -12.99 ms |
| df5d108+dirty | 527.06 ms | 603.58 ms | 76.52 ms |
| 2c735cc+dirty | 414.09 ms | 438.47 ms | 24.38 ms |
| 4953e94+dirty | 442.02 ms | 456.52 ms | 14.50 ms |
| 4b87b12+dirty | 421.82 ms | 413.60 ms | -8.22 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7ac3378+dirty | 43.75 MiB | 48.13 MiB | 4.37 MiB |
| 3d377b5+dirty | 43.75 MiB | 48.14 MiB | 4.39 MiB |
| 04207c4+dirty | 43.75 MiB | 48.12 MiB | 4.37 MiB |
| 0d9949d+dirty | 43.75 MiB | 48.13 MiB | 4.37 MiB |
| 3ce5254+dirty | 43.75 MiB | 48.12 MiB | 4.37 MiB |
| 890d145+dirty | 43.75 MiB | 48.14 MiB | 4.39 MiB |
| df5d108+dirty | 43.75 MiB | 48.08 MiB | 4.33 MiB |
| 2c735cc+dirty | 43.75 MiB | 48.08 MiB | 4.33 MiB |
| 4953e94+dirty | 43.75 MiB | 48.08 MiB | 4.33 MiB |
| 4b87b12+dirty | 43.75 MiB | 48.14 MiB | 4.39 MiB |
📲 Install BuildsAndroid
|
Contributor
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7ac3378+dirty | 410.67 ms | 442.60 ms | 31.92 ms |
| 3d377b5+dirty | 425.38 ms | 440.67 ms | 15.30 ms |
| 04207c4+dirty | 395.40 ms | 456.55 ms | 61.15 ms |
| 0d9949d+dirty | 414.88 ms | 428.68 ms | 13.81 ms |
| 3ce5254+dirty | 373.90 ms | 427.84 ms | 53.94 ms |
| 890d145+dirty | 486.42 ms | 514.85 ms | 28.43 ms |
| df5d108+dirty | 434.82 ms | 447.39 ms | 12.57 ms |
| 2c735cc+dirty | 435.20 ms | 459.48 ms | 24.28 ms |
| 4953e94+dirty | 398.80 ms | 431.81 ms | 33.01 ms |
| 4b87b12+dirty | 356.23 ms | 399.86 ms | 43.63 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7ac3378+dirty | 43.94 MiB | 48.99 MiB | 5.05 MiB |
| 3d377b5+dirty | 43.94 MiB | 49.00 MiB | 5.06 MiB |
| 04207c4+dirty | 43.94 MiB | 48.98 MiB | 5.04 MiB |
| 0d9949d+dirty | 43.94 MiB | 48.99 MiB | 5.05 MiB |
| 3ce5254+dirty | 43.94 MiB | 48.98 MiB | 5.04 MiB |
| 890d145+dirty | 43.94 MiB | 49.00 MiB | 5.06 MiB |
| df5d108+dirty | 43.94 MiB | 48.94 MiB | 5.00 MiB |
| 2c735cc+dirty | 43.94 MiB | 48.94 MiB | 5.00 MiB |
| 4953e94+dirty | 43.94 MiB | 48.94 MiB | 5.00 MiB |
| 4b87b12+dirty | 43.94 MiB | 49.00 MiB | 5.06 MiB |
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
2 participants
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.
📢 Type of change
📜 Description
When the React instance is torn down (Metro reload, orderly Activity destroy, CodePush/Expo-Updates bundle swap), the Hermes sampling profiler's global timer thread is left running. It can then call
pthread_killon the now-joined JS thread, resulting in a native SIGABRT insidelibhermes.so(hermes::vm::sampling_profiler::Sampler::platformSuspendVMAndWalkStack).This change hooks RN's module-destruction lifecycle and stops the sampler before the JS thread goes away.
HermesSamplingProfiler.disable()is synchronous and joins the sampler's timer thread before returning (verified in HermesSampler::disable()), which closes the race window.Changes:
invalidate()on both the new-arch and old-arch module wrappers; call a cleanup hook onRNSentryModuleImplthat disables the Hermes profiler, ends the platform profiler, and deletes the discarded trace file.HermesSamplingProfiler.disable()beforeenable()instartProfiling()to flush any leaked registration from a prior run (Hermes offers no way to introspect its state).isProfilingasvolatilefor cross-thread visibility between@ReactMethod(isBlockingSynchronousMethod = true)calls on the JS thread andinvalidate()on the NativeModules thread.💡 Motivation and Context
Fixes #5441.
💚 How did you test it?
yarn lint:android— clean.:RNSentry:compileDebugJavaWithJavac(Java 17) — clean.:app:testDebugUnitTest— all 35 existing unit tests pass.profilesSampleRate: 1.0on anAndroid 16 emulator (API 36, matching the production crash cohort).
Confirmed:
hermes-samplingtimer thread is created when profiling engages(
ps -Tshows the thread).the invalidate path.
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps