From 50ddd5856b464e470920c67f101ddd72465eda44 Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Mon, 11 Jan 2021 17:00:29 -0800 Subject: [PATCH] AppWidgetServiceImpl: add mWidgetPackages lock JobScheduler calls isBoundWidgetPackage(), but this can be extremely slow while AppWidgetService is doing any meaningful work, causing problems for the rest of the system. Remove that serialization by moving mWidgetPackages behind its own lock. Test: atest CtsAppWidgetTestCases Bug: 161866124 Change-Id: Idae7d16d375dfd41f802ad193fa1ec939093c67a --- .../appwidget/AppWidgetServiceImpl.java | 45 +++++++++++-------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java b/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java index 449063d957706..d4bd5adb7b250 100644 --- a/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java +++ b/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java @@ -222,6 +222,7 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku private final SparseIntArray mLoadedUserIds = new SparseIntArray(); + private final Object mWidgetPackagesLock = new Object(); private final SparseArray> mWidgetPackages = new SparseArray<>(); private BackupRestoreController mBackupRestoreController; @@ -2941,11 +2942,13 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku if (widget.provider == null) return; int userId = widget.provider.getUserId(); - ArraySet packages = mWidgetPackages.get(userId); - if (packages == null) { - mWidgetPackages.put(userId, packages = new ArraySet()); + synchronized (mWidgetPackagesLock) { + ArraySet packages = mWidgetPackages.get(userId); + if (packages == null) { + mWidgetPackages.put(userId, packages = new ArraySet()); + } + packages.add(widget.provider.info.provider.getPackageName()); } - packages.add(widget.provider.info.provider.getPackageName()); // If we are adding a widget it might be for a provider that // is currently masked, if so mask the widget. @@ -2972,22 +2975,24 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku final int userId = widget.provider.getUserId(); final String packageName = widget.provider.info.provider.getPackageName(); - ArraySet packages = mWidgetPackages.get(userId); - if (packages == null) { - return; - } - // Check if there is any other widget with the same package name. - // Remove packageName if none. - final int N = mWidgets.size(); - for (int i = 0; i < N; i++) { - Widget w = mWidgets.get(i); - if (w.provider == null) continue; - if (w.provider.getUserId() == userId - && packageName.equals(w.provider.info.provider.getPackageName())) { + synchronized (mWidgetPackagesLock) { + ArraySet packages = mWidgetPackages.get(userId); + if (packages == null) { return; } + // Check if there is any other widget with the same package name. + // Remove packageName if none. + final int N = mWidgets.size(); + for (int i = 0; i < N; i++) { + Widget w = mWidgets.get(i); + if (w.provider == null) continue; + if (w.provider.getUserId() == userId + && packageName.equals(w.provider.info.provider.getPackageName())) { + return; + } + } + packages.remove(packageName); } - packages.remove(packageName); } /** @@ -3000,7 +3005,9 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku } private void onWidgetsClearedLocked() { - mWidgetPackages.clear(); + synchronized (mWidgetPackagesLock) { + mWidgetPackages.clear(); + } } @Override @@ -3008,7 +3015,7 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku if (Binder.getCallingUid() != Process.SYSTEM_UID) { throw new SecurityException("Only the system process can call this"); } - synchronized (mLock) { + synchronized (mWidgetPackagesLock) { final ArraySet packages = mWidgetPackages.get(userId); if (packages != null) { return packages.contains(packageName);