diff options
9 files changed, 212 insertions, 77 deletions
diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginManagerImpl.java b/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginManagerImpl.java index 851ea6558697..131f728be3f1 100644 --- 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) { diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/system/UncaughtExceptionPreHandlerManager.kt b/packages/SystemUI/shared/src/com/android/systemui/shared/system/UncaughtExceptionPreHandlerManager.kt new file mode 100644 index 000000000000..621778b55764 --- /dev/null +++ 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) + } + } +}
\ No newline at end of file diff --git a/packages/SystemUI/src/com/android/systemui/SystemUIService.java b/packages/SystemUI/src/com/android/systemui/SystemUIService.java index 8ffa9b470cf7..7bcba3cc1c46 100644 --- 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)) { diff --git a/packages/SystemUI/src/com/android/systemui/dump/DumpHandler.kt b/packages/SystemUI/src/com/android/systemui/dump/DumpHandler.kt index 7cc33a9d4135..08ef8f3d025f 100644 --- 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,9 +83,21 @@ 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]. */ fun dump(pw: PrintWriter, args: Array<String>) { diff --git a/packages/SystemUI/src/com/android/systemui/plugins/PluginsModule.java b/packages/SystemUI/src/com/android/systemui/plugins/PluginsModule.java index 16b971f876bc..638f81b71a34 100644 --- 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); } diff --git a/packages/SystemUI/src/com/android/systemui/util/concurrency/GlobalConcurrencyModule.java b/packages/SystemUI/src/com/android/systemui/util/concurrency/GlobalConcurrencyModule.java index bc9e5963c22d..e40d2766287d 100644 --- 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()); - } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/dump/DumpHandlerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/dump/DumpHandlerTest.kt index 871c3a839c7a..fc672016a886 100644 --- 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 diff --git a/packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginManagerTest.java index 1eadd522352e..40c84d611198 100644 --- 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) diff --git a/packages/SystemUI/tests/src/com/android/systemui/shared/system/UncaughtExceptionPreHandlerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shared/system/UncaughtExceptionPreHandlerTest.kt new file mode 100644 index 000000000000..f9e279eb5871 --- /dev/null +++ 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) + } + } +}
\ No newline at end of file |