Ensure log buffers are dumped to file on uncaught exceptions

Move the functionality for wrapping an uncaught exception pre-handler from PluginManager to a standalone manager. Make DumpHandler register another uncaught exception pre-handler that will tell the log buffer eulogizer to dump contents to a file. Fixes: 213884413 Test: manually verified, added tests, and ran atest Change-Id: If829fc33769819624530f766178689ecf0374926
2022-04-22 17:07:29 +00:00
parent fc9fdfe2a7
commit 7209b5baf1
9 changed files with 212 additions and 77 deletions
--- a/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginManagerImpl.java
+++ b/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginManagerImpl.java
@@ -31,13 +31,13 @@ import android.widget.Toast;
 import com.android.internal.messages.nano.SystemMessageProto.SystemMessage;
 import com.android.systemui.plugins.Plugin;
 import com.android.systemui.plugins.PluginListener;
+import com.android.systemui.shared.system.UncaughtExceptionPreHandlerManager;

 import java.io.FileDescriptor;
 import java.io.PrintWriter;
 import java.lang.Thread.UncaughtExceptionHandler;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;

 /**
 * @see Plugin
@@ -61,7 +61,7 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage
    public PluginManagerImpl(Context context,
            PluginActionManager.Factory actionManagerFactory,
            boolean debuggable,
-            Optional<UncaughtExceptionHandler> defaultHandlerOptional,
+            UncaughtExceptionPreHandlerManager preHandlerManager,
            PluginEnabler pluginEnabler,
            PluginPrefs pluginPrefs,
            List<String> privilegedPlugins) {
@@ -72,9 +72,7 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage
        mPluginPrefs = pluginPrefs;
        mPluginEnabler = pluginEnabler;

-        PluginExceptionHandler uncaughtExceptionHandler = new PluginExceptionHandler(
-                defaultHandlerOptional);
-        Thread.setUncaughtExceptionPreHandler(uncaughtExceptionHandler);
+        preHandlerManager.registerHandler(new PluginExceptionHandler());
    }

    public boolean isDebuggable() {
@@ -266,20 +264,12 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage
    }

    private class PluginExceptionHandler implements UncaughtExceptionHandler {
-        private final Optional<UncaughtExceptionHandler> mExceptionHandlerOptional;

-        private PluginExceptionHandler(
-                Optional<UncaughtExceptionHandler> exceptionHandlerOptional) {
-            mExceptionHandlerOptional = exceptionHandlerOptional;
-        }
+        private PluginExceptionHandler() {}

        @Override
        public void uncaughtException(Thread thread, Throwable throwable) {
            if (SystemProperties.getBoolean("plugin.debugging", false)) {
-                Throwable finalThrowable = throwable;
-                mExceptionHandlerOptional.ifPresent(
-                        handler -> handler.uncaughtException(thread, finalThrowable));
-
                return;
            }
            // Search for and disable plugins that may have been involved in this crash.
@@ -297,11 +287,6 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage
            if (disabledAny) {
                throwable = new CrashWhilePluginActiveException(throwable);
            }
-
-            // Run the normal exception handler so we can crash and cleanup our state.
-            Throwable finalThrowable = throwable;
-            mExceptionHandlerOptional.ifPresent(
-                    handler -> handler.uncaughtException(thread, finalThrowable));
        }

        private boolean checkStack(Throwable throwable) {
--- a/packages/SystemUI/shared/src/com/android/systemui/shared/system/UncaughtExceptionPreHandlerManager.kt
+++ b/packages/SystemUI/shared/src/com/android/systemui/shared/system/UncaughtExceptionPreHandlerManager.kt
@@ -0,0 +1,71 @@
+package com.android.systemui.shared.system
+
+import android.util.Log
+import java.lang.Thread.UncaughtExceptionHandler
+import java.util.concurrent.CopyOnWriteArrayList
+import javax.inject.Inject
+import javax.inject.Singleton
+
+/**
+ * Sets the global (static var in Thread) uncaught exception pre-handler to an implementation that
+ * delegates to each item in a list of registered UncaughtExceptionHandlers.
+ */
+@Singleton
+class UncaughtExceptionPreHandlerManager @Inject constructor() {
+    private val handlers: MutableList<UncaughtExceptionHandler> = CopyOnWriteArrayList()
+    private val globalUncaughtExceptionPreHandler = GlobalUncaughtExceptionHandler()
+
+    /**
+     * Adds an exception pre-handler to the list of handlers. If this has not yet set the global
+     * (static var in Thread) uncaught exception pre-handler yet, it will do so.
+     */
+    fun registerHandler(handler: UncaughtExceptionHandler) {
+        checkGlobalHandlerSetup()
+        addHandler(handler)
+    }
+
+    /**
+     * Verifies that the global handler is set in Thread. If not, sets is up.
+     */
+    private fun checkGlobalHandlerSetup() {
+        val currentHandler = Thread.getUncaughtExceptionPreHandler()
+        if (currentHandler != globalUncaughtExceptionPreHandler) {
+            if (currentHandler is GlobalUncaughtExceptionHandler) {
+                throw IllegalStateException("Two UncaughtExceptionPreHandlerManagers created")
+            }
+            currentHandler?.let { addHandler(it) }
+            Thread.setUncaughtExceptionPreHandler(globalUncaughtExceptionPreHandler)
+        }
+    }
+
+    /**
+     * Adds a handler if it has not already been added, preserving order.
+     */
+    private fun addHandler(it: UncaughtExceptionHandler) {
+        if (it !in handlers) {
+            handlers.add(it)
+        }
+    }
+
+    /**
+     * Calls uncaughtException on all registered handlers, catching and logging any new exceptions.
+     */
+    fun handleUncaughtException(thread: Thread?, throwable: Throwable?) {
+        for (handler in handlers) {
+            try {
+                handler.uncaughtException(thread, throwable)
+            } catch (e: Exception) {
+                Log.wtf("Uncaught exception pre-handler error", e)
+            }
+        }
+    }
+
+    /**
+     * UncaughtExceptionHandler impl that will be set as Thread's pre-handler static variable.
+     */
+    inner class GlobalUncaughtExceptionHandler : UncaughtExceptionHandler {
+        override fun uncaughtException(thread: Thread?, throwable: Throwable?) {
+            handleUncaughtException(thread, throwable)
+        }
+    }
+}
--- a/packages/SystemUI/src/com/android/systemui/SystemUIService.java
+++ b/packages/SystemUI/src/com/android/systemui/SystemUIService.java
@@ -71,6 +71,7 @@ public class SystemUIService extends Service {

        // Finish initializing dump logic
        mLogBufferFreezer.attach(mBroadcastDispatcher);
+        mDumpHandler.init();

        // If configured, set up a battery notification
        if (getResources().getBoolean(R.bool.config_showNotificationForUnknownBatteryState)) {
--- a/packages/SystemUI/src/com/android/systemui/dump/DumpHandler.kt
+++ b/packages/SystemUI/src/com/android/systemui/dump/DumpHandler.kt
@@ -25,6 +25,7 @@ import com.android.systemui.dump.DumpHandler.Companion.PRIORITY_ARG_CRITICAL
 import com.android.systemui.dump.DumpHandler.Companion.PRIORITY_ARG_HIGH
 import com.android.systemui.dump.DumpHandler.Companion.PRIORITY_ARG_NORMAL
 import com.android.systemui.log.LogBuffer
+import com.android.systemui.shared.system.UncaughtExceptionPreHandlerManager
 import java.io.PrintWriter
 import javax.inject.Inject
 import javax.inject.Provider
@@ -82,8 +83,20 @@ class DumpHandler @Inject constructor(
    private val context: Context,
    private val dumpManager: DumpManager,
    private val logBufferEulogizer: LogBufferEulogizer,
-    private val startables: MutableMap<Class<*>, Provider<CoreStartable>>
+    private val startables: MutableMap<Class<*>, Provider<CoreStartable>>,
+    private val uncaughtExceptionPreHandlerManager: UncaughtExceptionPreHandlerManager
 ) {
+    /**
+     * Registers an uncaught exception handler
+     */
+    fun init() {
+        uncaughtExceptionPreHandlerManager.registerHandler { _, e ->
+            if (e is Exception) {
+                logBufferEulogizer.record(e)
+            }
+        }
+    }
+
    /**
     * Dump the diagnostics! Behavior can be controlled via [args].
     */
--- a/packages/SystemUI/src/com/android/systemui/plugins/PluginsModule.java
+++ b/packages/SystemUI/src/com/android/systemui/plugins/PluginsModule.java
@@ -16,8 +16,6 @@

 package com.android.systemui.plugins;

-import static com.android.systemui.util.concurrency.GlobalConcurrencyModule.PRE_HANDLER;
-
 import android.app.NotificationManager;
 import android.content.Context;
 import android.content.pm.PackageManager;
@@ -32,12 +30,12 @@ import com.android.systemui.shared.plugins.PluginInstance;
 import com.android.systemui.shared.plugins.PluginManager;
 import com.android.systemui.shared.plugins.PluginManagerImpl;
 import com.android.systemui.shared.plugins.PluginPrefs;
+import com.android.systemui.shared.system.UncaughtExceptionPreHandlerManager;
 import com.android.systemui.util.concurrency.GlobalConcurrencyModule;
 import com.android.systemui.util.concurrency.ThreadFactory;

 import java.util.Arrays;
 import java.util.List;
-import java.util.Optional;
 import java.util.concurrent.Executor;

 import javax.inject.Named;
@@ -107,13 +105,12 @@ public abstract class PluginsModule {
            Context context,
            PluginActionManager.Factory instanceManagerFactory,
            @Named(PLUGIN_DEBUG) boolean debug,
-            @Named(PRE_HANDLER)
-                    Optional<Thread.UncaughtExceptionHandler> uncaughtExceptionHandlerOptional,
+            UncaughtExceptionPreHandlerManager preHandlerManager,
            PluginEnabler pluginEnabler,
            PluginPrefs pluginPrefs,
            @Named(PLUGIN_PRIVILEGED) List<String> privilegedPlugins) {
        return new PluginManagerImpl(context, instanceManagerFactory, debug,
-                uncaughtExceptionHandlerOptional, pluginEnabler, pluginPrefs,
+                preHandlerManager, pluginEnabler, pluginPrefs,
                privilegedPlugins);
    }

--- a/packages/SystemUI/src/com/android/systemui/util/concurrency/GlobalConcurrencyModule.java
+++ b/packages/SystemUI/src/com/android/systemui/util/concurrency/GlobalConcurrencyModule.java
@@ -23,11 +23,9 @@ import android.os.Looper;
 import com.android.systemui.dagger.qualifiers.Main;
 import com.android.systemui.dagger.qualifiers.UiBackground;

-import java.util.Optional;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;

-import javax.inject.Named;
 import javax.inject.Singleton;

 import dagger.Binds;
@@ -110,11 +108,4 @@ public abstract class GlobalConcurrencyModule {
    @Binds
    @Singleton
    public abstract Execution provideExecution(ExecutionImpl execution);
-
-    /** */
-    @Provides
-    @Named(PRE_HANDLER)
-    public static Optional<Thread.UncaughtExceptionHandler> providesUncaughtExceptionHandler() {
-        return Optional.ofNullable(Thread.getUncaughtExceptionPreHandler());
-    }
 }
--- a/packages/SystemUI/tests/src/com/android/systemui/dump/DumpHandlerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/dump/DumpHandlerTest.kt
@@ -20,6 +20,7 @@ import androidx.test.filters.SmallTest
 import com.android.systemui.Dumpable
 import com.android.systemui.SysuiTestCase
 import com.android.systemui.log.LogBuffer
+import com.android.systemui.shared.system.UncaughtExceptionPreHandlerManager
 import com.android.systemui.util.mockito.any
 import org.junit.Before
 import org.junit.Test
@@ -37,6 +38,8 @@ class DumpHandlerTest : SysuiTestCase() {

    @Mock
    private lateinit var logBufferEulogizer: LogBufferEulogizer
+    @Mock
+    private lateinit var exceptionHandlerManager: UncaughtExceptionPreHandlerManager

    @Mock
    private lateinit var pw: PrintWriter
@@ -59,7 +62,13 @@ class DumpHandlerTest : SysuiTestCase() {
    fun setUp() {
        MockitoAnnotations.initMocks(this)

-        dumpHandler = DumpHandler(mContext, dumpManager, logBufferEulogizer, mutableMapOf())
+        dumpHandler = DumpHandler(
+            mContext,
+            dumpManager,
+            logBufferEulogizer,
+            mutableMapOf(),
+            exceptionHandlerManager
+        )
    }

    @Test
--- a/packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginManagerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginManagerTest.java
@@ -15,6 +15,7 @@ package com.android.systemui.shared.plugins;

 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -27,65 +28,62 @@ import android.content.Intent;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
 import android.net.Uri;
-import android.test.suitebuilder.annotation.SmallTest;
 import android.testing.AndroidTestingRunner;
 import android.testing.TestableLooper.RunWithLooper;

+import androidx.test.filters.SmallTest;
+
 import com.android.internal.messages.nano.SystemMessageProto.SystemMessage;
 import com.android.systemui.SysuiTestCase;
 import com.android.systemui.plugins.Plugin;
 import com.android.systemui.plugins.PluginListener;
 import com.android.systemui.plugins.annotations.ProvidesInterface;
+import com.android.systemui.shared.system.UncaughtExceptionPreHandlerManager;

 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
 import org.mockito.Mockito;
+import org.mockito.MockitoAnnotations;

 import java.lang.Thread.UncaughtExceptionHandler;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Optional;
+import java.util.List;

@SmallTest
@RunWith(AndroidTestingRunner.class)
@RunWithLooper
 public class PluginManagerTest extends SysuiTestCase {
-
    private static final String PRIVILEGED_PACKAGE = "com.android.systemui";

-    private PluginActionManager.Factory mMockFactory;
-    private PluginActionManager<TestPlugin> mMockPluginInstance;
-    private PluginManagerImpl mPluginManager;
-    private PluginListener<TestPlugin> mMockListener;
-    private PackageManager mMockPackageManager;
-    private PluginEnabler mPluginEnabler;
-    private PluginPrefs mPluginPrefs;
+    @Mock PluginActionManager.Factory mMockFactory;
+    @Mock PluginActionManager<TestPlugin> mMockPluginInstance;
+    @Mock PluginListener<TestPlugin> mMockListener;
+    @Mock PackageManager mMockPackageManager;
+    @Mock PluginEnabler mMockPluginEnabler;
+    @Mock PluginPrefs mMockPluginPrefs;
+    @Mock UncaughtExceptionPreHandlerManager mMockExPreHandlerManager;

-    private UncaughtExceptionHandler mRealExceptionHandler;
-    private UncaughtExceptionHandler mMockExceptionHandler;
+    @Captor ArgumentCaptor<UncaughtExceptionHandler> mExPreHandlerCaptor;
+
+    private PluginManagerImpl mPluginManager;
    private UncaughtExceptionHandler mPluginExceptionHandler;

    @Before
    public void setup() throws Exception {
-        mRealExceptionHandler = Thread.getUncaughtExceptionPreHandler();
-        mMockExceptionHandler = mock(UncaughtExceptionHandler.class);
-        mMockFactory = mock(PluginActionManager.Factory.class);
-        mMockPluginInstance = mock(PluginActionManager.class);
-        mPluginEnabler = mock(PluginEnabler.class);
-        mPluginPrefs = mock(PluginPrefs.class);
+        MockitoAnnotations.initMocks(this);
        when(mMockFactory.create(any(), any(), eq(TestPlugin.class), anyBoolean(), anyBoolean()))
                .thenReturn(mMockPluginInstance);

-        mMockPackageManager = mock(PackageManager.class);
        mPluginManager = new PluginManagerImpl(
                getContext(), mMockFactory, true,
-                Optional.of(mMockExceptionHandler), mPluginEnabler,
-                mPluginPrefs, new ArrayList<>());
-
-        resetExceptionHandler();
-        mMockListener = mock(PluginListener.class);
+                mMockExPreHandlerManager, mMockPluginEnabler,
+                mMockPluginPrefs, new ArrayList<>());
+        captureExceptionHandler();
    }

    @Test
@@ -108,9 +106,9 @@ public class PluginManagerTest extends SysuiTestCase {
    public void testNonDebuggable_nonPrivileged() {
        mPluginManager = new PluginManagerImpl(
                getContext(), mMockFactory, false,
-                Optional.of(mMockExceptionHandler), mPluginEnabler,
-                mPluginPrefs, new ArrayList<>());
-        resetExceptionHandler();
+                mMockExPreHandlerManager, mMockPluginEnabler,
+                mMockPluginPrefs, new ArrayList<>());
+        captureExceptionHandler();

        String sourceDir = "myPlugin";
        ApplicationInfo applicationInfo = new ApplicationInfo();
@@ -127,9 +125,9 @@ public class PluginManagerTest extends SysuiTestCase {
    public void testNonDebuggable_privilegedPackage() {
        mPluginManager = new PluginManagerImpl(
                getContext(), mMockFactory, false,
-                Optional.of(mMockExceptionHandler), mPluginEnabler,
-                mPluginPrefs, Collections.singletonList(PRIVILEGED_PACKAGE));
-        resetExceptionHandler();
+                mMockExPreHandlerManager, mMockPluginEnabler,
+                mMockPluginPrefs, Collections.singletonList(PRIVILEGED_PACKAGE));
+        captureExceptionHandler();

        String sourceDir = "myPlugin";
        ApplicationInfo privilegedApplicationInfo = new ApplicationInfo();
@@ -154,9 +152,6 @@ public class PluginManagerTest extends SysuiTestCase {
        verify(mMockPluginInstance, Mockito.atLeastOnce()).checkAndDisable(
                ArgumentCaptor.forClass(String.class).capture());
        verify(mMockPluginInstance, Mockito.never()).disableAll();
-        verify(mMockExceptionHandler).uncaughtException(
-                ArgumentCaptor.forClass(Thread.class).capture(),
-                ArgumentCaptor.forClass(Throwable.class).capture());
    }

    @Test
@@ -169,9 +164,6 @@ public class PluginManagerTest extends SysuiTestCase {
        verify(mMockPluginInstance, Mockito.atLeastOnce()).checkAndDisable(
                ArgumentCaptor.forClass(String.class).capture());
        verify(mMockPluginInstance).disableAll();
-        verify(mMockExceptionHandler).uncaughtException(
-                ArgumentCaptor.forClass(Thread.class).capture(),
-                ArgumentCaptor.forClass(Throwable.class).capture());
    }

    @Test
@@ -186,13 +178,15 @@ public class PluginManagerTest extends SysuiTestCase {
        intent.setData(Uri.parse("package://" + testComponent.flattenToString()));
        mPluginManager.onReceive(mContext, intent);
        verify(nm).cancel(eq(testComponent.getClassName()), eq(SystemMessage.NOTE_PLUGIN));
-        verify(mPluginEnabler).setDisabled(testComponent, PluginEnabler.DISABLED_INVALID_VERSION);
+        verify(mMockPluginEnabler).setDisabled(testComponent,
+                PluginEnabler.DISABLED_INVALID_VERSION);
    }

-    private void resetExceptionHandler() {
-        mPluginExceptionHandler = Thread.getUncaughtExceptionPreHandler();
-        // Set back the real exception handler so the test can crash if it wants to.
-        Thread.setUncaughtExceptionPreHandler(mRealExceptionHandler);
+    private void captureExceptionHandler() {
+        verify(mMockExPreHandlerManager, atLeastOnce()).registerHandler(
+                mExPreHandlerCaptor.capture());
+        List<UncaughtExceptionHandler> allValues = mExPreHandlerCaptor.getAllValues();
+        mPluginExceptionHandler = allValues.get(allValues.size() - 1);
    }

    @ProvidesInterface(action = TestPlugin.ACTION, version = TestPlugin.VERSION)
--- a/packages/SystemUI/tests/src/com/android/systemui/shared/system/UncaughtExceptionPreHandlerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/shared/system/UncaughtExceptionPreHandlerTest.kt
@@ -0,0 +1,74 @@
+package com.android.systemui.shared.system
+
+import androidx.test.filters.SmallTest
+import com.android.systemui.SysuiTestCase
+import com.google.common.truth.Truth.assertThat
+import org.junit.Assert.assertThrows
+import org.junit.Before
+import org.junit.Test
+import org.mockito.Mock
+import org.mockito.Mockito.only
+import org.mockito.Mockito.any
+import org.mockito.Mockito.verify
+import org.mockito.Mockito.`when`
+import org.mockito.MockitoAnnotations
+import java.lang.Thread.UncaughtExceptionHandler
+
+@SmallTest
+class UncaughtExceptionPreHandlerTest : SysuiTestCase() {
+    private lateinit var preHandlerManager: UncaughtExceptionPreHandlerManager
+
+    @Mock
+    private lateinit var mockHandler: UncaughtExceptionHandler
+
+    @Mock
+    private lateinit var mockHandler2: UncaughtExceptionHandler
+
+    @Before
+    fun setUp() {
+        MockitoAnnotations.initMocks(this)
+        Thread.setUncaughtExceptionPreHandler(null)
+        preHandlerManager = UncaughtExceptionPreHandlerManager()
+    }
+
+    @Test
+    fun registerHandler_registersOnceOnly() {
+        preHandlerManager.registerHandler(mockHandler)
+        preHandlerManager.registerHandler(mockHandler)
+        preHandlerManager.handleUncaughtException(Thread.currentThread(), Exception())
+        verify(mockHandler, only()).uncaughtException(any(), any())
+    }
+
+    @Test
+    fun registerHandler_setsUncaughtExceptionPreHandler() {
+        Thread.setUncaughtExceptionPreHandler(null)
+        preHandlerManager.registerHandler(mockHandler)
+        assertThat(Thread.getUncaughtExceptionPreHandler()).isNotNull()
+    }
+
+    @Test
+    fun registerHandler_preservesOriginalHandler() {
+        Thread.setUncaughtExceptionPreHandler(mockHandler)
+        preHandlerManager.registerHandler(mockHandler2)
+        preHandlerManager.handleUncaughtException(Thread.currentThread(), Exception())
+        verify(mockHandler, only()).uncaughtException(any(), any())
+    }
+
+    @Test
+    fun registerHandler_toleratesHandlersThatThrow() {
+        `when`(mockHandler2.uncaughtException(any(), any())).thenThrow(RuntimeException())
+        preHandlerManager.registerHandler(mockHandler2)
+        preHandlerManager.registerHandler(mockHandler)
+        preHandlerManager.handleUncaughtException(Thread.currentThread(), Exception())
+        verify(mockHandler2, only()).uncaughtException(any(), any())
+        verify(mockHandler, only()).uncaughtException(any(), any())
+    }
+
+    @Test
+    fun registerHandler_doesNotSetUpTwice() {
+        UncaughtExceptionPreHandlerManager().registerHandler(mockHandler2)
+        assertThrows(IllegalStateException::class.java) {
+            preHandlerManager.registerHandler(mockHandler)
+        }
+    }
+}