From 0e2578370d37e7416aac2b8c193268113480a34f Mon Sep 17 00:00:00 2001 From: Jernej Virag Date: Wed, 1 Feb 2023 11:18:41 +0100 Subject: [PATCH] Fix SysUI crash when memory collection times out runBlocking for coroutines will throw InterruptedException when its thread is interrupted. StatsD does that when wall clock time for collection exceeds the timeout. Don't allow this exception to crash SysUI and correctly signal statsd to discard the result. Bug: 267162163 Bug: 267355629 Test: wrote new unit test cases, verified them on raven-userdebug Change-Id: I2e7a659ed5d33df7d543a935617b1a97213c08b6 --- .../logging/NotificationLogger.java | 2 +- .../logging/NotificationMemoryLogger.kt | 85 +++++++++++-------- .../logging/NotificationMemoryViewWalker.kt | 18 ++-- .../logging/NotificationMemoryLoggerTest.kt | 19 +++++ 4 files changed, 80 insertions(+), 44 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java index 58f59be1db6fc..a37bbbcda555b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java @@ -62,7 +62,7 @@ import javax.inject.Inject; * are not. */ public class NotificationLogger implements StateListener { - private static final String TAG = "NotificationLogger"; + static final String TAG = "NotificationLogger"; private static final boolean DEBUG = Compile.IS_DEBUG && Log.isLoggable(TAG, Log.DEBUG); /** The minimum delay in ms between reports of notification visibility. */ diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryLogger.kt index ec8501a79fa5c..cc1103de8ec05 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryLogger.kt @@ -18,6 +18,7 @@ package com.android.systemui.statusbar.notification.logging import android.app.StatsManager +import android.util.Log import android.util.StatsEvent import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dagger.qualifiers.Background @@ -25,6 +26,7 @@ import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.shared.system.SysUiStatsLog import com.android.systemui.statusbar.notification.collection.NotifPipeline import com.android.systemui.util.traceSection +import java.lang.Exception import java.util.concurrent.Executor import javax.inject.Inject import kotlin.math.roundToInt @@ -82,43 +84,56 @@ constructor( return StatsManager.PULL_SKIP } - // Notifications can only be retrieved on the main thread, so switch to that thread. - val notifications = getAllNotificationsOnMainThread() - val notificationMemoryUse = - NotificationMemoryMeter.notificationMemoryUse(notifications) - .sortedWith( - compareBy( - { it.packageName }, - { it.objectUsage.style }, - { it.notificationKey } + try { + // Notifications can only be retrieved on the main thread, so switch to that thread. + val notifications = getAllNotificationsOnMainThread() + val notificationMemoryUse = + NotificationMemoryMeter.notificationMemoryUse(notifications) + .sortedWith( + compareBy( + { it.packageName }, + { it.objectUsage.style }, + { it.notificationKey } + ) + ) + val usageData = aggregateMemoryUsageData(notificationMemoryUse) + usageData.forEach { (_, use) -> + data.add( + SysUiStatsLog.buildStatsEvent( + SysUiStatsLog.NOTIFICATION_MEMORY_USE, + use.uid, + use.style, + use.count, + use.countWithInflatedViews, + toKb(use.smallIconObject), + use.smallIconBitmapCount, + toKb(use.largeIconObject), + use.largeIconBitmapCount, + toKb(use.bigPictureObject), + use.bigPictureBitmapCount, + toKb(use.extras), + toKb(use.extenders), + toKb(use.smallIconViews), + toKb(use.largeIconViews), + toKb(use.systemIconViews), + toKb(use.styleViews), + toKb(use.customViews), + toKb(use.softwareBitmaps), + use.seenCount ) ) - val usageData = aggregateMemoryUsageData(notificationMemoryUse) - usageData.forEach { (_, use) -> - data.add( - SysUiStatsLog.buildStatsEvent( - SysUiStatsLog.NOTIFICATION_MEMORY_USE, - use.uid, - use.style, - use.count, - use.countWithInflatedViews, - toKb(use.smallIconObject), - use.smallIconBitmapCount, - toKb(use.largeIconObject), - use.largeIconBitmapCount, - toKb(use.bigPictureObject), - use.bigPictureBitmapCount, - toKb(use.extras), - toKb(use.extenders), - toKb(use.smallIconViews), - toKb(use.largeIconViews), - toKb(use.systemIconViews), - toKb(use.styleViews), - toKb(use.customViews), - toKb(use.softwareBitmaps), - use.seenCount - ) - ) + } + } catch (e: InterruptedException) { + // This can happen if the device is sleeping or view walking takes too long. + // The statsd collector will interrupt the thread and we need to handle it + // gracefully. + Log.w(NotificationLogger.TAG, "Timed out when measuring notification memory.", e) + return@traceSection StatsManager.PULL_SKIP + } catch (e: Exception) { + // Error while collecting data, this should not crash prod SysUI. Just + // log WTF and move on. + Log.wtf(NotificationLogger.TAG, "Failed to measure notification memory.", e) + return@traceSection StatsManager.PULL_SKIP } return StatsManager.PULL_SUCCESS diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryViewWalker.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryViewWalker.kt index 2d042118b3d7e..6491223e6e102 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryViewWalker.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryViewWalker.kt @@ -184,19 +184,21 @@ internal object NotificationMemoryViewWalker { private fun computeDrawableUse(drawable: Drawable, seenObjects: HashSet): Int = when (drawable) { is BitmapDrawable -> { - val ref = System.identityHashCode(drawable.bitmap) - if (seenObjects.contains(ref)) { - 0 - } else { - seenObjects.add(ref) - drawable.bitmap.allocationByteCount - } + drawable.bitmap?.let { + val ref = System.identityHashCode(it) + if (seenObjects.contains(ref)) { + 0 + } else { + seenObjects.add(ref) + it.allocationByteCount + } + } ?: 0 } else -> 0 } private fun isDrawableSoftwareBitmap(drawable: Drawable) = - drawable is BitmapDrawable && drawable.bitmap.config != Bitmap.Config.HARDWARE + drawable is BitmapDrawable && drawable.bitmap?.config != Bitmap.Config.HARDWARE private fun identifierForView(view: View) = if (view.id == View.NO_ID) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryLoggerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryLoggerTest.kt index 33b94e39c0196..bd039031cecc6 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryLoggerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryLoggerTest.kt @@ -32,6 +32,7 @@ import com.android.systemui.util.mockito.mock import com.android.systemui.util.mockito.whenever import com.android.systemui.util.time.FakeSystemClock import com.google.common.truth.Truth.assertThat +import java.lang.RuntimeException import kotlinx.coroutines.Dispatchers import org.junit.Before import org.junit.Test @@ -113,6 +114,24 @@ class NotificationMemoryLoggerTest : SysuiTestCase() { assertThat(data).hasSize(2) } + @Test + fun onPullAtom_throwsInterruptedException_failsGracefully() { + val pipeline: NotifPipeline = mock() + whenever(pipeline.allNotifs).thenAnswer { throw InterruptedException("Timeout") } + val logger = NotificationMemoryLogger(pipeline, statsManager, immediate, bgExecutor) + assertThat(logger.onPullAtom(SysUiStatsLog.NOTIFICATION_MEMORY_USE, mutableListOf())) + .isEqualTo(StatsManager.PULL_SKIP) + } + + @Test + fun onPullAtom_throwsRuntimeException_failsGracefully() { + val pipeline: NotifPipeline = mock() + whenever(pipeline.allNotifs).thenThrow(RuntimeException("Something broke!")) + val logger = NotificationMemoryLogger(pipeline, statsManager, immediate, bgExecutor) + assertThat(logger.onPullAtom(SysUiStatsLog.NOTIFICATION_MEMORY_USE, mutableListOf())) + .isEqualTo(StatsManager.PULL_SKIP) + } + private fun createLoggerWithNotifications( notifications: List ): NotificationMemoryLogger {