fix(android): Stop Hermes profiler on React instance teardown (#6035)

* fix(android): Stop Hermes sampling profiler on React instance teardown (#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>

* Fix changelog

* fix(android): Address review: atomic state + startProfiling unwind + 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>

* fix(android): Isolate Hermes disable from Android profiler cleanup in 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>

* Update changelog

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: antonis

CHANGELOG.md

@@ -6,6 +6,12 @@
 > make sure you follow our [migration guide](https://docs.sentry.io/platforms/react-native/migration/) first.
 <!-- prettier-ignore-end -->
 
+## Unreleased
+
+### Fixes
+
+- Stop the Hermes sampling profiler on React instance teardown to prevent `pthread_kill` SIGABRT when the JS thread is torn down with profiling active ([#6035](https://github.com/getsentry/sentry-react-native/pull/6035))
+
 ## 8.9.1
 
 ### Features

packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryProfilingTest.kt

@@ -0,0 +1,33 @@
+package io.sentry.react
+
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+
+/**
+ * Tests for the profiling lifecycle on the RNSentryModuleImpl.
+ *
+ * Note: the Hermes sampling profiler is a JNI-backed static API and cannot be loaded in a pure
+ * JVM unit-test environment. These tests exercise only the paths that do not cross the JNI
+ * boundary — specifically, the invalidate() guard that returns early when profiling is not
+ * active. If the guard were to regress and call into Hermes, the tests would fail with
+ * UnsatisfiedLinkError.
+ */
+@RunWith(RobolectricTestRunner::class)
+class RNSentryProfilingTest {
+    @Test
+    fun `invalidate is a no-op on a fresh module`() {
+        val module = Utils.createRNSentryModuleWithMockedContext()
+        // isProfiling starts false; invalidate() should return early without touching
+        // HermesSamplingProfiler. If it didn't, the JNI call would throw UnsatisfiedLinkError.
+        module.invalidate()
+    }
+
+    @Test
+    fun `invalidate is idempotent`() {
+        val module = Utils.createRNSentryModuleWithMockedContext()
+        module.invalidate()
+        module.invalidate()
+        module.invalidate()
+    }
+}

packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java

@@ -77,6 +77,8 @@
 import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.regex.Pattern;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -114,7 +116,15 @@ public class RNSentryModuleImpl {
    */
   private int profilingTracesHz = 101;
 
-  private AndroidProfiler androidProfiler = null;
+  // Atomic because invalidate() (NativeModules/teardown thread) can null the reference
+  // concurrently with stopProfiling() (JS thread). getAndSet gives us an atomic snapshot-and-clear.
+  private final AtomicReference<AndroidProfiler> androidProfiler = new AtomicReference<>();
+
+  // Atomic so that invalidate() can claim cleanup responsibility via compareAndSet/getAndSet.
+  // startProfiling/stopProfiling run on the JS thread (@ReactMethod isBlockingSynchronousMethod);
+  // invalidate runs on the NativeModules/teardown thread. Without atomicity, invalidate could
+  // observe a stale false and skip disabling a running Hermes sampler.
+  private final AtomicBoolean isProfiling = new AtomicBoolean(false);
 
   private boolean isProguardDebugMetaLoaded = false;
   private @Nullable String proguardUuid = null;
@@ -773,29 +783,45 @@ private void initializeAndroidProfiler() {
     }
     final String tracesFilesDirPath = getProfilingTracesDirPath();
 
-    androidProfiler =
+    androidProfiler.set(
         new AndroidProfiler(
             tracesFilesDirPath,
             (int) SECONDS.toMicros(1) / profilingTracesHz,
             new SentryFrameMetricsCollector(reactApplicationContext, logger, buildInfo),
             () -> executorService,
-            logger);
+            logger));
   }
 
   public WritableMap startProfiling(boolean platformProfilers) {
     final WritableMap result = new WritableNativeMap();
-    if (androidProfiler == null && platformProfilers) {
+    if (androidProfiler.get() == null && platformProfilers) {
       initializeAndroidProfiler();
     }
 
     try {
+      // Defensive reset: the Hermes sampling profiler is global with no state
+      // introspection, so flush any leaked registration from a prior run before
+      // enabling. See https://github.com/facebook/hermes/issues/1853.
+      HermesSamplingProfiler.disable();
       HermesSamplingProfiler.enable();
-      if (androidProfiler != null) {
-        androidProfiler.start();
+      // Mark profiling active immediately after enable() succeeds so a concurrent
+      // invalidate() observes the running state even if androidProfiler.start() below throws.
+      isProfiling.set(true);
+      final AndroidProfiler profiler = androidProfiler.get();
+      if (profiler != null) {
+        profiler.start();
       }
 
       result.putBoolean("started", true);
     } catch (Throwable e) { // NOPMD - We don't want to crash in any case
+      // Unwind Hermes if we enabled it but failed before returning. getAndSet(false) makes
+      // this idempotent with a concurrent invalidate()/stopProfiling().
+      if (isProfiling.getAndSet(false)) {
+        try {
+          HermesSamplingProfiler.disable();
+        } catch (Throwable ignored) { // NOPMD - Best-effort unwind.
+        }
+      }
       result.putBoolean("started", false);
       result.putString("error", e.toString());
     }
@@ -805,11 +831,20 @@ public WritableMap startProfiling(boolean platformProfilers) {
   public WritableMap stopProfiling() {
     final boolean isDebug = ScopesAdapter.getInstance().getOptions().isDebug();
     final WritableMap result = new WritableNativeMap();
+    // Claim cleanup ownership atomically. If invalidate() got here first, it already
+    // disabled Hermes and collected the Android profiler; there's nothing to return.
+    if (!isProfiling.compareAndSet(true, false)) {
+      result.putString("error", "Profiling not active");
+      return result;
+    }
+    // Take sole ownership of the AndroidProfiler reference so a concurrent invalidate()
+    // cannot also call endAndCollect() on the same instance.
+    final AndroidProfiler profiler = androidProfiler.getAndSet(null);
     File output = null;
     try {
       AndroidProfiler.ProfileEndData end = null;
-      if (androidProfiler != null) {
-        end = androidProfiler.endAndCollect(false, null);
+      if (profiler != null) {
+        end = profiler.endAndCollect(false, null);
       }
       HermesSamplingProfiler.disable();
 
@@ -852,6 +887,57 @@ public WritableMap stopProfiling() {
     return result;
   }
 
+  /**
+   * Cleanup hook invoked from the module wrappers when the React instance is destroyed (e.g. Metro
+   * reload, orderly Activity teardown). Calls HermesSamplingProfiler.disable(), which synchronously
+   * joins the sampler's timer thread, so no pthread_kill can fire after the JS thread is torn down.
+   *
+   * <p>Correctness depends on RN invoking module invalidate() before joining the JS message queue
+   * thread. That is the documented contract for both old-arch CatalystInstance shutdown and
+   * new-arch TurboModule teardown.
+   *
+   * <p>See https://github.com/getsentry/sentry-react-native/issues/5441 and
+   * https://github.com/facebook/hermes/issues/1853.
+   */
+  public void invalidate() {
+    // Atomic gate: only one caller (invalidate vs stopProfiling vs a re-entrant invalidate)
+    // wins the right to clean up; the rest no-op.
+    if (!isProfiling.getAndSet(false)) {
+      return;
+    }
+    // getAndSet(null) atomically snatches the reference so a concurrent stopProfiling()
+    // sees null and skips its own endAndCollect() on the same instance.
+    final AndroidProfiler profiler = androidProfiler.getAndSet(null);
+
+    // Crash-critical: disable the Hermes sampler first. disable() synchronously joins the
+    // timer thread, so once it returns no pthread_kill can fire after the JS thread is
+    // joined. Isolated in its own try/catch so a failure in the Android profiler cleanup
+    // below cannot skip this call.
+    try {
+      HermesSamplingProfiler.disable();
+      logger.log(SentryLevel.INFO, "Stopped Hermes sampling profiler on React instance destroy.");
+    } catch (Throwable e) { // NOPMD - Guards the JNI boundary; does not catch native SIGABRT.
+      logger.log(SentryLevel.WARNING, "Failed to stop Hermes sampling profiler on teardown: " + e);
+    }
+
+    // Android platform profiler is independent of Hermes; a failure here must not leak
+    // back into the Hermes shutdown above. The profile is being discarded; delete the
+    // trace file to avoid leaking it in cacheDir.
+    if (profiler != null) {
+      try {
+        final AndroidProfiler.ProfileEndData end = profiler.endAndCollect(false, null);
+        if (end != null && end.traceFile != null) {
+          try {
+            end.traceFile.delete();
+          } catch (Throwable ignored) { // NOPMD - File cleanup is best-effort.
+          }
+        }
+      } catch (Throwable e) { // NOPMD - Android profiler cleanup is best-effort on teardown.
+        logger.log(SentryLevel.WARNING, "AndroidProfiler cleanup failed during invalidate: " + e);
+      }
+    }
+  }
+
   private @Nullable String getProguardUuid() {
     if (isProguardDebugMetaLoaded) {
       return proguardUuid;

packages/core/android/src/newarch/java/io/sentry/react/RNSentryModule.java

@@ -233,4 +233,10 @@ public void enableShakeDetection() {
   public void disableShakeDetection() {
     this.impl.disableShakeDetection();
   }
+
+  @Override
+  public void invalidate() {
+    this.impl.invalidate();
+    super.invalidate();
+  }
 }

packages/core/android/src/oldarch/java/io/sentry/react/RNSentryModule.java

@@ -233,4 +233,10 @@ public void enableShakeDetection() {
   public void disableShakeDetection() {
     this.impl.disableShakeDetection();
   }
+
+  @Override
+  public void invalidate() {
+    this.impl.invalidate();
+    super.invalidate();
+  }
 }