From 11a2ed074318ffe20735ef3a1e9cf4ece5bd3879 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 10 Apr 2019 12:53:55 -0400 Subject: [PATCH] Increase reliability of locally logging crashes. Exception logging tends to be race-y, so now we block and wait for all logs to be written before continuing with the crash. --- .../securesms/ApplicationContext.java | 2 +- .../securesms/logging/AndroidLogger.java | 4 ++++ src/org/thoughtcrime/securesms/logging/Log.java | 9 +++++++++ .../securesms/logging/PersistentLogger.java | 16 +++++++++++++++- .../logging/UncaughtExceptionLogger.java | 7 +++---- 5 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/org/thoughtcrime/securesms/ApplicationContext.java b/src/org/thoughtcrime/securesms/ApplicationContext.java index d75df36da1..b8b0b0cf80 100644 --- a/src/org/thoughtcrime/securesms/ApplicationContext.java +++ b/src/org/thoughtcrime/securesms/ApplicationContext.java @@ -185,7 +185,7 @@ public class ApplicationContext extends MultiDexApplication implements Dependenc private void initializeCrashHandling() { final Thread.UncaughtExceptionHandler originalHandler = Thread.getDefaultUncaughtExceptionHandler(); - Thread.setDefaultUncaughtExceptionHandler(new UncaughtExceptionLogger(originalHandler, persistentLogger)); + Thread.setDefaultUncaughtExceptionHandler(new UncaughtExceptionLogger(originalHandler)); } private void initializeJobManager() { diff --git a/src/org/thoughtcrime/securesms/logging/AndroidLogger.java b/src/org/thoughtcrime/securesms/logging/AndroidLogger.java index e7695a0b65..570fda623a 100644 --- a/src/org/thoughtcrime/securesms/logging/AndroidLogger.java +++ b/src/org/thoughtcrime/securesms/logging/AndroidLogger.java @@ -31,4 +31,8 @@ public class AndroidLogger extends Log.Logger { public void wtf(String tag, String message, Throwable t) { android.util.Log.wtf(tag, message, t); } + + @Override + public void blockUntilAllWritesFinished() { + } } diff --git a/src/org/thoughtcrime/securesms/logging/Log.java b/src/org/thoughtcrime/securesms/logging/Log.java index 9ec2b11aea..31dc299227 100644 --- a/src/org/thoughtcrime/securesms/logging/Log.java +++ b/src/org/thoughtcrime/securesms/logging/Log.java @@ -127,6 +127,14 @@ public class Log { return simpleName; } + public static void blockUntilAllWritesFinished() { + if (loggers != null) { + for (Logger logger : loggers) { + logger.blockUntilAllWritesFinished(); + } + } + } + public static abstract class Logger { public abstract void v(String tag, String message, Throwable t); public abstract void d(String tag, String message, Throwable t); @@ -134,5 +142,6 @@ public class Log { public abstract void w(String tag, String message, Throwable t); public abstract void e(String tag, String message, Throwable t); public abstract void wtf(String tag, String message, Throwable t); + public abstract void blockUntilAllWritesFinished(); } } diff --git a/src/org/thoughtcrime/securesms/logging/PersistentLogger.java b/src/org/thoughtcrime/securesms/logging/PersistentLogger.java index 2720557f2e..334df55762 100644 --- a/src/org/thoughtcrime/securesms/logging/PersistentLogger.java +++ b/src/org/thoughtcrime/securesms/logging/PersistentLogger.java @@ -17,6 +17,7 @@ import java.util.Arrays; import java.util.Date; import java.util.LinkedList; import java.util.List; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; import java.util.concurrent.Executors; @@ -47,7 +48,7 @@ public class PersistentLogger extends Log.Logger { this.context = context.getApplicationContext(); this.secret = LogSecretProvider.getOrCreateAttachmentSecret(context); this.executor = Executors.newSingleThreadExecutor(r -> { - Thread thread = new Thread(r, "logger"); + Thread thread = new Thread(r, "PersistentLogger"); thread.setPriority(Thread.MIN_PRIORITY); return thread; }); @@ -85,6 +86,19 @@ public class PersistentLogger extends Log.Logger { write(LOG_WTF, tag, message, t); } + @Override + public void blockUntilAllWritesFinished() { + CountDownLatch latch = new CountDownLatch(1); + + executor.execute(latch::countDown); + + try { + latch.await(); + } catch (InterruptedException e) { + android.util.Log.w(TAG, "Failed to wait for all writes."); + } + } + @WorkerThread public ListenableFuture getLogs() { final SettableFuture future = new SettableFuture<>(); diff --git a/src/org/thoughtcrime/securesms/logging/UncaughtExceptionLogger.java b/src/org/thoughtcrime/securesms/logging/UncaughtExceptionLogger.java index a3da1aeac9..642703695a 100644 --- a/src/org/thoughtcrime/securesms/logging/UncaughtExceptionLogger.java +++ b/src/org/thoughtcrime/securesms/logging/UncaughtExceptionLogger.java @@ -7,16 +7,15 @@ public class UncaughtExceptionLogger implements Thread.UncaughtExceptionHandler private static final String TAG = UncaughtExceptionLogger.class.getSimpleName(); private final Thread.UncaughtExceptionHandler originalHandler; - private final PersistentLogger persistentLogger; - public UncaughtExceptionLogger(@NonNull Thread.UncaughtExceptionHandler originalHandler, @NonNull PersistentLogger persistentLogger) { - this.originalHandler = originalHandler; - this.persistentLogger = persistentLogger; + public UncaughtExceptionLogger(@NonNull Thread.UncaughtExceptionHandler originalHandler) { + this.originalHandler = originalHandler; } @Override public void uncaughtException(Thread t, Throwable e) { Log.e(TAG, "", e); + Log.blockUntilAllWritesFinished(); originalHandler.uncaughtException(t, e); } }