From 9ae7ba267da4f6bf098bc3dd1ea0edc6aea59894 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sun, 12 Jun 2022 13:17:38 -0400 Subject: [PATCH 5/5] ShortcutPackage: Acquire the service's lock before reporting usage When `ShortcutPackage.pushDynamicShortcut()` is called from `ShortcutService.pushDynamicShortcut()`, the lock `ShortcutService.mLock` is acquired. However, the final `injectPostToHandler()` call drops this lock before calling `awaitInAppSearch()`. The `awaitInAppSearch()` method assumes that it is called while being guarded by the `ShortcutService` lock. When called without acquiring the lock, it will end up acquiring `ShortcutPackage.mLock` first, then acquiring `ShortcutService.mLock` anyway due to various calls into `ShortcutService` methods. This creates an opportunity for a dead lock to happen, since two threads can wait on each other for releasing the other lock they are currently trying to acquire. On one of my development devices, this happens quite regularly: > "android.bg" prio=5 tid=22 Blocked > | group="main" sCount=1 ucsCount=0 flags=1 obj=0x16040c48 self=0xb40000709b1a3c00 > | sysTid=1515 nice=10 cgrp=default sched=0/0 handle=0x70904ebcb0 > | state=S schedstat=( 14737747935 45952650258 46431 ) utm=610 stm=862 core=0 HZ=100 > | stack=0x70903e8000-0x70903ea000 stackSize=1039KB > | held mutexes= > at com.android.server.pm.ShortcutService.scheduleSaveInner(ShortcutService.java:1176) > - waiting to lock <0x0a5766e4> (a java.lang.Object) held by thread 122 > at com.android.server.pm.ShortcutService.scheduleSaveUser(ShortcutService.java:1166) > at com.android.server.pm.ShortcutService.packageShortcutsChanged(ShortcutService.java:1769) > at com.android.server.pm.ShortcutPackage.rescanPackage(ShortcutPackage.java:1224) > at com.android.server.pm.ShortcutPackage.awaitInAppSearch(ShortcutPackage.java:2627) > - locked <0x00e8774d> (a java.lang.Object) > at com.android.server.pm.ShortcutPackage.awaitInAppSearch(ShortcutPackage.java:2593) > at com.android.server.pm.ShortcutPackage.lambda$pushDynamicShortcut$3$ShortcutPackage(ShortcutPackage.java:452) > at com.android.server.pm.ShortcutPackage$$ExternalSyntheticLambda3.run(unavailable:-1) > at android.os.Handler.handleCallback(Handler.java:938) > at android.os.Handler.dispatchMessage(Handler.java:99) > at android.os.Looper.loopOnce(Looper.java:201) > at android.os.Looper.loop(Looper.java:288) > at android.os.HandlerThread.run(HandlerThread.java:67) Meanwhile, in thread 122: > "Binder:1489_6" prio=5 tid=122 Blocked > | group="main" sCount=1 ucsCount=0 flags=1 obj=0x16047388 self=0xb400007074947c00 > | sysTid=2070 nice=0 cgrp=default sched=0/0 handle=0x70218e4cb0 > | state=S schedstat=( 10396689438 3745391103 31237 ) utm=808 stm=231 core=1 HZ=100 > | stack=0x70217ed000-0x70217ef000 stackSize=991KB > | held mutexes= > at com.android.server.pm.ShortcutPackage.awaitInAppSearch(ShortcutPackage.java:2605) > - waiting to lock <0x00e8774d> (a java.lang.Object) held by thread 22 > at com.android.server.pm.ShortcutPackage.awaitInAppSearch(ShortcutPackage.java:2593) > at com.android.server.pm.ShortcutPackage.forEachShortcutStopWhen(ShortcutPackage.java:2542) > at com.android.server.pm.ShortcutPackage.forEachShortcutStopWhen(ShortcutPackage.java:2524) > at com.android.server.pm.ShortcutPackage.areAllActivitiesStillEnabled(ShortcutPackage.java:1039) > at com.android.server.pm.ShortcutPackage.rescanPackageIfNeeded(ShortcutPackage.java:1096) > at com.android.server.pm.ShortcutUser.rescanPackageIfNeeded(ShortcutUser.java:338) > at com.android.server.pm.ShortcutUser.onCalledByPublisher(ShortcutUser.java:294) > at com.android.server.pm.ShortcutService.getPackageShortcutsForPublisherLocked(ShortcutService.java:1355) > at com.android.server.pm.ShortcutService.lambda$pushDynamicShortcut$9$ShortcutService(ShortcutService.java:2210) > - locked <0x0a5766e4> (a java.lang.Object) > at com.android.server.pm.ShortcutService$$ExternalSyntheticLambda14.run(unavailable:-1) > at com.android.server.pm.ShortcutService.injectPostToHandlerIfAppSearch(ShortcutService.java:1740) > at com.android.server.pm.ShortcutService.pushDynamicShortcut(ShortcutService.java:2200) > at android.content.pm.IShortcutService$Stub.onTransact(IShortcutService.java:715) > at android.os.Binder.execTransactInternal(Binder.java:1201) > at android.os.Binder.execTransact(Binder.java:1164) The straightforward fix here seems to be just taking the `ShortcutService` lock before calling `awaitInAppSearch()`. This commit also added a method in `ShortcutService` to facilitate this interaction. Test: manual Change-Id: I0f997a02820b1598ca2a4ec25415c03f8f87fc87 --- .../core/java/com/android/server/pm/ShortcutPackage.java | 2 +- .../core/java/com/android/server/pm/ShortcutService.java | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java index b4bd086af272..d4f5a237f250 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackage.java +++ b/services/core/java/com/android/server/pm/ShortcutPackage.java @@ -449,7 +449,7 @@ class ShortcutPackage extends ShortcutPackageItem { forceReplaceShortcutInner(newShortcut); if (isAppSearchEnabled()) { - mShortcutUser.mService.injectPostToHandler(() -> awaitInAppSearch("reportUsage", + mShortcutUser.mService.injectPostToHandlerWithLock(() -> awaitInAppSearch("reportUsage", session -> { final AndroidFuture future = new AndroidFuture<>(); session.reportUsage( diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java index 4e9e7a026a90..c688943d2838 100644 --- a/services/core/java/com/android/server/pm/ShortcutService.java +++ b/services/core/java/com/android/server/pm/ShortcutService.java @@ -1731,6 +1731,14 @@ public class ShortcutService extends IShortcutService.Stub { mHandler.post(r); } + void injectPostToHandlerWithLock(Runnable r) { + injectPostToHandler(() -> { + synchronized (mLock) { + r.run(); + } + }); + } + void injectRunOnNewThread(Runnable r) { new Thread(r).start(); } -- 2.36.1