Render better crash stack traces for executors.

app/src/main/java/org/thoughtcrime/securesms/database/SqlCipherErrorHandler.java

@@ -7,9 +7,9 @@ import androidx.annotation.NonNull;
 import net.zetetic.database.DatabaseErrorHandler;
 import net.zetetic.database.sqlcipher.SQLiteDatabase;
 
+import org.signal.core.util.ExceptionUtil;
 import org.signal.core.util.logging.Log;
 import org.thoughtcrime.securesms.util.CursorUtil;
-import org.thoughtcrime.securesms.util.Util;
 
 /**
  * The default error handler wipes the file. This one instead prints some diagnostics and then crashes so the original corrupt file isn't lost.
@@ -45,7 +45,7 @@ public final class SqlCipherErrorHandler implements DatabaseErrorHandler {
       }
     } catch (Throwable t) {
       output.append("Failed to do integrity_check!").append("\n")
-            .append(Util.convertThrowableToString(t));
+            .append(ExceptionUtil.convertThrowableToString(t));
     }
 
     output.append("\n").append("===== PRAGMA cipher_integrity_check =====").append("\n");
@@ -56,7 +56,7 @@ public final class SqlCipherErrorHandler implements DatabaseErrorHandler {
       }
     } catch (Throwable t) {
       output.append("Failed to do cipher_integrity_check!").append("\n")
-            .append(Util.convertThrowableToString(t));
+            .append(ExceptionUtil.convertThrowableToString(t));
     }
 
     Log.e(TAG, output.toString());

app/src/main/java/org/thoughtcrime/securesms/jobmanager/JobManager.java

@@ -58,7 +58,7 @@ public class JobManager implements ConstraintObserver.Notifier {
   public JobManager(@NonNull Application application, @NonNull Configuration configuration) {
     this.application   = application;
     this.configuration = configuration;
-    this.executor      = new FilteredExecutor(configuration.getExecutorFactory().newSingleThreadExecutor("signal-JobManager"), ThreadUtil::isMainThread);
+    this.executor      = ThreadUtil.trace(new FilteredExecutor(configuration.getExecutorFactory().newSingleThreadExecutor("signal-JobManager"), ThreadUtil::isMainThread));
     this.jobTracker    = configuration.getJobTracker();
     this.jobController = new JobController(application,
                                            configuration.getJobStorage(),

app/src/main/java/org/thoughtcrime/securesms/notifications/OptimizedMessageNotifier.java

@@ -7,12 +7,12 @@ import android.os.Handler;
 import androidx.annotation.MainThread;
 import androidx.annotation.NonNull;
 
+import org.signal.core.util.ExceptionUtil;
 import org.signal.core.util.concurrent.SignalExecutors;
 import org.thoughtcrime.securesms.notifications.v2.MessageNotifierV2;
 import org.thoughtcrime.securesms.recipients.Recipient;
 import org.thoughtcrime.securesms.util.BubbleUtil;
 import org.thoughtcrime.securesms.util.LeakyBucketLimiter;
-import org.thoughtcrime.securesms.util.Util;
 
 /**
  * Uses a leaky-bucket strategy to limiting notification updates.
@@ -109,7 +109,7 @@ public class OptimizedMessageNotifier implements MessageNotifier {
       try {
         runnable.run();
       } catch (RuntimeException e) {
-        throw Util.appendStackTrace(e, prettyException);
+        throw ExceptionUtil.joinStackTrace(e, prettyException);
       }
     });
   }

app/src/main/java/org/thoughtcrime/securesms/recipients/LiveRecipientCache.java

@@ -9,7 +9,9 @@ import androidx.annotation.NonNull;
 
 import net.zetetic.database.sqlcipher.SQLiteDatabase;
 
+import org.signal.core.util.ThreadUtil;
 import org.signal.core.util.concurrent.SignalExecutors;
+import org.signal.core.util.concurrent.TracingExecutorService;
 import org.signal.core.util.logging.Log;
 import org.thoughtcrime.securesms.database.DatabaseFactory;
 import org.thoughtcrime.securesms.database.RecipientDatabase;
@@ -28,7 +30,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
-import java.util.UUID;
 import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
@@ -60,7 +61,7 @@ public final class LiveRecipientCache {
     this.localRecipientId  = new AtomicReference<>(null);
     this.unknown           = new LiveRecipient(context, Recipient.UNKNOWN);
     this.db                = DatabaseFactory.getInstance(context).getRawDatabase();
-    this.resolveExecutor   = new FilteredExecutor(SignalExecutors.BOUNDED, () -> !db.inTransaction());
+    this.resolveExecutor   = ThreadUtil.trace(new FilteredExecutor(SignalExecutors.BOUNDED, () -> !db.inTransaction()));
   }
 
   @AnyThread
@@ -83,16 +84,7 @@ public final class LiveRecipientCache {
     }
 
     if (needsResolve) {
-      final LiveRecipient toResolve = live;
+      resolveExecutor.execute(live::resolve);
-
-      MissingRecipientException prettyStackTraceError = new MissingRecipientException(toResolve.getId());
-      resolveExecutor.execute(() -> {
-        try {
-          toResolve.resolve();
-        } catch (MissingRecipientException e) {
-          throw prettyStackTraceError;
-        }
-      });
     }
 
     return live;

app/src/main/java/org/thoughtcrime/securesms/util/Util.java

@@ -515,31 +515,4 @@ public class Util {
       return defaultValue;
     }
   }
-
-  /**
-   * Appends the stack trace of the provided throwable onto the provided primary exception. This is
-   * useful for when exceptions are thrown inside of asynchronous systems (like runnables in an
-   * executor) where you'd otherwise lose important parts of the stack trace. This lets you save a
-   * throwable at the entry point, and then combine it with any caught exceptions later.
-   *
-   * @return The provided primary exception, for convenience.
-   */
-  public static RuntimeException appendStackTrace(@NonNull RuntimeException primary, @NonNull Throwable secondary) {
-    StackTraceElement[] now      = primary.getStackTrace();
-    StackTraceElement[] then     = secondary.getStackTrace();
-    StackTraceElement[] combined = new StackTraceElement[now.length + then.length];
-
-    System.arraycopy(now, 0, combined, 0, now.length);
-    System.arraycopy(then, 0, combined, now.length, then.length);
-
-    primary.setStackTrace(combined);
-
-    return primary;
-  }
-
-  public static @NonNull String convertThrowableToString(@NonNull Throwable throwable) {
-    ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
-    throwable.printStackTrace(new PrintStream(outputStream));
-    return outputStream.toString();
-  }
 }

core-util/src/main/java/org/signal/core/util/ExceptionUtil.java

@@ -0,0 +1,53 @@
+package org.signal.core.util;
+
+import androidx.annotation.NonNull;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+
+public final class ExceptionUtil {
+
+  private ExceptionUtil() {}
+
+  /**
+   * Joins the stack trace of the inferred call site with the original exception. This is
+   * useful for when exceptions are thrown inside of asynchronous systems (like runnables in an
+   * executor) where you'd otherwise lose important parts of the stack trace. This lets you save a
+   * throwable at the entry point, and then combine it with any caught exceptions later.
+   *
+   * The resulting stack trace will look like this:
+   *
+   * Inferred
+   * Stack
+   * Trace
+   * [[ ↑↑ Inferred Trace ↑↑ ]]
+   * [[ ↓↓ Original Trace ↓↓ ]]
+   * Original
+   * Stack
+   * Trace
+   *
+   * @return The provided original exception, for convenience.
+   */
+  public static <E extends Throwable> E joinStackTrace(@NonNull E original, @NonNull Throwable inferred) {
+    StackTraceElement[] originalTrace = original.getStackTrace();
+    StackTraceElement[] inferredTrace = inferred.getStackTrace();
+    StackTraceElement[] combinedTrace = new StackTraceElement[originalTrace.length + inferredTrace.length + 2];
+
+    System.arraycopy(originalTrace, 0, combinedTrace, 0, originalTrace.length);
+
+    combinedTrace[originalTrace.length]     = new StackTraceElement("[[ ↑↑ Original Trace ↑↑ ]]", "", "", 0);
+    combinedTrace[originalTrace.length + 1] = new StackTraceElement("[[ ↓↓ Inferred Trace ↓↓ ]]", "", "", 0);
+
+    System.arraycopy(inferredTrace, 0, combinedTrace, originalTrace.length + 2, inferredTrace.length);
+
+    original.setStackTrace(combinedTrace);
+
+    return original;
+  }
+
+  public static @NonNull String convertThrowableToString(@NonNull Throwable throwable) {
+    ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+    throwable.printStackTrace(new PrintStream(outputStream));
+    return outputStream.toString();
+  }
+}

core-util/src/main/java/org/signal/core/util/ThreadUtil.java

@@ -5,7 +5,12 @@ import android.os.Looper;
 
 import androidx.annotation.NonNull;
 
+import org.signal.core.util.concurrent.TracingExecutor;
+import org.signal.core.util.concurrent.TracingExecutorService;
+
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
 
 /**
  * Thread related utility functions.
@@ -93,4 +98,12 @@ public final class ThreadUtil {
       Thread.sleep(millis);
     } catch (InterruptedException ignored) { }
   }
+
+  public static Executor trace(Executor executor) {
+    return new TracingExecutor(executor);
+  }
+
+  public static ExecutorService trace(ExecutorService executor) {
+    return new TracingExecutorService(executor);
+  }
 }

core-util/src/main/java/org/signal/core/util/concurrent/SignalExecutors.java

@@ -5,6 +5,7 @@ import android.os.HandlerThread;
 import androidx.annotation.NonNull;
 
 import org.signal.core.util.LinkedBlockingLifoQueue;
+import org.signal.core.util.ThreadUtil;
 
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -16,10 +17,10 @@ import java.util.concurrent.atomic.AtomicInteger;
 
 public final class SignalExecutors {
 
-  public static final ExecutorService UNBOUNDED  = Executors.newCachedThreadPool(new NumberedThreadFactory("signal-unbounded"));
+  public static final ExecutorService UNBOUNDED  = ThreadUtil.trace(Executors.newCachedThreadPool(new NumberedThreadFactory("signal-unbounded")));
-  public static final ExecutorService BOUNDED    = Executors.newFixedThreadPool(4, new NumberedThreadFactory("signal-bounded"));
+  public static final ExecutorService BOUNDED    = ThreadUtil.trace(Executors.newFixedThreadPool(4, new NumberedThreadFactory("signal-bounded")));
-  public static final ExecutorService SERIAL     = Executors.newSingleThreadExecutor(new NumberedThreadFactory("signal-serial"));
+  public static final ExecutorService SERIAL     = ThreadUtil.trace(Executors.newSingleThreadExecutor(new NumberedThreadFactory("signal-serial")));
-  public static final ExecutorService BOUNDED_IO = newCachedBoundedExecutor("signal-io-bounded", 1, 32, 30);
+  public static final ExecutorService BOUNDED_IO = ThreadUtil.trace(newCachedBoundedExecutor("signal-io-bounded", 1, 32, 30));
 
   private SignalExecutors() {}
 

core-util/src/main/java/org/signal/core/util/concurrent/TracingExecutor.kt

@@ -0,0 +1,22 @@
+package org.signal.core.util.concurrent
+
+import java.util.concurrent.Executor
+
+/**
+ * An executor that will keep track of the stack trace at the time of calling [execute] and use that to build a more useful stack trace in the event of a crash.
+ */
+internal class TracingExecutor(val wrapped: Executor) : Executor by wrapped {
+
+  override fun execute(command: Runnable?) {
+    val callerStackTrace = Throwable()
+
+    wrapped.execute {
+      val currentHandler: Thread.UncaughtExceptionHandler? = Thread.currentThread().uncaughtExceptionHandler
+      val originalHandler: Thread.UncaughtExceptionHandler? = if (currentHandler is TracingUncaughtExceptionHandler) currentHandler.originalHandler else currentHandler
+
+      Thread.currentThread().uncaughtExceptionHandler = TracingUncaughtExceptionHandler(originalHandler, callerStackTrace)
+
+      command?.run()
+    }
+  }
+}

core-util/src/main/java/org/signal/core/util/concurrent/TracingExecutorService.kt

@@ -0,0 +1,22 @@
+package org.signal.core.util.concurrent
+
+import java.util.concurrent.ExecutorService
+
+/**
+ * An executor that will keep track of the stack trace at the time of calling [execute] and use that to build a more useful stack trace in the event of a crash.
+ */
+internal class TracingExecutorService(val wrapped: ExecutorService) : ExecutorService by wrapped {
+
+  override fun execute(command: Runnable?) {
+    val callerStackTrace = Throwable()
+
+    wrapped.execute {
+      val currentHandler: Thread.UncaughtExceptionHandler? = Thread.currentThread().uncaughtExceptionHandler
+      val originalHandler: Thread.UncaughtExceptionHandler? = if (currentHandler is TracingUncaughtExceptionHandler) currentHandler.originalHandler else currentHandler
+
+      Thread.currentThread().uncaughtExceptionHandler = TracingUncaughtExceptionHandler(originalHandler, callerStackTrace)
+
+      command?.run()
+    }
+  }
+}

core-util/src/main/java/org/signal/core/util/concurrent/TracingUncaughtExceptionHandler.kt

@@ -0,0 +1,16 @@
+package org.signal.core.util.concurrent
+
+import org.signal.core.util.ExceptionUtil
+
+/**
+ * An uncaught exception handler that will combine a caller stack trace with the exception to print a more useful stack trace.
+ */
+internal class TracingUncaughtExceptionHandler (
+    val originalHandler: Thread.UncaughtExceptionHandler?,
+    private val callerStackTrace: Throwable) : Thread.UncaughtExceptionHandler {
+
+  override fun uncaughtException(thread: Thread, exception: Throwable) {
+    val updated = ExceptionUtil.joinStackTrace(exception, callerStackTrace)
+    originalHandler?.uncaughtException(thread, updated)
+  }
+}