feature: support dispose and re-init in the android exception handler

packages/react-native/android/build.gradle

@@ -69,6 +69,10 @@ android {
     sourceCompatibility JavaVersion.VERSION_1_8
     targetCompatibility JavaVersion.VERSION_1_8
   }
+
+  testOptions {
+    unitTests.returnDefaultValues = true
+  }
 }
 
 repositories {

packages/react-native/android/src/main/java/backtrace/library/BacktraceAndroidBackgroundUnhandledExceptionHandler.java

@@ -15,6 +15,7 @@
 import java.io.StringWriter;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * Handle unhandled Android exceptions from background threads.
@@ -23,24 +24,16 @@
 public class BacktraceAndroidBackgroundUnhandledExceptionHandler extends ReactContextBaseJavaModule implements Thread.UncaughtExceptionHandler  {
     private final static transient String LOG_TAG = BacktraceAndroidBackgroundUnhandledExceptionHandler.class.getSimpleName();
 
-    private Thread.UncaughtExceptionHandler _rootHandler;
+    private static final Object INSTALL_LOCK = new Object();
 
-    private Thread _lastCaughtBackgroundExceptionThread;
-    private Throwable _lastCaughtBackgroundException;
+    private volatile Thread.UncaughtExceptionHandler _rootHandler;
 
-    /**
-     * Check if data shouldn't be reported.
-     */
-    private volatile boolean _shouldStop = false;
+    private boolean _installed = false;
 
-    /**
-     * React native callback method
-     */
-    private Callback _callback;
+    // React Native callbacks are single-use; invoking one twice throws.
+    private final AtomicReference<Callback> _callback = new AtomicReference<>();
 
-    private boolean _callbackInvoked = false;
-
-    private final CountDownLatch _reportProcessed = new CountDownLatch(1);
+    private volatile CountDownLatch _reportProcessed = new CountDownLatch(0);
 
     private static final long REPORT_PROCESSED_TIMEOUT_MS = 5000;
 
@@ -60,32 +53,47 @@ public String getName() {
     @ReactMethod
     public void start(Callback callback) {
         Log.d(LOG_TAG, "Initializing Android unhandled exception handler");
-        _callback = callback;
-        _rootHandler = Thread.getDefaultUncaughtExceptionHandler();
-        Thread.setDefaultUncaughtExceptionHandler(this);
+        synchronized (INSTALL_LOCK) {
+            if (!_installed) {
+                _rootHandler = Thread.getDefaultUncaughtExceptionHandler();
+                Thread.setDefaultUncaughtExceptionHandler(this);
+                _installed = true;
+            }
+        }
+        _callback.set(callback);
     }
 
     @Override
     public synchronized void uncaughtException(final Thread thread, final Throwable throwable) {
-        _lastCaughtBackgroundExceptionThread = thread;
-        _lastCaughtBackgroundException = throwable;
-        if (_shouldStop == true) {
-            finish();
-            return;
+        try {
+            if (throwable instanceof Exception) {
+                report(throwable);
+            }
+        } catch (RuntimeException ex) {
+            Log.w(LOG_TAG, "Failed to report the unhandled exception.", ex);
+        } finally {
+            Thread.UncaughtExceptionHandler rootHandler = _rootHandler;
+            if (rootHandler != null) {
+                rootHandler.uncaughtException(thread, throwable);
+            }
         }
-        // React Native callbacks are single-use; invoking one twice throws.
-        if (throwable instanceof Exception && !_callbackInvoked) {
-            _callbackInvoked = true;
-            String throwableType = throwable.getClass().getName();
-            _callback.invoke(throwableType, throwable.getMessage(), stackTraceToString(throwable.getStackTrace()));
-            waitForReportProcessing();
+    }
+
+    private void report(Throwable throwable) {
+        Callback callback = _callback.getAndSet(null);
+        if (callback == null) {
+            return;
         }
-        finish();
+        CountDownLatch reportProcessed = new CountDownLatch(1);
+        _reportProcessed = reportProcessed;
+        String throwableType = throwable.getClass().getName();
+        callback.invoke(throwableType, throwable.getMessage(), stackTraceToString(throwable.getStackTrace()));
+        waitForReportProcessing(reportProcessed);
     }
 
-    private void waitForReportProcessing() {
+    private void waitForReportProcessing(CountDownLatch reportProcessed) {
         try {
-            if (!_reportProcessed.await(REPORT_PROCESSED_TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
+            if (!reportProcessed.await(REPORT_PROCESSED_TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
                 Log.d(LOG_TAG, "Timed out waiting for the unhandled exception report to be processed.");
             }
         } catch (InterruptedException ex) {
@@ -112,21 +120,21 @@ private static void printStackTrace(StackTraceElement[] stackTrace, PrintWriter
         }
     }
 
-    public void finish() {
-        if (_lastCaughtBackgroundExceptionThread == null || _lastCaughtBackgroundException == null) {
-            Log.d(LOG_TAG, "The exception object or the exception thread is not available. This is probably a bug.");
-            return;
-        }
-        if (_shouldStop) {
-            Log.d(LOG_TAG, "Backtrace client has been disposed. The report won't be available.");
-            return;
-        }
-        _rootHandler.uncaughtException(_lastCaughtBackgroundExceptionThread, _lastCaughtBackgroundException);
-    }
-
     @ReactMethod
     public void stop() {
         Log.d(LOG_TAG, "Uncaught exception handler has been disabled.");
-        _shouldStop = true;
+        _callback.set(null);
+        synchronized (INSTALL_LOCK) {
+            if (Thread.getDefaultUncaughtExceptionHandler() == this) {
+                Thread.setDefaultUncaughtExceptionHandler(_rootHandler);
+                _installed = false;
+            }
+        }
+    }
+
+    @Override
+    public void invalidate() {
+        stop();
+        super.invalidate();
     }
-}
+}

packages/react-native/android/src/test/java/backtraceio/library/BacktraceAndroidBackgroundUnhandledExceptionHandlerTest.java

@@ -0,0 +1,241 @@
+package backtraceio.library;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
+
+import com.facebook.react.bridge.Callback;
+import java.util.ArrayList;
+import java.util.List;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class BacktraceAndroidBackgroundUnhandledExceptionHandlerTest {
+    private Thread.UncaughtExceptionHandler originalDefaultHandler;
+    private RecordingHandler rootHandler;
+    private BacktraceAndroidBackgroundUnhandledExceptionHandler handler;
+
+    @Before
+    public void setUp() {
+        originalDefaultHandler = Thread.getDefaultUncaughtExceptionHandler();
+        rootHandler = new RecordingHandler();
+        Thread.setDefaultUncaughtExceptionHandler(rootHandler);
+        handler = new BacktraceAndroidBackgroundUnhandledExceptionHandler(null);
+    }
+
+    @After
+    public void tearDown() {
+        Thread.setDefaultUncaughtExceptionHandler(originalDefaultHandler);
+    }
+
+    @Test
+    public void startReportsExceptionAndForwardsToPreviousHandler() {
+        RecordingCallback callback = new RecordingCallback(handler);
+        handler.start(callback);
+
+        crash(new IllegalStateException("boom"));
+
+        assertEquals(1, callback.calls.size());
+        assertEquals("java.lang.IllegalStateException", callback.calls.get(0)[0]);
+        assertEquals("boom", callback.calls.get(0)[1]);
+        assertEquals(1, rootHandler.received.size());
+    }
+
+    @Test
+    public void stopRestoresPreviousHandler() {
+        handler.start(new RecordingCallback(handler));
+
+        handler.stop();
+
+        assertSame(rootHandler, Thread.getDefaultUncaughtExceptionHandler());
+    }
+
+    @Test
+    public void stopWhileWrappedForwardsWithoutReporting() {
+        RecordingCallback callback = new RecordingCallback(handler);
+        handler.start(callback);
+        WrappingHandler wrapper = WrappingHandler.install();
+
+        handler.stop();
+        crash(new RuntimeException("after dispose"));
+
+        assertSame(wrapper, Thread.getDefaultUncaughtExceptionHandler());
+        assertEquals(0, callback.calls.size());
+        assertEquals(1, rootHandler.received.size());
+    }
+
+    @Test
+    public void startAfterStopReportsAndForwardsOnce() {
+        RecordingCallback firstCallback = new RecordingCallback(handler);
+        RecordingCallback secondCallback = new RecordingCallback(handler);
+        handler.start(firstCallback);
+        handler.stop();
+
+        handler.start(secondCallback);
+        crash(new RuntimeException("after re-init"));
+
+        assertSame(handler, Thread.getDefaultUncaughtExceptionHandler());
+        assertEquals(0, firstCallback.calls.size());
+        assertEquals(1, secondCallback.calls.size());
+        assertEquals(1, rootHandler.received.size());
+    }
+
+    @Test
+    public void secondStartDoesNotChainToItself() {
+        RecordingCallback firstCallback = new RecordingCallback(handler);
+        RecordingCallback secondCallback = new RecordingCallback(handler);
+        handler.start(firstCallback);
+
+        handler.start(secondCallback);
+        crash(new RuntimeException("second start"));
+        handler.stop();
+
+        assertEquals(0, firstCallback.calls.size());
+        assertEquals(1, secondCallback.calls.size());
+        assertEquals(1, rootHandler.received.size());
+        assertSame(rootHandler, Thread.getDefaultUncaughtExceptionHandler());
+    }
+
+    @Test
+    public void startAfterStopWhileWrappedDoesNotLoop() {
+        handler.start(new RecordingCallback(handler));
+        WrappingHandler wrapper = WrappingHandler.install();
+        handler.stop();
+        RecordingCallback callback = new RecordingCallback(handler);
+
+        handler.start(callback);
+        crash(new RuntimeException("re-init while wrapped"));
+
+        assertSame(wrapper, Thread.getDefaultUncaughtExceptionHandler());
+        assertEquals(1, wrapper.calls);
+        assertEquals(1, callback.calls.size());
+        assertEquals(1, rootHandler.received.size());
+    }
+
+    @Test(timeout = 2000)
+    public void disposeAndReinitDuringReportForwardsWithoutWaiting() {
+        RecordingCallback nextCallback = new RecordingCallback(handler);
+        handler.start(args -> {
+            handler.stop();
+            handler.start(nextCallback);
+            handler.reportProcessed();
+        });
+
+        crash(new RuntimeException("dispose during report"));
+
+        assertEquals(0, nextCallback.calls.size());
+        assertEquals(1, rootHandler.received.size());
+    }
+
+    @Test
+    public void failingCallbackStillForwards() {
+        handler.start(args -> {
+            throw new RuntimeException("callback failed");
+        });
+
+        crash(new RuntimeException("boom"));
+
+        assertEquals(1, rootHandler.received.size());
+    }
+
+    @Test
+    public void errorIsForwardedWithoutConsumingTheCallback() {
+        RecordingCallback callback = new RecordingCallback(handler);
+        handler.start(callback);
+
+        crash(new StackOverflowError());
+        crash(new RuntimeException("after error"));
+
+        assertEquals(1, callback.calls.size());
+        assertEquals(2, rootHandler.received.size());
+    }
+
+    @Test
+    public void stopBeforeStartKeepsCurrentHandler() {
+        handler.stop();
+
+        assertSame(rootHandler, Thread.getDefaultUncaughtExceptionHandler());
+
+        RecordingCallback callback = new RecordingCallback(handler);
+        handler.start(callback);
+        crash(new RuntimeException("start after early stop"));
+
+        assertEquals(1, callback.calls.size());
+        assertEquals(1, rootHandler.received.size());
+    }
+
+    @Test
+    public void invalidateRestoresPreviousHandler() {
+        handler.start(new RecordingCallback(handler));
+
+        handler.invalidate();
+
+        assertSame(rootHandler, Thread.getDefaultUncaughtExceptionHandler());
+    }
+
+    @Test
+    public void reloadedInstanceStartingFirstForwardsThroughInvalidatedInstance() {
+        RecordingCallback oldCallback = new RecordingCallback(handler);
+        handler.start(oldCallback);
+        BacktraceAndroidBackgroundUnhandledExceptionHandler reloaded =
+                new BacktraceAndroidBackgroundUnhandledExceptionHandler(null);
+        RecordingCallback newCallback = new RecordingCallback(reloaded);
+
+        reloaded.start(newCallback);
+        handler.invalidate();
+        reloaded.uncaughtException(Thread.currentThread(), new RuntimeException("after reload"));
+
+        assertEquals(0, oldCallback.calls.size());
+        assertEquals(1, newCallback.calls.size());
+        assertEquals(1, rootHandler.received.size());
+    }
+
+    private void crash(Throwable throwable) {
+        Thread.getDefaultUncaughtExceptionHandler().uncaughtException(Thread.currentThread(), throwable);
+    }
+
+    private static class RecordingHandler implements Thread.UncaughtExceptionHandler {
+        final List<Throwable> received = new ArrayList<>();
+
+        @Override
+        public void uncaughtException(Thread thread, Throwable throwable) {
+            received.add(throwable);
+        }
+    }
+
+    private static class WrappingHandler implements Thread.UncaughtExceptionHandler {
+        private final Thread.UncaughtExceptionHandler previous;
+        int calls = 0;
+
+        private WrappingHandler(Thread.UncaughtExceptionHandler previous) {
+            this.previous = previous;
+        }
+
+        static WrappingHandler install() {
+            WrappingHandler wrapper = new WrappingHandler(Thread.getDefaultUncaughtExceptionHandler());
+            Thread.setDefaultUncaughtExceptionHandler(wrapper);
+            return wrapper;
+        }
+
+        @Override
+        public void uncaughtException(Thread thread, Throwable throwable) {
+            calls++;
+            previous.uncaughtException(thread, throwable);
+        }
+    }
+
+    private static class RecordingCallback implements Callback {
+        private final BacktraceAndroidBackgroundUnhandledExceptionHandler owner;
+        final List<Object[]> calls = new ArrayList<>();
+
+        RecordingCallback(BacktraceAndroidBackgroundUnhandledExceptionHandler owner) {
+            this.owner = owner;
+        }
+
+        @Override
+        public void invoke(Object... args) {
+            calls.add(args);
+            owner.reportProcessed();
+        }
+    }
+}