Fix widget manager / alarm manager deadlock

Don't hold locks while manipulating alarms within the app widget
server, because the joint dependencies on activity manager locking
cause problems.

Change-Id: Ic7a09e6938277df7201be515f9785f326552f166
Bug: 115894687
Test: manual (repeated rebooting including with alarms forced)

diff --git a/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java b/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java
index b71d7a7..da52d40 100644 (file)
--- a/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java
+++ b/services/appwidget/java/com/android/server/appwidget/AppWidgetServiceImpl.java
@@ -1834,7 +1834,7 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku
 
                 if (provider.widgets.isEmpty()) {
                     // cancel the future updates
-                    cancelBroadcasts(provider);
+                    cancelBroadcastsLocked(provider);
 
                     // send the broacast saying that the provider is not in use any more
                     sendDisabledIntentLocked(provider);
@@ -1843,18 +1843,16 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku
         }
     }
 
-    private void cancelBroadcasts(Provider provider) {
+    private void cancelBroadcastsLocked(Provider provider) {
         if (DEBUG) {
-            Slog.i(TAG, "cancelBroadcasts() for " + provider);
+            Slog.i(TAG, "cancelBroadcastsLocked() for " + provider);
         }
         if (provider.broadcast != null) {
-            mAlarmManager.cancel(provider.broadcast);
-            long token = Binder.clearCallingIdentity();
-            try {
-                provider.broadcast.cancel();
-            } finally {
-                Binder.restoreCallingIdentity(token);
-            }
+            final PendingIntent broadcast = provider.broadcast;
+            mSaveStateHandler.post(() -> {
+                    mAlarmManager.cancel(broadcast);
+                    broadcast.cancel();
+            });
             provider.broadcast = null;
         }
     }
@@ -2315,7 +2313,7 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku
         mProviders.remove(provider);
 
         // no need to send the DISABLE broadcast, since the receiver is gone anyway
-        cancelBroadcasts(provider);
+        cancelBroadcastsLocked(provider);
     }
 
     private void sendEnableIntentLocked(Provider p) {
@@ -2369,17 +2367,14 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku
                 Binder.restoreCallingIdentity(token);
             }
             if (!alreadyRegistered) {
-                long period = provider.info.updatePeriodMillis;
-                if (period < MIN_UPDATE_PERIOD) {
-                    period = MIN_UPDATE_PERIOD;
-                }
-                final long oldId = Binder.clearCallingIdentity();
-                try {
+                // Set the alarm outside of our locks; we've latched the first-time
+                // invariant and established the PendingIntent safely.
+                final long period = Math.max(provider.info.updatePeriodMillis, MIN_UPDATE_PERIOD);
+                final PendingIntent broadcast = provider.broadcast;
+                mSaveStateHandler.post(() ->
                     mAlarmManager.setInexactRepeating(AlarmManager.ELAPSED_REALTIME_WAKEUP,
-                            SystemClock.elapsedRealtime() + period, period, provider.broadcast);
-                } finally {
-                    Binder.restoreCallingIdentity(oldId);
-                }
+                            SystemClock.elapsedRealtime() + period, period, broadcast)
+                );
             }
         }
     }
@@ -3382,7 +3377,7 @@ class AppWidgetServiceImpl extends IAppWidgetService.Stub implements WidgetBacku
                             // Reschedule for the new updatePeriodMillis (don't worry about handling
                             // it specially if updatePeriodMillis didn't change because we just sent
                             // an update, and the next one will be updatePeriodMillis from now).
-                            cancelBroadcasts(provider);
+                            cancelBroadcastsLocked(provider);
                             registerForBroadcastsLocked(provider, appWidgetIds);
                             // If it's currently showing, call back with the new
                             // AppWidgetProviderInfo.