Avoid recycling recently removed user IDs.

Currently, installd doesn't correctly evict VFS cache entries for
FUSE emulated external storage.  This means zygote processes have an
inconsistent view of the FUSE daemon when the system rapidly
recycles user IDs.

To work around this, only consider recycling a user ID after its
VFS cache entries have expired.  The emulated storage FUSE daemon
currently uses a 'entry_valid' timeout of 10 seconds.

Bug: 7407902
Change-Id: Id80cbdd2215d8456467fb31e4c209ca12a505e16

1 files changed, 42 insertions, 23 deletions

diff --git a/services/java/com/android/server/pm/UserManagerService.java b/services/java/com/android/server/pm/UserManagerService.java
index e05442b3b905..66c3ce927bcc 100644
--- a/services/java/com/android/server/pm/UserManagerService.java
+++ b/services/java/com/android/server/pm/UserManagerService.java
@@ -16,8 +16,7 @@
 
 package com.android.server.pm;
 
-import com.android.internal.util.ArrayUtils;
-import com.android.internal.util.FastXmlSerializer;
+import static android.text.format.DateUtils.MINUTE_IN_MILLIS;
 
 import android.app.Activity;
 import android.app.ActivityManager;
@@ -26,7 +25,6 @@ import android.app.IStopUserCallback;
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
-import android.content.SharedPreferences;
 import android.content.pm.PackageManager;
 import android.content.pm.UserInfo;
 import android.graphics.Bitmap;
@@ -34,6 +32,7 @@ import android.graphics.BitmapFactory;
 import android.os.Binder;
 import android.os.Environment;
 import android.os.FileUtils;
+import android.os.Handler;
 import android.os.IUserManager;
 import android.os.Process;
 import android.os.RemoteException;
@@ -42,9 +41,17 @@ import android.os.UserManager;
 import android.util.AtomicFile;
 import android.util.Slog;
 import android.util.SparseArray;
+import android.util.SparseBooleanArray;
 import android.util.TimeUtils;
 import android.util.Xml;
 
+import com.android.internal.util.ArrayUtils;
+import com.android.internal.util.FastXmlSerializer;
+
+import org.xmlpull.v1.XmlPullParser;
+import org.xmlpull.v1.XmlPullParserException;
+import org.xmlpull.v1.XmlSerializer;
+
 import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.FileDescriptor;
@@ -54,13 +61,8 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
 
-import org.xmlpull.v1.XmlPullParser;
-import org.xmlpull.v1.XmlPullParserException;
-import org.xmlpull.v1.XmlSerializer;
-
 public class UserManagerService extends IUserManager.Stub {
 
     private static final String LOG_TAG = "UserManagerService";
@@ -95,19 +97,24 @@ public class UserManagerService extends IUserManager.Stub {
     private final Object mInstallLock;
     private final Object mPackagesLock;
 
+    private final Handler mHandler;
+
     private final File mUsersDir;
     private final File mUserListFile;
     private final File mBaseUserPath;
 
-    private SparseArray<UserInfo> mUsers = new SparseArray<UserInfo>();
-    private HashSet<Integer> mRemovingUserIds = new HashSet<Integer>();
+    private final SparseArray<UserInfo> mUsers = new SparseArray<UserInfo>();
+
+    /**
+     * Set of user IDs being actively removed. Removed IDs linger in this set
+     * for several seconds to work around a VFS caching issue.
+     */
+    // @GuardedBy("mPackagesLock")
+    private final SparseBooleanArray mRemovingUserIds = new SparseBooleanArray();
 
     private int[] mUserIds;
     private boolean mGuestEnabled;
     private int mNextSerialNumber;
-    // This resets on a reboot. Otherwise it keeps incrementing so that user ids are
-    // not reused in quick succession
-    private int mNextUserId = MIN_USER_ID;
     private int mUserVersion = 0;
 
     private static UserManagerService sInstance;
@@ -147,6 +154,7 @@ public class UserManagerService extends IUserManager.Stub {
         mPm = pm;
         mInstallLock = installLock;
         mPackagesLock = packagesLock;
+        mHandler = new Handler();
         synchronized (mInstallLock) {
             synchronized (mPackagesLock) {
                 mUsersDir = new File(dataDir, USER_INFO_DIR);
@@ -190,7 +198,7 @@ public class UserManagerService extends IUserManager.Stub {
                 if (ui.partial) {
                     continue;
                 }
-                if (!excludeDying || !mRemovingUserIds.contains(ui.id)) {
+                if (!excludeDying || !mRemovingUserIds.get(ui.id)) {
                     users.add(ui);
                 }
             }
@@ -212,7 +220,7 @@ public class UserManagerService extends IUserManager.Stub {
     private UserInfo getUserInfoLocked(int userId) {
         UserInfo ui = mUsers.get(userId);
         // If it is partial and not in the process of being removed, return as unknown user.
-        if (ui != null && ui.partial && !mRemovingUserIds.contains(userId)) {
+        if (ui != null && ui.partial && !mRemovingUserIds.get(userId)) {
             Slog.w(LOG_TAG, "getUserInfo: unknown user #" + userId);
             return null;
         }
@@ -502,7 +510,7 @@ public class UserManagerService extends IUserManager.Stub {
 
     private void fallbackToSingleUserLocked() {
         // Create the primary user
-        UserInfo primary = new UserInfo(0, 
+        UserInfo primary = new UserInfo(0,
                 mContext.getResources().getString(com.android.internal.R.string.owner_name), null,
                 UserInfo.FLAG_ADMIN | UserInfo.FLAG_PRIMARY | UserInfo.FLAG_INITIALIZED);
         mUsers.put(0, primary);
@@ -749,7 +757,7 @@ public class UserManagerService extends IUserManager.Stub {
             if (userHandle == 0 || user == null) {
                 return false;
             }
-            mRemovingUserIds.add(userHandle);
+            mRemovingUserIds.put(userHandle, true);
             // Set this to a partially created user, so that the user will be purged
             // on next startup, in case the runtime stops now before stopping and
             // removing the user completely.
@@ -813,13 +821,25 @@ public class UserManagerService extends IUserManager.Stub {
         }
     }
 
-    private void removeUserStateLocked(int userHandle) {
+    private void removeUserStateLocked(final int userHandle) {
         // Cleanup package manager settings
         mPm.cleanUpUserLILPw(userHandle);
 
         // Remove this user from the list
         mUsers.remove(userHandle);
-        mRemovingUserIds.remove(userHandle);
+
+        // Have user ID linger for several seconds to let external storage VFS
+        // cache entries expire. This must be greater than the 'entry_valid'
+        // timeout used by the FUSE daemon.
+        mHandler.postDelayed(new Runnable() {
+            @Override
+            public void run() {
+                synchronized (mPackagesLock) {
+                    mRemovingUserIds.delete(userHandle);
+                }
+            }
+        }, MINUTE_IN_MILLIS);
+
         // Remove user file
         AtomicFile userFile = new AtomicFile(new File(mUsersDir, userHandle + ".xml"));
         userFile.delete();
@@ -906,14 +926,13 @@ public class UserManagerService extends IUserManager.Stub {
      */
     private int getNextAvailableIdLocked() {
         synchronized (mPackagesLock) {
-            int i = mNextUserId;
+            int i = MIN_USER_ID;
             while (i < Integer.MAX_VALUE) {
-                if (mUsers.indexOfKey(i) < 0 && !mRemovingUserIds.contains(i)) {
+                if (mUsers.indexOfKey(i) < 0 && !mRemovingUserIds.get(i)) {
                     break;
                 }
                 i++;
             }
-            mNextUserId = i + 1;
             return i;
         }
     }
@@ -938,7 +957,7 @@ public class UserManagerService extends IUserManager.Stub {
                 UserInfo user = mUsers.valueAt(i);
                 if (user == null) continue;
                 pw.print("  "); pw.print(user); pw.print(" serialNo="); pw.print(user.serialNumber);
-                if (mRemovingUserIds.contains(mUsers.keyAt(i))) pw.print(" <removing> ");
+                if (mRemovingUserIds.get(mUsers.keyAt(i))) pw.print(" <removing> ");
                 if (user.partial) pw.print(" <partial>");
                 pw.println();
                 pw.print("    Created: ");