From 51f38fb6f3858b1431eb63586079a4550795721c Mon Sep 17 00:00:00 2001 From: Pascal Muetschard Date: Tue, 23 Aug 2022 12:40:59 +0200 Subject: [PATCH 1/2] Check the DeviceConfig permission early. This causes the permission error to be thrown from the thread that creates the JankMonitor rather than some background thread. This makes it easier to debug the permission error and can prevent crashes due to unhandled exceptions from background threads. Change-Id: I942c705342962ce6e15ae5cd6041dc85f6f83a95 --- .../com/android/internal/jank/InteractionJankMonitor.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/java/com/android/internal/jank/InteractionJankMonitor.java b/core/java/com/android/internal/jank/InteractionJankMonitor.java index 72de78c148f8c..6e8a0c734caca 100644 --- a/core/java/com/android/internal/jank/InteractionJankMonitor.java +++ b/core/java/com/android/internal/jank/InteractionJankMonitor.java @@ -87,6 +87,7 @@ import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.UiThread; import android.annotation.WorkerThread; +import android.app.ActivityThread; import android.content.Context; import android.os.Build; import android.os.Handler; @@ -402,6 +403,11 @@ public class InteractionJankMonitor { */ @VisibleForTesting public InteractionJankMonitor(@NonNull HandlerThread worker) { + // Check permission early. + DeviceConfig.enforceReadPermission( + ActivityThread.currentApplication().getApplicationContext(), + DeviceConfig.NAMESPACE_INTERACTION_JANK_MONITOR); + mRunningTrackers = new SparseArray<>(); mTimeoutActions = new SparseArray<>(); mWorker = worker; From 24a356607cd3f7acb916a3453aae90c9f73c3db1 Mon Sep 17 00:00:00 2001 From: Pascal Muetschard Date: Wed, 17 Aug 2022 17:40:30 +0200 Subject: [PATCH 2/2] DCL is broken and should not be used. Use the acceptable instance holder pattern as a replacement. Change-Id: I2fb29edd48e00ac36a7fb0907a21ae5ca1d5163f --- config/preloaded-classes-denylist | 1 + .../internal/jank/InteractionJankMonitor.java | 15 +++++---------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/config/preloaded-classes-denylist b/config/preloaded-classes-denylist index 02f2df6167a57..502d8c6dadb12 100644 --- a/config/preloaded-classes-denylist +++ b/config/preloaded-classes-denylist @@ -9,3 +9,4 @@ android.net.rtp.AudioGroup android.net.rtp.AudioStream android.net.rtp.RtpStream java.util.concurrent.ThreadLocalRandom +com.android.internal.jank.InteractionJankMonitor$InstanceHolder diff --git a/core/java/com/android/internal/jank/InteractionJankMonitor.java b/core/java/com/android/internal/jank/InteractionJankMonitor.java index 6e8a0c734caca..fc4e041058b1d 100644 --- a/core/java/com/android/internal/jank/InteractionJankMonitor.java +++ b/core/java/com/android/internal/jank/InteractionJankMonitor.java @@ -293,7 +293,10 @@ public class InteractionJankMonitor { UIINTERACTION_FRAME_INFO_REPORTED__INTERACTION_TYPE__SHADE_CLEAR_ALL, }; - private static volatile InteractionJankMonitor sInstance; + private static class InstanceHolder { + public static final InteractionJankMonitor INSTANCE = + new InteractionJankMonitor(new HandlerThread(DEFAULT_WORKER_NAME)); + } private final DeviceConfig.OnPropertiesChangedListener mPropertiesChangedListener = this::updateProperties; @@ -385,15 +388,7 @@ public class InteractionJankMonitor { * @return instance of InteractionJankMonitor */ public static InteractionJankMonitor getInstance() { - // Use DCL here since this method might be invoked very often. - if (sInstance == null) { - synchronized (InteractionJankMonitor.class) { - if (sInstance == null) { - sInstance = new InteractionJankMonitor(new HandlerThread(DEFAULT_WORKER_NAME)); - } - } - } - return sInstance; + return InstanceHolder.INSTANCE; } /**