From 4428e17c5e05c0dad76da8f1c28ccba62a66cd91 Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Fri, 24 Aug 2012 17:43:05 -0700 Subject: Some clean up of app install and user management. UserManagerService is now closely tied to PackageManagerService, sharing the same locks. There is no longer direct access of Installer by UserManagerService, instead the package manager is back to solely owning it. Creating a new user now correctly only installs system apps for that user. Fixed some misc bugs, where we were getting nulls when querying content providers and instrumentation in uninstalled users, incorrect locking, etc. Change-Id: Ife69b6e373d0cf7c5cfc03fc588e36b43ad5d8b0 --- services/java/com/android/server/SystemServer.java | 2 +- .../android/server/pm/PackageManagerService.java | 145 +++++++++----- services/java/com/android/server/pm/Settings.java | 2 +- .../com/android/server/pm/UserManagerService.java | 219 +++++++++------------ 4 files changed, 185 insertions(+), 183 deletions(-) (limited to 'services/java') diff --git a/services/java/com/android/server/SystemServer.java b/services/java/com/android/server/SystemServer.java index a6af14420605..7097891fd1f0 100644 --- a/services/java/com/android/server/SystemServer.java +++ b/services/java/com/android/server/SystemServer.java @@ -197,7 +197,7 @@ class ServerThread extends Thread { Slog.i(TAG, "User Service"); ServiceManager.addService(Context.USER_SERVICE, - UserManagerService.getInstance(context)); + UserManagerService.getInstance()); mContentResolver = context.getContentResolver(); diff --git a/services/java/com/android/server/pm/PackageManagerService.java b/services/java/com/android/server/pm/PackageManagerService.java index b0aca21e1eec..2a545cd358ea 100644 --- a/services/java/com/android/server/pm/PackageManagerService.java +++ b/services/java/com/android/server/pm/PackageManagerService.java @@ -950,8 +950,8 @@ public class PackageManagerService extends IPackageManager.Stub { mUserAppDataDir = new File(dataDir, "user"); mDrmAppPrivateInstallDir = new File(dataDir, "app-private"); - sUserManager = UserManagerService.getInstance(context); - sUserManager.setInstaller(this, mInstaller); + sUserManager = new UserManagerService(context, this, + mInstallLock, mPackages); readPermissions(); @@ -1145,8 +1145,7 @@ public class PackageManagerService extends IPackageManager.Stub { String msg = "System package " + ps.name + " no longer exists; wiping its data"; reportSettingsProblem(Log.WARN, msg); - mInstaller.remove(ps.name, 0); - sUserManager.removePackageForAllUsers(ps.name); + removeDataDirsLI(ps.name); } else { final PackageSetting disabledPs = mSettings.getDisabledSystemPkgLPr(ps.name); if (disabledPs.codePath == null || !disabledPs.codePath.exists()) { @@ -1195,9 +1194,7 @@ public class PackageManagerService extends IPackageManager.Stub { if (deletedPkg == null) { msg = "Updated system package " + deletedAppName + " no longer exists; wiping its data"; - - mInstaller.remove(deletedAppName, 0); - sUserManager.removePackageForAllUsers(deletedAppName); + removeDataDirsLI(deletedAppName); } else { msg = "Updated system app + " + deletedAppName + " no longer present; removing system privileges for " @@ -1332,13 +1329,7 @@ public class PackageManagerService extends IPackageManager.Stub { void cleanupInstallFailedPackage(PackageSetting ps) { Slog.i(TAG, "Cleaning up incompletely installed app: " + ps.name); - int retCode = mInstaller.remove(ps.name, 0); - if (retCode < 0) { - Slog.w(TAG, "Couldn't remove app data directory for package: " - + ps.name + ", retcode=" + retCode); - } else { - sUserManager.removePackageForAllUsers(ps.name); - } + removeDataDirsLI(ps.name); if (ps.codePath != null) { if (!ps.codePath.delete()) { Slog.w(TAG, "Unable to remove old code file: " + ps.codePath); @@ -1818,9 +1809,11 @@ public class PackageManagerService extends IPackageManager.Stub { public void run() { mHandler.removeCallbacks(this); int retCode = -1; - retCode = mInstaller.freeCache(freeStorageSize); - if (retCode < 0) { - Slog.w(TAG, "Couldn't clear application caches"); + synchronized (mInstallLock) { + retCode = mInstaller.freeCache(freeStorageSize); + if (retCode < 0) { + Slog.w(TAG, "Couldn't clear application caches"); + } } if (observer != null) { try { @@ -1841,9 +1834,11 @@ public class PackageManagerService extends IPackageManager.Stub { public void run() { mHandler.removeCallbacks(this); int retCode = -1; - retCode = mInstaller.freeCache(freeStorageSize); - if (retCode < 0) { - Slog.w(TAG, "Couldn't clear application caches"); + synchronized (mInstallLock) { + retCode = mInstaller.freeCache(freeStorageSize); + if (retCode < 0) { + Slog.w(TAG, "Couldn't clear application caches"); + } } if(pi != null) { try { @@ -2994,7 +2989,9 @@ public class PackageManagerService extends IPackageManager.Stub { } ProviderInfo info = PackageParser.generateProviderInfo(p, flags, ps.readUserState(userId), userId); - finalList.add(info); + if (info != null) { + finalList.add(info); + } } } } @@ -3027,8 +3024,11 @@ public class PackageManagerService extends IPackageManager.Stub { final PackageParser.Instrumentation p = i.next(); if (targetPackage == null || targetPackage.equals(p.info.targetPackage)) { - finalList.add(PackageParser.generateInstrumentationInfo(p, - flags)); + InstrumentationInfo ii = PackageParser.generateInstrumentationInfo(p, + flags); + if (ii != null) { + finalList.add(ii); + } } } } @@ -3189,7 +3189,7 @@ public class PackageManagerService extends IPackageManager.Stub { InstallArgs args = createInstallArgs(packageFlagsToInstallFlags(ps), ps.codePathString, ps.resourcePathString, ps.nativeLibraryPathString); - synchronized (mInstaller) { + synchronized (mInstallLock) { args.cleanUpResourcesLI(); } synchronized (mPackages) { @@ -3245,7 +3245,7 @@ public class PackageManagerService extends IPackageManager.Stub { + " better than installed " + ps.versionCode); InstallArgs args = createInstallArgs(packageFlagsToInstallFlags(ps), ps.codePathString, ps.resourcePathString, ps.nativeLibraryPathString); - synchronized (mInstaller) { + synchronized (mInstallLock) { args.cleanUpResourcesLI(); } } @@ -3479,6 +3479,36 @@ public class PackageManagerService extends IPackageManager.Stub { } } + private int createDataDirsLI(String packageName, int uid) { + int[] users = sUserManager.getUserIds(); + int res = mInstaller.install(packageName, uid, uid); + if (res < 0) { + return res; + } + for (int user : users) { + if (user != 0) { + res = mInstaller.createUserData(packageName, + UserHandle.getUid(user, uid), user); + if (res < 0) { + return res; + } + } + } + return res; + } + + private int removeDataDirsLI(String packageName) { + int[] users = sUserManager.getUserIds(); + int res = 0; + for (int user : users) { + int resInner = mInstaller.remove(packageName, user); + if (resInner < 0) { + res = resInner; + } + } + return res; + } + private PackageParser.Package scanPackageLI(PackageParser.Package pkg, int parseFlags, int scanMode, long currentTime, UserHandle user) { File scanFile = new File(pkg.mScanPath); @@ -3831,11 +3861,9 @@ public class PackageManagerService extends IPackageManager.Stub { || (scanMode&SCAN_BOOTING) != 0)) { // If this is a system app, we can at least delete its // current data so the application will still work. - int ret = mInstaller.remove(pkgName, 0); + int ret = removeDataDirsLI(pkgName); if (ret >= 0) { // TODO: Kill the processes first - // Remove the data directories for all users - sUserManager.removePackageForAllUsers(pkgName); // Old data gone! String prefix = (parseFlags&PackageParser.PARSE_IS_SYSTEM) != 0 ? "System package " : "Third party package "; @@ -3847,8 +3875,7 @@ public class PackageManagerService extends IPackageManager.Stub { recovered = true; // And now re-install the app. - ret = mInstaller.install(pkgName, pkg.applicationInfo.uid, - pkg.applicationInfo.uid); + ret = createDataDirsLI(pkgName, pkg.applicationInfo.uid); if (ret == -1) { // Ack should not happen! msg = prefix + pkg.packageName @@ -3857,9 +3884,6 @@ public class PackageManagerService extends IPackageManager.Stub { mLastScanError = PackageManager.INSTALL_FAILED_INSUFFICIENT_STORAGE; return null; } - // Create data directories for all users - sUserManager.installPackageForAllUsers(pkgName, - pkg.applicationInfo.uid); } if (!recovered) { mHasSystemUidErrors = true; @@ -3897,15 +3921,12 @@ public class PackageManagerService extends IPackageManager.Stub { Log.v(TAG, "Want this data dir: " + dataPath); } //invoke installer to do the actual installation - int ret = mInstaller.install(pkgName, pkg.applicationInfo.uid, - pkg.applicationInfo.uid); + int ret = createDataDirsLI(pkgName, pkg.applicationInfo.uid); if (ret < 0) { // Error from installer mLastScanError = PackageManager.INSTALL_FAILED_INSUFFICIENT_STORAGE; return null; } - // Create data directories for all users - sUserManager.installPackageForAllUsers(pkgName, pkg.applicationInfo.uid); if (dataPath.exists()) { pkg.applicationInfo.dataDir = dataPath.getPath(); @@ -5078,6 +5099,9 @@ public class PackageManagerService extends IPackageManager.Stub { } ServiceInfo si = PackageParser.generateServiceInfo(service, mFlags, ps.readUserState(userId), userId); + if (si == null) { + return null; + } final ResolveInfo res = new ResolveInfo(); res.serviceInfo = si; if ((mFlags&PackageManager.GET_RESOLVED_FILTER) != 0) { @@ -7819,15 +7843,7 @@ public class PackageManagerService extends IPackageManager.Stub { } } if ((flags&PackageManager.DELETE_KEEP_DATA) == 0) { - int retCode = mInstaller.remove(packageName, 0); - if (retCode < 0) { - Slog.w(TAG, "Couldn't remove app data or cache directory for package: " - + packageName + ", retcode=" + retCode); - // we don't consider this to be a failure of the core package deletion - } else { - // TODO: Kill the processes first - sUserManager.removePackageForAllUsers(packageName); - } + removeDataDirsLI(packageName); schedulePackageCleaning(packageName, UserHandle.USER_ALL, true); } // writer @@ -9742,15 +9758,36 @@ public class PackageManagerService extends IPackageManager.Stub { } /** Called by UserManagerService */ - void cleanUpUser(int userHandle) { + void cleanUpUserLILPw(int userHandle) { // Disable all the packages for the user first - synchronized (mPackages) { - Set> entries = mSettings.mPackages.entrySet(); - for (Entry entry : entries) { - entry.getValue().removeUser(userHandle); - } - if (mDirtyUsers.remove(userHandle)); - mSettings.removeUserLPr(userHandle); + Set> entries = mSettings.mPackages.entrySet(); + for (Entry entry : entries) { + entry.getValue().removeUser(userHandle); + } + if (mDirtyUsers.remove(userHandle)); + mSettings.removeUserLPr(userHandle); + if (mInstaller != null) { + // Technically, we shouldn't be doing this with the package lock + // held. However, this is very rare, and there is already so much + // other disk I/O going on, that we'll let it slide for now. + mInstaller.removeUserDataDirs(userHandle); + } + } + + /** Called by UserManagerService */ + void createNewUserLILPw(int userHandle, File path) { + if (mInstaller != null) { + path.mkdir(); + FileUtils.setPermissions(path.toString(), FileUtils.S_IRWXU | FileUtils.S_IRWXG + | FileUtils.S_IXOTH, -1, -1); + for (PackageSetting ps : mSettings.mPackages.values()) { + // Only system apps are initially installed. + ps.setInstalled((ps.pkgFlags&ApplicationInfo.FLAG_SYSTEM) != 0, userHandle); + // Need to create a data directory for all apps under this user. + mInstaller.createUserData(ps.name, + UserHandle.getUid(userHandle, ps.appId), userHandle); + } + mSettings.writePackageRestrictionsLPr(userHandle); } } diff --git a/services/java/com/android/server/pm/Settings.java b/services/java/com/android/server/pm/Settings.java index a341304162ad..68b594a1ce49 100644 --- a/services/java/com/android/server/pm/Settings.java +++ b/services/java/com/android/server/pm/Settings.java @@ -2476,7 +2476,7 @@ final class Settings { private List getAllUsers() { long id = Binder.clearCallingIdentity(); try { - return UserManagerService.getInstance(mContext).getUsers(); + return UserManagerService.getInstance().getUsers(); } catch (NullPointerException npe) { // packagemanager not yet initialized } finally { diff --git a/services/java/com/android/server/pm/UserManagerService.java b/services/java/com/android/server/pm/UserManagerService.java index 8899ea2fb874..750aa720e953 100644 --- a/services/java/com/android/server/pm/UserManagerService.java +++ b/services/java/com/android/server/pm/UserManagerService.java @@ -25,7 +25,6 @@ import com.android.internal.util.FastXmlSerializer; import android.app.ActivityManager; import android.content.Context; import android.content.Intent; -import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.pm.UserInfo; import android.os.Binder; @@ -34,10 +33,8 @@ import android.os.FileUtils; import android.os.IUserManager; import android.os.ParcelFileDescriptor; import android.os.Process; -import android.os.SystemClock; import android.os.UserHandle; import android.util.AtomicFile; -import android.util.Log; import android.util.Slog; import android.util.SparseArray; import android.util.Xml; @@ -72,59 +69,79 @@ public class UserManagerService extends IUserManager.Stub { private static final String USER_LIST_FILENAME = "userlist.xml"; private static final String USER_PHOTO_FILENAME = "photo.png"; - private SparseArray mUsers = new SparseArray(); + private final Context mContext; + private final PackageManagerService mPm; + private final Object mInstallLock; + private final Object mPackagesLock; private final File mUsersDir; private final File mUserListFile; + private final File mBaseUserPath; + + private SparseArray mUsers = new SparseArray(); + private int[] mUserIds; private boolean mGuestEnabled; private int mNextSerialNumber; - private Installer mInstaller; - private File mBaseUserPath; - private Context mContext; private static UserManagerService sInstance; - private PackageManagerService mPm; - public synchronized static UserManagerService getInstance(Context context) { - if (sInstance == null) { - sInstance = new UserManagerService(context); + public static UserManagerService getInstance() { + synchronized (UserManagerService.class) { + return sInstance; } - return sInstance; } /** * Available for testing purposes. */ UserManagerService(File dataDir, File baseUserPath) { - mUsersDir = new File(dataDir, USER_INFO_DIR); - mUsersDir.mkdirs(); - // Make zeroth user directory, for services to migrate their files to that location - File userZeroDir = new File(mUsersDir, "0"); - userZeroDir.mkdirs(); - mBaseUserPath = baseUserPath; - FileUtils.setPermissions(mUsersDir.toString(), - FileUtils.S_IRWXU|FileUtils.S_IRWXG - |FileUtils.S_IROTH|FileUtils.S_IXOTH, - -1, -1); - mUserListFile = new File(mUsersDir, USER_LIST_FILENAME); - readUserList(); + this(null, null, new Object(), new Object(), dataDir, baseUserPath); } - public UserManagerService(Context context) { - this(Environment.getDataDirectory(), new File(Environment.getDataDirectory(), "user")); - mContext = context; + /** + * Called by package manager to create the service. This is closely + * associated with the package manager, and the given lock is the + * package manager's own lock. + */ + UserManagerService(Context context, PackageManagerService pm, + Object installLock, Object packagesLock) { + this(context, pm, installLock, packagesLock, + Environment.getDataDirectory(), + new File(Environment.getDataDirectory(), "user")); } - void setInstaller(PackageManagerService pm, Installer installer) { - mInstaller = installer; - mPm = pm; + /** + * Available for testing purposes. + */ + private UserManagerService(Context context, PackageManagerService pm, + Object installLock, Object packagesLock, + File dataDir, File baseUserPath) { + synchronized (UserManagerService.class) { + mContext = context; + mPm = pm; + mInstallLock = installLock; + mPackagesLock = packagesLock; + mUsersDir = new File(dataDir, USER_INFO_DIR); + mUsersDir.mkdirs(); + // Make zeroth user directory, for services to migrate their files to that location + File userZeroDir = new File(mUsersDir, "0"); + userZeroDir.mkdirs(); + mBaseUserPath = baseUserPath; + FileUtils.setPermissions(mUsersDir.toString(), + FileUtils.S_IRWXU|FileUtils.S_IRWXG + |FileUtils.S_IROTH|FileUtils.S_IXOTH, + -1, -1); + mUserListFile = new File(mUsersDir, USER_LIST_FILENAME); + readUserList(); + sInstance = this; + } } @Override public List getUsers() { checkManageUsersPermission("query users"); - synchronized (mUsers) { + synchronized (mPackagesLock) { ArrayList users = new ArrayList(mUsers.size()); for (int i = 0; i < mUsers.size(); i++) { users.add(mUsers.valueAt(i)); @@ -136,7 +153,7 @@ public class UserManagerService extends IUserManager.Stub { @Override public UserInfo getUserInfo(int userId) { checkManageUsersPermission("query user"); - synchronized (mUsers) { + synchronized (mPackagesLock) { return getUserInfoLocked(userId); } } @@ -149,7 +166,7 @@ public class UserManagerService extends IUserManager.Stub { } public boolean exists(int userId) { - synchronized (mUsers) { + synchronized (mPackagesLock) { return ArrayUtils.contains(mUserIds, userId); } } @@ -157,7 +174,7 @@ public class UserManagerService extends IUserManager.Stub { @Override public void setUserName(int userId, String name) { checkManageUsersPermission("rename users"); - synchronized (mUsers) { + synchronized (mPackagesLock) { UserInfo info = mUsers.get(userId); if (name != null && !name.equals(info.name)) { info.name = name; @@ -169,7 +186,7 @@ public class UserManagerService extends IUserManager.Stub { @Override public ParcelFileDescriptor setUserIcon(int userId) { checkManageUsersPermission("update users"); - synchronized (mUsers) { + synchronized (mPackagesLock) { UserInfo info = mUsers.get(userId); if (info == null) return null; ParcelFileDescriptor fd = updateIconBitmapLocked(info); @@ -183,7 +200,7 @@ public class UserManagerService extends IUserManager.Stub { @Override public void setGuestEnabled(boolean enable) { checkManageUsersPermission("enable guest users"); - synchronized (mUsers) { + synchronized (mPackagesLock) { if (mGuestEnabled != enable) { mGuestEnabled = enable; // Erase any guest user that currently exists @@ -206,7 +223,7 @@ public class UserManagerService extends IUserManager.Stub { @Override public boolean isGuestEnabled() { - synchronized (mUsers) { + synchronized (mPackagesLock) { return mGuestEnabled; } } @@ -262,13 +279,17 @@ public class UserManagerService extends IUserManager.Stub { * @return the array of user ids. */ int[] getUserIds() { - synchronized (mUsers) { + synchronized (mPackagesLock) { return mUserIds; } } + int[] getUserIdsLPr() { + return mUserIds; + } + private void readUserList() { - synchronized (mUsers) { + synchronized (mPackagesLock) { readUserListLocked(); } } @@ -501,15 +522,15 @@ public class UserManagerService extends IUserManager.Stub { int userId = getNextAvailableId(); UserInfo userInfo = new UserInfo(userId, name, null, flags); File userPath = new File(mBaseUserPath, Integer.toString(userId)); - if (!createPackageFolders(userId, userPath)) { - return null; - } - synchronized (mUsers) { - userInfo.serialNumber = mNextSerialNumber++; - mUsers.put(userId, userInfo); - writeUserListLocked(); - writeUserLocked(userInfo); - updateUserIdsLocked(); + synchronized (mInstallLock) { + synchronized (mPackagesLock) { + userInfo.serialNumber = mNextSerialNumber++; + mUsers.put(userId, userInfo); + writeUserListLocked(); + writeUserLocked(userInfo); + updateUserIdsLocked(); + mPm.createNewUserLILPw(userId, userPath); + } } if (userInfo != null) { Intent addedIntent = new Intent(Intent.ACTION_USER_ADDED); @@ -528,24 +549,37 @@ public class UserManagerService extends IUserManager.Stub { */ public boolean removeUser(int userHandle) { checkManageUsersPermission("Only the system can remove users"); - boolean result; - synchronized (mUsers) { - result = removeUserLocked(userHandle); - } + synchronized (mInstallLock) { + synchronized (mPackagesLock) { + final UserInfo user = mUsers.get(userHandle); + if (userHandle == 0 || user == null) { + return false; + } - // Cleanup package manager settings - mPm.cleanUpUser(userHandle); + // Cleanup package manager settings + mPm.cleanUpUserLILPw(userHandle); + + // Remove this user from the list + mUsers.remove(userHandle); + // Remove user file + AtomicFile userFile = new AtomicFile(new File(mUsersDir, userHandle + ".xml")); + userFile.delete(); + // Update the user list + writeUserListLocked(); + updateUserIdsLocked(); + } + } // Let other services shutdown any activity Intent addedIntent = new Intent(Intent.ACTION_USER_REMOVED); addedIntent.putExtra(Intent.EXTRA_USER_HANDLE, userHandle); mContext.sendBroadcast(addedIntent, android.Manifest.permission.MANAGE_USERS); - return result; + return true; } @Override public int getUserSerialNumber(int userHandle) { - synchronized (mUsers) { + synchronized (mPackagesLock) { if (!exists(userHandle)) return -1; return getUserInfoLocked(userHandle).serialNumber; } @@ -553,7 +587,7 @@ public class UserManagerService extends IUserManager.Stub { @Override public int getUserHandle(int userSerialNumber) { - synchronized (mUsers) { + synchronized (mPackagesLock) { for (int userId : mUserIds) { if (getUserInfoLocked(userId).serialNumber == userSerialNumber) return userId; } @@ -562,53 +596,6 @@ public class UserManagerService extends IUserManager.Stub { } } - private boolean removeUserLocked(int userHandle) { - final UserInfo user = mUsers.get(userHandle); - if (userHandle == 0 || user == null) { - return false; - } - - // Remove this user from the list - mUsers.remove(userHandle); - // Remove user file - AtomicFile userFile = new AtomicFile(new File(mUsersDir, userHandle + ".xml")); - userFile.delete(); - // Update the user list - writeUserListLocked(); - updateUserIdsLocked(); - - removePackageFolders(userHandle); - return true; - } - - public void installPackageForAllUsers(String packageName, int uid) { - for (int userId : mUserIds) { - // Don't do it for the primary user, it will become recursive. - if (userId == 0) - continue; - mInstaller.createUserData(packageName, UserHandle.getUid(userId, uid), - userId); - } - } - - public void clearUserDataForAllUsers(String packageName) { - for (int userId : mUserIds) { - // Don't do it for the primary user, it will become recursive. - if (userId == 0) - continue; - mInstaller.clearUserData(packageName, userId); - } - } - - public void removePackageForAllUsers(String packageName) { - for (int userId : mUserIds) { - // Don't do it for the primary user, it will become recursive. - if (userId == 0) - continue; - mInstaller.remove(packageName, userId); - } - } - /** * Caches the list of user ids in an array, adjusting the array size when necessary. */ @@ -627,7 +614,7 @@ public class UserManagerService extends IUserManager.Stub { * @return */ private int getNextAvailableId() { - synchronized (mUsers) { + synchronized (mPackagesLock) { int i = 0; while (i < Integer.MAX_VALUE) { if (mUsers.indexOfKey(i) < 0) { @@ -638,26 +625,4 @@ public class UserManagerService extends IUserManager.Stub { return i; } } - - private boolean createPackageFolders(int id, File userPath) { - // mInstaller may not be available for unit-tests. - if (mInstaller == null) return true; - - // Create the user path - userPath.mkdir(); - FileUtils.setPermissions(userPath.toString(), FileUtils.S_IRWXU | FileUtils.S_IRWXG - | FileUtils.S_IXOTH, -1, -1); - - mInstaller.cloneUserData(0, id, false); - - return true; - } - - boolean removePackageFolders(int id) { - // mInstaller may not be available for unit-tests. - if (mInstaller == null) return true; - - mInstaller.removeUserDataDirs(id); - return true; - } } -- cgit v1.2.3-59-g8ed1b