Fix crashes when quickly adding and removing users

Make USER_REMOVED an ordered broadcast and send it before the user's
state is completely removed from the system. This gives services the
opportunity to clean up their state, while still having access to the
user's directory and UserInfo object (such as serial number).

Tell SyncManager to skip over dying/partially created users.

Improve UserManager tests, waiting for users to be removed fully.

Bug: 7382252

Change-Id: I93cfb39c9efe6f15087bf83c569a2d154ef27168

6 files changed, 94 insertions, 24 deletions

diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java
index 97d299a..cf0603e 100644
--- a/core/java/android/content/Intent.java
+++ b/core/java/android/content/Intent.java
@@ -2430,7 +2430,8 @@ public class Intent implements Parcelable, Cloneable {
     /**
      * Broadcast sent to the system when a user is removed. Carries an extra EXTRA_USER_HANDLE that has
      * the userHandle of the user.  It is sent to all running users except the
-     * one that has been removed.  You must hold
+     * one that has been removed. The user will not be completely removed until all receivers have
+     * handled the broadcast. You must hold
      * {@link android.Manifest.permission#MANAGE_USERS} to receive this broadcast.
      * @hide
      */
diff --git a/core/java/android/content/SyncManager.java b/core/java/android/content/SyncManager.java
index bb0c686..05bab9c 100644
--- a/core/java/android/content/SyncManager.java
+++ b/core/java/android/content/SyncManager.java
@@ -265,7 +265,9 @@ public class SyncManager {
     }
 
     private void doDatabaseCleanup() {
-        for (UserInfo user : mUserManager.getUsers()) {
+        for (UserInfo user : mUserManager.getUsers(true)) {
+            // Skip any partially created/removed users
+            if (user.partial) continue;
             Account[] accountsForUser = AccountManagerService.getSingleton().getAccounts(user.id);
             mSyncStorageEngine.doDatabaseCleanup(accountsForUser, user.id);
         }
diff --git a/services/java/com/android/server/pm/PackageManagerService.java b/services/java/com/android/server/pm/PackageManagerService.java
index f59e30d..2dab88d 100644
--- a/services/java/com/android/server/pm/PackageManagerService.java
+++ b/services/java/com/android/server/pm/PackageManagerService.java
@@ -2575,7 +2575,7 @@ public class PackageManagerService extends IPackageManager.Stub {
     @Override
     public List<ResolveInfo> queryIntentActivities(Intent intent,
             String resolvedType, int flags, int userId) {
-        if (!sUserManager.exists(userId)) return null;
+        if (!sUserManager.exists(userId)) return Collections.emptyList();
         enforceCrossUserPermission(Binder.getCallingUid(), userId, false, "query intent activities");
         ComponentName comp = intent.getComponent();
         if (comp == null) {
@@ -2615,7 +2615,7 @@ public class PackageManagerService extends IPackageManager.Stub {
     public List<ResolveInfo> queryIntentActivityOptions(ComponentName caller,
             Intent[] specifics, String[] specificTypes, Intent intent,
             String resolvedType, int flags, int userId) {
-        if (!sUserManager.exists(userId)) return null;
+        if (!sUserManager.exists(userId)) return Collections.emptyList();
         enforceCrossUserPermission(Binder.getCallingUid(), userId, false,
                 "query intent activity options");
         final String resultsAction = intent.getAction();
@@ -2787,7 +2787,7 @@ public class PackageManagerService extends IPackageManager.Stub {
     @Override
     public List<ResolveInfo> queryIntentReceivers(Intent intent, String resolvedType, int flags,
             int userId) {
-        if (!sUserManager.exists(userId)) return null;
+        if (!sUserManager.exists(userId)) return Collections.emptyList();
         ComponentName comp = intent.getComponent();
         if (comp == null) {
             if (intent.getSelector() != null) {
@@ -2838,7 +2838,7 @@ public class PackageManagerService extends IPackageManager.Stub {
     @Override
     public List<ResolveInfo> queryIntentServices(Intent intent, String resolvedType, int flags,
             int userId) {
-        if (!sUserManager.exists(userId)) return null;
+        if (!sUserManager.exists(userId)) return Collections.emptyList();
         ComponentName comp = intent.getComponent();
         if (comp == null) {
             if (intent.getSelector() != null) {
diff --git a/services/java/com/android/server/pm/UserManagerService.java b/services/java/com/android/server/pm/UserManagerService.java
index 072dd33..fb93d05 100644
--- a/services/java/com/android/server/pm/UserManagerService.java
+++ b/services/java/com/android/server/pm/UserManagerService.java
@@ -19,9 +19,11 @@ package com.android.server.pm;
 import com.android.internal.util.ArrayUtils;
 import com.android.internal.util.FastXmlSerializer;
 
+import android.app.Activity;
 import android.app.ActivityManager;
 import android.app.ActivityManagerNative;
 import android.app.IStopUserCallback;
+import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
 import android.content.pm.PackageManager;
@@ -736,21 +738,38 @@ public class UserManagerService extends IUserManager.Stub {
         return res == ActivityManager.USER_OP_SUCCESS;
     }
 
-    void finishRemoveUser(int userHandle) {
+    void finishRemoveUser(final int userHandle) {
         if (DBG) Slog.i(LOG_TAG, "finishRemoveUser " + userHandle);
-        synchronized (mInstallLock) {
-            synchronized (mPackagesLock) {
-                removeUserStateLocked(userHandle);
-            }
-        }
-        if (DBG) Slog.i(LOG_TAG, "Removed user " + userHandle + ", sending broadcast");
-        // Let other services shutdown any activity
+        // Let other services shutdown any activity and clean up their state before completely
+        // wiping the user's system directory and removing from the user list
         long ident = Binder.clearCallingIdentity();
         try {
             Intent addedIntent = new Intent(Intent.ACTION_USER_REMOVED);
             addedIntent.putExtra(Intent.EXTRA_USER_HANDLE, userHandle);
-            mContext.sendBroadcastAsUser(addedIntent, UserHandle.ALL,
-                    android.Manifest.permission.MANAGE_USERS);
+            mContext.sendOrderedBroadcastAsUser(addedIntent, UserHandle.ALL,
+                    android.Manifest.permission.MANAGE_USERS,
+
+                    new BroadcastReceiver() {
+                        @Override
+                        public void onReceive(Context context, Intent intent) {
+                            if (DBG) {
+                                Slog.i(LOG_TAG,
+                                        "USER_REMOVED broadcast sent, cleaning up user data "
+                                        + userHandle);
+                            }
+                            new Thread() {
+                                public void run() {
+                                    synchronized (mInstallLock) {
+                                        synchronized (mPackagesLock) {
+                                            removeUserStateLocked(userHandle);
+                                        }
+                                    }
+                                }
+                            }.start();
+                        }
+                    },
+
+                    null, Activity.RESULT_OK, null, null);
         } finally {
             Binder.restoreCallingIdentity(ident);
         }
diff --git a/services/tests/servicestests/AndroidManifest.xml b/services/tests/servicestests/AndroidManifest.xml
index 8aeb2af..7848b1d 100644
--- a/services/tests/servicestests/AndroidManifest.xml
+++ b/services/tests/servicestests/AndroidManifest.xml
@@ -34,7 +34,7 @@
     <uses-permission android:name="android.permission.CONNECTIVITY_INTERNAL" />
     <uses-permission android:name="android.permission.ACCESS_WIFI_STATE" />
     <uses-permission android:name="android.permission.MANAGE_USERS" />
-    <uses-permission android:name="android.permission.INTERACT_ACROSS_USERS" />
+    <uses-permission android:name="android.permission.INTERACT_ACROSS_USERS_FULL" />
     
     <application>
         <uses-library android:name="android.test.runner" />
diff --git a/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java b/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java
index 59a86c2..1758d93 100644
--- a/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java
@@ -16,23 +16,37 @@
 
 package com.android.server.pm;
 
+import android.content.BroadcastReceiver;
 import android.content.Context;
+import android.content.Intent;
+import android.content.IntentFilter;
 import android.content.pm.UserInfo;
 import android.os.Debug;
 import android.os.Environment;
 import android.os.UserManager;
 import android.test.AndroidTestCase;
 
+import java.util.ArrayList;
 import java.util.List;
 
 /** Test {@link UserManager} functionality. */
 public class UserManagerTest extends AndroidTestCase {
 
     UserManager mUserManager = null;
+    Object mUserLock = new Object();
 
     @Override
     public void setUp() throws Exception {
         mUserManager = (UserManager) getContext().getSystemService(Context.USER_SERVICE);
+        IntentFilter filter = new IntentFilter(Intent.ACTION_USER_REMOVED);
+        getContext().registerReceiver(new BroadcastReceiver() {
+            @Override
+            public void onReceive(Context context, Intent intent) {
+                synchronized (mUserLock) {
+                    mUserLock.notifyAll();
+                }
+            }
+        }, filter);
     }
 
     public void testHasPrimary() throws Exception {
@@ -54,7 +68,7 @@ public class UserManagerTest extends AndroidTestCase {
             }
         }
         assertTrue(found);
-        mUserManager.removeUser(userInfo.id);
+        removeUser(userInfo.id);
     }
 
     public void testAdd2Users() throws Exception {
@@ -67,14 +81,13 @@ public class UserManagerTest extends AndroidTestCase {
         assertTrue(findUser(0));
         assertTrue(findUser(user1.id));
         assertTrue(findUser(user2.id));
-        mUserManager.removeUser(user1.id);
-        mUserManager.removeUser(user2.id);
+        removeUser(user1.id);
+        removeUser(user2.id);
     }
 
     public void testRemoveUser() throws Exception {
         UserInfo userInfo = mUserManager.createUser("Guest 1", UserInfo.FLAG_GUEST);
-
-        mUserManager.removeUser(userInfo.id);
+        removeUser(userInfo.id);
 
         assertFalse(findUser(userInfo.id));
     }
@@ -95,12 +108,47 @@ public class UserManagerTest extends AndroidTestCase {
         int serialNumber1 = user1.serialNumber;
         assertEquals(serialNumber1, mUserManager.getUserSerialNumber(user1.id));
         assertEquals(user1.id, mUserManager.getUserHandle(serialNumber1));
-        mUserManager.removeUser(user1.id);
+        removeUser(user1.id);
         UserInfo user2 = mUserManager.createUser("User 2", UserInfo.FLAG_RESTRICTED);
         int serialNumber2 = user2.serialNumber;
         assertFalse(serialNumber1 == serialNumber2);
         assertEquals(serialNumber2, mUserManager.getUserSerialNumber(user2.id));
         assertEquals(user2.id, mUserManager.getUserHandle(serialNumber2));
-        mUserManager.removeUser(user2.id);
+        removeUser(user2.id);
+    }
+
+    public void testMaxUsers() {
+        int N = UserManager.getMaxSupportedUsers();
+        int count = mUserManager.getUsers().size();
+        List<UserInfo> created = new ArrayList<UserInfo>();
+        // Create as many users as permitted and make sure creation passes
+        while (count < N) {
+            UserInfo ui = mUserManager.createUser("User " + count, 0);
+            assertNotNull(ui);
+            created.add(ui);
+            count++;
+        }
+        // Try to create one more user and make sure it fails
+        UserInfo extra = null;
+        assertNull(extra = mUserManager.createUser("One more", 0));
+        if (extra != null) {
+            removeUser(extra.id);
+        }
+        while (!created.isEmpty()) {
+            UserInfo user = created.remove(0);
+            removeUser(user.id);
+        }
+    }
+
+    private void removeUser(int userId) {
+        synchronized (mUserLock) {
+            mUserManager.removeUser(userId);
+            while (mUserManager.getUserInfo(userId) != null) {
+                try {
+                    mUserLock.wait(1000);
+                } catch (InterruptedException ie) {
+                }
+            }
+        }
     }
 }