Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0e257837 authored by Jernej Virag's avatar Jernej Virag
Browse files

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
parent e37f841e

packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java

+1 −1
Original line number Diff line number Diff line
@@ -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. */

packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryLogger.kt

+50 −35
Original line number Diff line number Diff line
@@ -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,6 +84,7 @@ constructor( 
                return StatsManager.PULL_SKIP

            }



            try {

                // Notifications can only be retrieved on the main thread, so switch to that thread.

                val notifications = getAllNotificationsOnMainThread()

                val notificationMemoryUse =
@@ -120,6 +123,18 @@ constructor( 
                        )

                    )

                }

            } 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

        }

packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryViewWalker.kt

+10 −8
Original line number Diff line number Diff line
@@ -184,19 +184,21 @@ internal object NotificationMemoryViewWalker { 
    private fun computeDrawableUse(drawable: Drawable, seenObjects: HashSet<Int>): Int =

        when (drawable) {

            is BitmapDrawable -> {

                val ref = System.identityHashCode(drawable.bitmap)

                drawable.bitmap?.let {

                    val ref = System.identityHashCode(it)

                    if (seenObjects.contains(ref)) {

                        0

                    } else {

                        seenObjects.add(ref)

                    drawable.bitmap.allocationByteCount

                        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) {

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationMemoryLoggerTest.kt

+19 −0
Original line number Diff line number Diff line
@@ -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<Notification>

    ): NotificationMemoryLogger {