From 30fed8c21b4954f9c3f92b665635b950ea737473 Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Thu, 1 Sep 2022 11:19:31 -0700 Subject: [PATCH] Fixes an edge case which leads to deadlock in ShortcutService ShortcutService uses two levels of synchronization lock, one on the service-level, another on package-level. To prevent deadlock, we made an implicit rule that a thread can never acquire service-level lock if they were holding onto package-level lock. An edge case emerges when we ran into "what a terrible failure", service-level lock was acquired to increase wtf counter and capture stack traces, regardless of the implicit rule mentioned above. This CL solves the deadlock by using a separate lock in wtf. Bug: 243880581 Test: manual Change-Id: I136ed4018b07ae17591207c5c855e9d34082ae77 --- .../android/server/pm/ShortcutService.java | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java index f2bcf5e461a77..0b20683185f01 100644 --- a/services/core/java/com/android/server/pm/ShortcutService.java +++ b/services/core/java/com/android/server/pm/ShortcutService.java @@ -280,6 +280,7 @@ public class ShortcutService extends IShortcutService.Stub { private final Object mLock = new Object(); private final Object mNonPersistentUsersLock = new Object(); + private final Object mWtfLock = new Object(); private static List EMPTY_RESOLVE_INFO = new ArrayList<>(0); @@ -444,10 +445,10 @@ public class ShortcutService extends IShortcutService.Stub { @interface ShortcutOperation { } - @GuardedBy("mLock") + @GuardedBy("mWtfLock") private int mWtfCount = 0; - @GuardedBy("mLock") + @GuardedBy("mWtfLock") private Exception mLastWtfStacktrace; @GuardedBy("mLock") @@ -4727,13 +4728,15 @@ public class ShortcutService extends IShortcutService.Stub { mStatLogger.dump(pw, " "); - pw.println(); - pw.print(" #Failures: "); - pw.println(mWtfCount); + synchronized (mWtfLock) { + pw.println(); + pw.print(" #Failures: "); + pw.println(mWtfCount); - if (mLastWtfStacktrace != null) { - pw.print(" Last failure stack trace: "); - pw.println(Log.getStackTraceString(mLastWtfStacktrace)); + if (mLastWtfStacktrace != null) { + pw.print(" Last failure stack trace: "); + pw.println(Log.getStackTraceString(mLastWtfStacktrace)); + } } pw.println(); @@ -5148,7 +5151,7 @@ public class ShortcutService extends IShortcutService.Stub { if (e == null) { e = new RuntimeException("Stacktrace"); } - synchronized (mLock) { + synchronized (mWtfLock) { mWtfCount++; mLastWtfStacktrace = new Exception("Last failure was logged here:"); }