OSDN Git Service

Unlock should always wait for pending PRE_BOOT.
authorJeff Sharkey <jsharkey@android.com>
Mon, 18 Apr 2016 21:36:39 +0000 (15:36 -0600)
committerJeff Sharkey <jsharkey@android.com>
Mon, 18 Apr 2016 21:38:35 +0000 (15:38 -0600)
While processing an unlock request, we might go async to handle
long-running operations like dispatching PRE_BOOT_COMPLETED.  This
change ensures that all unlock requests for a particular user wait
in line behind any pending async operations.

Without this CL, any subsequent unlock requests would immediately
return successful, even though PRE_BOOT_COMPLETED events were still
being processed.

Bug: 28240584
Change-Id: I307d6aaebfb8f38028f3666a2e19e4399b7cf3a7

core/java/com/android/internal/util/ProgressReporter.java
core/tests/coretests/src/com/android/internal/util/ProgressReporterTest.java
services/core/java/com/android/server/am/ActivityManagerService.java
services/core/java/com/android/server/am/UserController.java
services/core/java/com/android/server/am/UserState.java

index 796f8ac..7a8efba 100644 (file)
@@ -20,9 +20,12 @@ import android.annotation.Nullable;
 import android.content.Intent;
 import android.os.Bundle;
 import android.os.IProgressListener;
+import android.os.RemoteCallbackList;
 import android.os.RemoteException;
 import android.util.MathUtils;
 
+import com.android.internal.annotations.GuardedBy;
+
 /**
  * Tracks and reports progress of a single task to a {@link IProgressListener}.
  * The reported progress of a task ranges from 0-100, but the task can be
@@ -44,33 +47,67 @@ import android.util.MathUtils;
  * }
  * </pre>
  *
- * This class is not thread safe.
- *
  * @hide
  */
 public class ProgressReporter {
-    public static final ProgressReporter NO_OP = new ProgressReporter(0, null);
+    private static final int STATE_INIT = 0;
+    private static final int STATE_STARTED = 1;
+    private static final int STATE_FINISHED = 2;
 
     private final int mId;
-    private final IProgressListener mListener;
 
-    private Bundle mExtras = new Bundle();
+    @GuardedBy("this")
+    private final RemoteCallbackList<IProgressListener> mListeners = new RemoteCallbackList<>();
 
+    @GuardedBy("this")
+    private int mState = STATE_INIT;
+    @GuardedBy("this")
     private int mProgress = 0;
+    @GuardedBy("this")
+    private Bundle mExtras = new Bundle();
 
     /**
      * Current segment range: first element is starting progress of this
      * segment, second element is length of segment.
      */
+    @GuardedBy("this")
     private int[] mSegmentRange = new int[] { 0, 100 };
 
     /**
      * Create a new task with the given identifier whose progress will be
      * reported to the given listener.
      */
-    public ProgressReporter(int id, @Nullable IProgressListener listener) {
+    public ProgressReporter(int id) {
         mId = id;
-        mListener = listener;
+    }
+
+    /**
+     * Add given listener to watch for progress events. The current state will
+     * be immediately dispatched to the given listener.
+     */
+    public void addListener(@Nullable IProgressListener listener) {
+        if (listener == null) return;
+        synchronized (this) {
+            mListeners.register(listener);
+            switch (mState) {
+                case STATE_INIT:
+                    // Nothing has happened yet
+                    break;
+                case STATE_STARTED:
+                    try {
+                        listener.onStarted(mId, null);
+                        listener.onProgress(mId, mProgress, mExtras);
+                    } catch (RemoteException ignored) {
+                    }
+                    break;
+                case STATE_FINISHED:
+                    try {
+                        listener.onFinished(mId, null);
+                    } catch (RemoteException ignored) {
+                    }
+                    break;
+            }
+        }
     }
 
     /**
@@ -102,12 +139,17 @@ public class ProgressReporter {
      * Set the fractional progress of the currently active segment.
      */
     public void setProgress(int n, int m, @Nullable CharSequence title) {
-        mProgress = mSegmentRange[0]
-                + MathUtils.constrain((n * mSegmentRange[1]) / m, 0, mSegmentRange[1]);
-        if (title != null) {
-            mExtras.putCharSequence(Intent.EXTRA_TITLE, title);
+        synchronized (this) {
+            if (mState != STATE_STARTED) {
+                throw new IllegalStateException("Must be started to change progress");
+            }
+            mProgress = mSegmentRange[0]
+                    + MathUtils.constrain((n * mSegmentRange[1]) / m, 0, mSegmentRange[1]);
+            if (title != null) {
+                mExtras.putCharSequence(Intent.EXTRA_TITLE, title);
+            }
+            notifyProgress(mId, mProgress, mExtras);
         }
-        notifyProgress(mId, mProgress, mExtras);
     }
 
     /**
@@ -116,17 +158,21 @@ public class ProgressReporter {
      * {@link #endSegment(int[])} when finished.
      */
     public int[] startSegment(int size) {
-        final int[] lastRange = mSegmentRange;
-        mSegmentRange = new int[] { mProgress, (size * mSegmentRange[1] / 100) };
-        return lastRange;
+        synchronized (this) {
+            final int[] lastRange = mSegmentRange;
+            mSegmentRange = new int[] { mProgress, (size * mSegmentRange[1] / 100) };
+            return lastRange;
+        }
     }
 
     /**
      * End the current segment.
      */
     public void endSegment(int[] lastRange) {
-        mProgress = mSegmentRange[0] + mSegmentRange[1];
-        mSegmentRange = lastRange;
+        synchronized (this) {
+            mProgress = mSegmentRange[0] + mSegmentRange[1];
+            mSegmentRange = lastRange;
+        }
     }
 
     int getProgress() {
@@ -138,27 +184,54 @@ public class ProgressReporter {
     }
 
     /**
+     * Report this entire task as being started.
+     */
+    public void start() {
+        synchronized (this) {
+            mState = STATE_STARTED;
+            notifyStarted(mId, null);
+            notifyProgress(mId, mProgress, mExtras);
+        }
+    }
+
+    /**
      * Report this entire task as being finished.
      */
     public void finish() {
-        notifyFinished(mId, null);
+        synchronized (this) {
+            mState = STATE_FINISHED;
+            notifyFinished(mId, null);
+            mListeners.kill();
+        }
+    }
+
+    private void notifyStarted(int id, Bundle extras) {
+        for (int i = mListeners.beginBroadcast() - 1; i >= 0; i--) {
+            try {
+                mListeners.getBroadcastItem(i).onStarted(id, extras);
+            } catch (RemoteException ignored) {
+            }
+        }
+        mListeners.finishBroadcast();
     }
 
     private void notifyProgress(int id, int progress, Bundle extras) {
-        if (mListener != null) {
+        for (int i = mListeners.beginBroadcast() - 1; i >= 0; i--) {
             try {
-                mListener.onProgress(id, progress, extras);
+                mListeners.getBroadcastItem(i).onProgress(id, progress, extras);
             } catch (RemoteException ignored) {
             }
         }
+        mListeners.finishBroadcast();
     }
 
-    public void notifyFinished(int id, Bundle extras) {
-        if (mListener != null) {
+    private void notifyFinished(int id, Bundle extras) {
+        for (int i = mListeners.beginBroadcast() - 1; i >= 0; i--) {
             try {
-                mListener.onFinished(id, extras);
+                mListeners.getBroadcastItem(i).onFinished(id, extras);
             } catch (RemoteException ignored) {
             }
         }
+        mListeners.finishBroadcast();
     }
 }
index fbf5523..87f2a8a 100644 (file)
@@ -24,7 +24,7 @@ public class ProgressReporterTest extends TestCase {
     @Override
     protected void setUp() throws Exception {
         super.setUp();
-        r = new ProgressReporter(0, null);
+        r = new ProgressReporter(0);
     }
 
     private void assertProgress(int expected) {
index 8653f1a..5fd8741 100644 (file)
@@ -20669,7 +20669,7 @@ public final class ActivityManagerService extends ActivityManagerNative
 
     @Override
     public boolean unlockUser(int userId, byte[] token, byte[] secret, IProgressListener listener) {
-        return mUserController.unlockUser(userId, token, secret, new ProgressReporter(0, listener));
+        return mUserController.unlockUser(userId, token, secret, listener);
     }
 
     @Override
index 4292fcf..4c050c4 100644 (file)
@@ -63,6 +63,7 @@ import android.os.Bundle;
 import android.os.Debug;
 import android.os.Handler;
 import android.os.IBinder;
+import android.os.IProgressListener;
 import android.os.IRemoteCallback;
 import android.os.IUserManager;
 import android.os.Process;
@@ -83,7 +84,6 @@ import android.util.SparseIntArray;
 import com.android.internal.R;
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.util.ArrayUtils;
-import com.android.internal.util.ProgressReporter;
 import com.android.internal.widget.LockPatternUtils;
 import com.android.server.LocalServices;
 import com.android.server.pm.UserManagerService;
@@ -260,7 +260,7 @@ final class UserController {
      * Step from {@link UserState#STATE_RUNNING_LOCKED} to
      * {@link UserState#STATE_RUNNING_UNLOCKING}.
      */
-    void finishUserUnlocking(final UserState uss, final ProgressReporter progress) {
+    private void finishUserUnlocking(final UserState uss) {
         final int userId = uss.mHandle.getIdentifier();
         synchronized (mService) {
             // Bail if we ended up with a stale user
@@ -270,10 +270,13 @@ final class UserController {
             if (!isUserKeyUnlocked(userId)) return;
 
             if (uss.setState(STATE_RUNNING_LOCKED, STATE_RUNNING_UNLOCKING)) {
+                uss.mUnlockProgress.start();
+
                 // Prepare app storage before we go any further
-                progress.setProgress(5, mService.mContext.getString(R.string.android_start_title));
+                uss.mUnlockProgress.setProgress(5,
+                        mService.mContext.getString(R.string.android_start_title));
                 mUserManager.onBeforeUnlockUser(userId);
-                progress.setProgress(20);
+                uss.mUnlockProgress.setProgress(20);
 
                 // Dispatch unlocked to system services
                 mHandler.sendMessage(mHandler.obtainMessage(SYSTEM_USER_UNLOCK_MSG, userId, 0));
@@ -306,15 +309,15 @@ final class UserController {
                 // Send PRE_BOOT broadcasts if fingerprint changed
                 final UserInfo info = getUserInfo(userId);
                 if (!Objects.equals(info.lastLoggedInFingerprint, Build.FINGERPRINT)) {
-                    progress.startSegment(80);
-                    new PreBootBroadcaster(mService, userId, progress) {
+                    uss.mUnlockProgress.startSegment(80);
+                    new PreBootBroadcaster(mService, userId, uss.mUnlockProgress) {
                         @Override
                         public void onFinished() {
-                            finishUserUnlocked(uss, progress);
+                            finishUserUnlocked(uss);
                         }
                     }.sendNext();
                 } else {
-                    finishUserUnlocked(uss, progress);
+                    finishUserUnlocked(uss);
                 }
             }
         }
@@ -324,15 +327,15 @@ final class UserController {
      * Step from {@link UserState#STATE_RUNNING_UNLOCKING} to
      * {@link UserState#STATE_RUNNING_UNLOCKED}.
      */
-    void finishUserUnlocked(UserState uss, ProgressReporter progress) {
+    private void finishUserUnlocked(UserState uss) {
         try {
             finishUserUnlockedInternal(uss);
         } finally {
-            progress.finish();
+            uss.mUnlockProgress.finish();
         }
     }
 
-    void finishUserUnlockedInternal(UserState uss) {
+    private void finishUserUnlockedInternal(UserState uss) {
         final int userId = uss.mHandle.getIdentifier();
         synchronized (mService) {
             // Bail if we ended up with a stale user
@@ -860,7 +863,7 @@ final class UserController {
         return result;
     }
 
-    boolean unlockUser(final int userId, byte[] token, byte[] secret, ProgressReporter progress) {
+    boolean unlockUser(final int userId, byte[] token, byte[] secret, IProgressListener listener) {
         if (mService.checkCallingPermission(INTERACT_ACROSS_USERS_FULL)
                 != PackageManager.PERMISSION_GRANTED) {
             String msg = "Permission Denial: unlockUser() from pid="
@@ -873,7 +876,7 @@ final class UserController {
 
         final long binderToken = Binder.clearCallingIdentity();
         try {
-            return unlockUserCleared(userId, token, secret, progress);
+            return unlockUserCleared(userId, token, secret, listener);
         } finally {
             Binder.restoreCallingIdentity(binderToken);
         }
@@ -887,23 +890,29 @@ final class UserController {
      */
     boolean maybeUnlockUser(final int userId) {
         // Try unlocking storage using empty token
-        return unlockUserCleared(userId, null, null, ProgressReporter.NO_OP);
+        return unlockUserCleared(userId, null, null, null);
+    }
+
+    private static void notifyFinished(int userId, IProgressListener listener) {
+        if (listener == null) return;
+        try {
+            listener.onFinished(userId, null);
+        } catch (RemoteException ignored) {
+        }
     }
 
     boolean unlockUserCleared(final int userId, byte[] token, byte[] secret,
-            ProgressReporter progress) {
+            IProgressListener listener) {
+        final UserState uss;
         synchronized (mService) {
-            // Bail if already running unlocked, or if not running at all
-            final UserState uss = mStartedUsers.get(userId);
+            // Bail if user isn't actually running, otherwise register the given
+            // listener to watch for unlock progress
+            uss = mStartedUsers.get(userId);
             if (uss == null) {
-                progress.finish();
+                notifyFinished(userId, listener);
                 return false;
-            }
-            switch (uss.state) {
-                case STATE_RUNNING_UNLOCKING:
-                case STATE_RUNNING_UNLOCKED:
-                    progress.finish();
-                    return true;
+            } else {
+                uss.mUnlockProgress.addListener(listener);
             }
         }
 
@@ -914,14 +923,13 @@ final class UserController {
                 mountService.unlockUserKey(userId, userInfo.serialNumber, token, secret);
             } catch (RemoteException | RuntimeException e) {
                 Slog.w(TAG, "Failed to unlock: " + e.getMessage());
-                progress.finish();
+                notifyFinished(userId, listener);
                 return false;
             }
         }
 
         synchronized (mService) {
-            final UserState uss = mStartedUsers.get(userId);
-            finishUserUnlocking(uss, progress);
+            finishUserUnlocking(uss);
 
             // We just unlocked a user, so let's now attempt to unlock any
             // managed profiles under that user.
index 6e2342b..56abd95 100644 (file)
@@ -16,9 +16,6 @@
 
 package com.android.server.am;
 
-import java.io.PrintWriter;
-import java.util.ArrayList;
-
 import static com.android.server.am.ActivityManagerDebugConfig.DEBUG_MU;
 import static com.android.server.am.ActivityManagerDebugConfig.TAG_AM;
 import static com.android.server.am.ActivityManagerDebugConfig.TAG_WITH_CLASS_NAME;
@@ -28,6 +25,11 @@ import android.os.UserHandle;
 import android.util.ArrayMap;
 import android.util.Slog;
 
+import com.android.internal.util.ProgressReporter;
+
+import java.io.PrintWriter;
+import java.util.ArrayList;
+
 public final class UserState {
     private static final String TAG = TAG_WITH_CLASS_NAME ? "UserState" : TAG_AM;
 
@@ -47,6 +49,7 @@ public final class UserState {
     public final UserHandle mHandle;
     public final ArrayList<IStopUserCallback> mStopCallbacks
             = new ArrayList<IStopUserCallback>();
+    public final ProgressReporter mUnlockProgress;
 
     public int state = STATE_BOOTING;
     public int lastState = STATE_BOOTING;
@@ -61,6 +64,7 @@ public final class UserState {
 
     public UserState(UserHandle handle) {
         mHandle = handle;
+        mUnlockProgress = new ProgressReporter(handle.getIdentifier());
     }
 
     public boolean setState(int oldState, int newState) {