Merge "Fix crashes when quickly adding and removing users" into jb-mr1-dev

Amith Yamasani · Android (Google) Code Review · commit 8074e98b20ac · 2012-10-21T07:44:46.000-07:00
diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java
@@ -2430,7 +2430,8 @@ public static Intent createChooser(Intent target, CharSequence title) {
     /**
      * 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
@@ -265,7 +265,9 @@ public void updateRunningAccounts() {
     }
 
     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
@@ -2575,7 +2575,7 @@ ResolveInfo findPreferredActivity(Intent intent, String resolvedType,
     @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 List<ResolveInfo> queryIntentActivities(Intent intent,
     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 List<ResolveInfo> queryIntentActivityOptions(ComponentName caller,
     @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 ResolveInfo resolveService(Intent intent, String resolvedType, int flags,
     @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
@@ -19,9 +19,11 @@
 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 void userStopAborted(int userId) {
         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
@@ -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
@@ -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 void testAddUser() throws Exception {
             }
         }
         assertTrue(found);
-        mUserManager.removeUser(userInfo.id);
+        removeUser(userInfo.id);
     }
 
     public void testAdd2Users() throws Exception {
@@ -67,14 +81,13 @@ public void testAdd2Users() throws Exception {
         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 void testSerialNumber() {
         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) {
+                }
+            }
+        }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -265,7 +265,9 @@ public void updateRunningAccounts() {`
`265`	`265`	`}`
`266`	`266`
`267`	`267`	`private void doDatabaseCleanup() {`
`268`		`- for (UserInfo user : mUserManager.getUsers()) {`
	`268`	`+ for (UserInfo user : mUserManager.getUsers(true)) {`
	`269`	`+ // Skip any partially created/removed users`
	`270`	`+ if (user.partial) continue;`
`269`	`271`	`Account[] accountsForUser = AccountManagerService.getSingleton().getAccounts(user.id);`
`270`	`272`	`mSyncStorageEngine.doDatabaseCleanup(accountsForUser, user.id);`
`271`	`273`	`}`