From a9bf579ea12d10128a0baa9a352bca72c767e9d3 Mon Sep 17 00:00:00 2001 From: Winson Date: Mon, 1 Apr 2019 13:29:12 -0700 Subject: [PATCH 1/2] Defer post-install resource clean up for 5 seconds There is an inherent problem with deleting files on disk while an app is running, since it might still try to access code/resources in the deleted files. We cannot instantaneously update all application threads to the new paths without blocking the world, so keep the old ones around for an arbitrary amount of time. As mentioned in the linked bug, if PackageManager boots with duplicated app directories, it will automatically clean itself up, so we are not concerned with the case where the deferred delete message doesn't get executed in the current device boot cycle. 5 seconds was chosen with no reasoning beyond it feels safe. Bug: 129291162 Test: manual reboot device with invalid dirs on disk Test: manual dynamic feature delivery app with --dont-kill split Merged-In: Id249a185a8556ebf133be3c36b1fec3f14b588ac Change-Id: Id249a185a8556ebf133be3c36b1fec3f14b588ac (cherry picked from commit 87380366ecf60f2cb2697cb26e07ccaf666b45e6) --- .../server/pm/PackageManagerService.java | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index c4d4106804e15..f03240072807b 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -1319,6 +1319,9 @@ public class PackageManagerService extends IPackageManager.Stub static final int INSTANT_APP_RESOLUTION_PHASE_TWO = 20; static final int ENABLE_ROLLBACK_STATUS = 21; static final int ENABLE_ROLLBACK_TIMEOUT = 22; + static final int DEFERRED_NO_KILL_POST_DELETE = 23; + + static final int DEFERRED_NO_KILL_POST_DELETE_DELAY_MS = 5 * 1000; static final int WRITE_SETTINGS_DELAY = 10*1000; // 10 seconds @@ -1525,6 +1528,12 @@ public class PackageManagerService extends IPackageManager.Stub Trace.asyncTraceEnd(TRACE_TAG_PACKAGE_MANAGER, "postInstall", msg.arg1); } break; + case DEFERRED_NO_KILL_POST_DELETE: { + synchronized (mInstallLock) { + InstallArgs args = (InstallArgs) msg.obj; + args.doPostDeleteLI(true); + } + } break; case WRITE_SETTINGS: { Process.setThreadPriority(Process.THREAD_PRIORITY_DEFAULT); synchronized (mPackages) { @@ -2029,11 +2038,18 @@ public class PackageManagerService extends IPackageManager.Stub getUnknownSourcesSettings()); // Remove the replaced package's older resources safely now - // We delete after a gc for applications on sdcard. - if (res.removedInfo != null && res.removedInfo.args != null) { - Runtime.getRuntime().gc(); - synchronized (mInstallLock) { - res.removedInfo.args.doPostDeleteLI(true); + InstallArgs args = res.removedInfo != null ? res.removedInfo.args : null; + if (args != null) { + if (!killApp) { + // If we didn't kill the app, defer the deletion of code/resource files, since + // they may still be in use by the running application. This mitigates problems + // in cases where resources or code is loaded by a new Activity before + // ApplicationInfo changes have propagated to all application threads. + scheduleDeferredNoKillPostDelete(args); + } else { + synchronized (mInstallLock) { + args.doPostDeleteLI(true); + } } } else { // Force a gc to clear up things. Ask for a background one, it's fine to go on @@ -2068,6 +2084,11 @@ public class PackageManagerService extends IPackageManager.Stub } } + private void scheduleDeferredNoKillPostDelete(InstallArgs args) { + Message message = mHandler.obtainMessage(DEFERRED_NO_KILL_POST_DELETE, args); + mHandler.sendMessageDelayed(message, DEFERRED_NO_KILL_POST_DELETE_DELAY_MS); + } + /** * Gets the type of the external storage a package is installed on. * @param packageVolume The storage volume of the package. From 23863be910ae5ef929639a3d9b0c8343305ce42b Mon Sep 17 00:00:00 2001 From: Winson Date: Thu, 4 Apr 2019 17:41:28 -0700 Subject: [PATCH 2/2] Delayed install completion This change is an attempt at delaying the install complete callback to the installer on a DONT_KILL install until the package indicates that it as completed updating its classpath with the new APKs. Bug: 80269951 Bug: 109751013 Test: manual test of dynamic delivery/instant apps Merged-In: I689ec523522da37987cff9b1a67eaae9e5633ffb Change-Id: I689ec523522da37987cff9b1a67eaae9e5633ffb (cherry picked from commit abfc054c9065f96ae34023119623bdd310cb6e48) --- core/java/android/app/ActivityThread.java | 17 ++++- .../android/content/pm/IPackageManager.aidl | 2 + .../com/android/server/am/ProcessList.java | 13 ++++ .../server/pm/PackageManagerService.java | 71 ++++++++++++++++--- 4 files changed, 91 insertions(+), 12 deletions(-) diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index 474f25bd5533d..a83f91e360df2 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -5718,14 +5718,18 @@ public final class ActivityThread extends ClientTransactionHandler { if (packages == null) { break; } + + List packagesHandled = new ArrayList<>(); + synchronized (mResourcesManager) { for (int i = packages.length - 1; i >= 0; i--) { - WeakReference ref = mPackages.get(packages[i]); + String packageName = packages[i]; + WeakReference ref = mPackages.get(packageName); LoadedApk pkgInfo = ref != null ? ref.get() : null; if (pkgInfo != null) { hasPkgInfo = true; } else { - ref = mResourcePackages.get(packages[i]); + ref = mResourcePackages.get(packageName); pkgInfo = ref != null ? ref.get() : null; if (pkgInfo != null) { hasPkgInfo = true; @@ -5736,8 +5740,8 @@ public final class ActivityThread extends ClientTransactionHandler { // Adjust it's internal references to the application info and // resources. if (pkgInfo != null) { + packagesHandled.add(packageName); try { - final String packageName = packages[i]; final ApplicationInfo aInfo = sPackageManager.getApplicationInfo( packageName, @@ -5769,6 +5773,13 @@ public final class ActivityThread extends ClientTransactionHandler { } } } + + try { + getPackageManager().notifyPackagesReplacedReceived( + packagesHandled.toArray(new String[0])); + } catch (RemoteException ignored) { + } + break; } } diff --git a/core/java/android/content/pm/IPackageManager.aidl b/core/java/android/content/pm/IPackageManager.aidl index cf704d52cba05..6ab4657d727de 100644 --- a/core/java/android/content/pm/IPackageManager.aidl +++ b/core/java/android/content/pm/IPackageManager.aidl @@ -770,4 +770,6 @@ interface IPackageManager { int getRuntimePermissionsVersion(int userId); void setRuntimePermissionsVersion(int version, int userId); + + void notifyPackagesReplacedReceived(in String[] packages); } diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java index 1bcc4c8a6d085..a93f2187db082 100644 --- a/services/core/java/com/android/server/am/ProcessList.java +++ b/services/core/java/com/android/server/am/ProcessList.java @@ -3172,15 +3172,28 @@ public final class ProcessList { @GuardedBy("mService") void sendPackageBroadcastLocked(int cmd, String[] packages, int userId) { + boolean foundProcess = false; for (int i = mLruProcesses.size() - 1; i >= 0; i--) { ProcessRecord r = mLruProcesses.get(i); if (r.thread != null && (userId == UserHandle.USER_ALL || r.userId == userId)) { try { + for (int index = packages.length - 1; index >= 0 && !foundProcess; index--) { + if (packages[index].equals(r.info.packageName)) { + foundProcess = true; + } + } r.thread.dispatchPackageBroadcast(cmd, packages); } catch (RemoteException ex) { } } } + + if (!foundProcess) { + try { + AppGlobals.getPackageManager().notifyPackagesReplacedReceived(packages); + } catch (RemoteException ignored) { + } + } } /** Returns the uid's process state or PROCESS_STATE_NONEXISTENT if not running */ diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index f03240072807b..3c336eac1ae68 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -952,6 +952,9 @@ public class PackageManagerService extends IPackageManager.Stub ActivityInfo mInstantAppInstallerActivity; final ResolveInfo mInstantAppInstallerInfo = new ResolveInfo(); + private final Map> + mNoKillInstallObservers = Collections.synchronizedMap(new HashMap<>()); + final SparseArray mIntentFilterVerificationStates = new SparseArray<>(); @@ -1320,8 +1323,10 @@ public class PackageManagerService extends IPackageManager.Stub static final int ENABLE_ROLLBACK_STATUS = 21; static final int ENABLE_ROLLBACK_TIMEOUT = 22; static final int DEFERRED_NO_KILL_POST_DELETE = 23; + static final int DEFERRED_NO_KILL_INSTALL_OBSERVER = 24; - static final int DEFERRED_NO_KILL_POST_DELETE_DELAY_MS = 5 * 1000; + static final int DEFERRED_NO_KILL_POST_DELETE_DELAY_MS = 3 * 1000; + static final int DEFERRED_NO_KILL_INSTALL_OBSERVER_DELAY_MS = 500; static final int WRITE_SETTINGS_DELAY = 10*1000; // 10 seconds @@ -1531,7 +1536,15 @@ public class PackageManagerService extends IPackageManager.Stub case DEFERRED_NO_KILL_POST_DELETE: { synchronized (mInstallLock) { InstallArgs args = (InstallArgs) msg.obj; - args.doPostDeleteLI(true); + if (args != null) { + args.doPostDeleteLI(true); + } + } + } break; + case DEFERRED_NO_KILL_INSTALL_OBSERVER: { + String packageName = (String) msg.obj; + if (packageName != null) { + notifyInstallObserver(packageName); } } break; case WRITE_SETTINGS: { @@ -1800,7 +1813,10 @@ public class PackageManagerService extends IPackageManager.Stub String[] grantedPermissions, List whitelistedRestrictedPermissions, boolean launchedForRestore, String installerPackage, IPackageInstallObserver2 installObserver) { - if (res.returnCode == PackageManager.INSTALL_SUCCEEDED) { + final boolean succeeded = res.returnCode == PackageManager.INSTALL_SUCCEEDED; + final boolean update = res.removedInfo != null && res.removedInfo.removedPackage != null; + + if (succeeded) { // Send the removed broadcasts if (res.removedInfo != null) { res.removedInfo.sendPackageRemovedBroadcasts(killApp); @@ -1828,8 +1844,6 @@ public class PackageManagerService extends IPackageManager.Stub mPermissionCallback); } - final boolean update = res.removedInfo != null - && res.removedInfo.removedPackage != null; final String installerPackageName = res.installerPackageName != null ? res.installerPackageName @@ -2072,12 +2086,43 @@ public class PackageManagerService extends IPackageManager.Stub } } - // If someone is watching installs - notify them + final boolean deferInstallObserver = succeeded && update && !killApp; + if (deferInstallObserver) { + scheduleDeferredNoKillInstallObserver(res, installObserver); + } else { + notifyInstallObserver(res, installObserver); + } + } + + @Override + public void notifyPackagesReplacedReceived(String[] packages) { + final int callingUid = Binder.getCallingUid(); + final int callingUserId = UserHandle.getUserId(callingUid); + + for (String packageName : packages) { + PackageSetting setting = mSettings.mPackages.get(packageName); + if (setting != null && filterAppAccessLPr(setting, callingUid, callingUserId)) { + notifyInstallObserver(packageName); + } + } + } + + private void notifyInstallObserver(String packageName) { + Pair pair = + mNoKillInstallObservers.remove(packageName); + + if (pair != null) { + notifyInstallObserver(pair.first, pair.second); + } + } + + private void notifyInstallObserver(PackageInstalledInfo info, + IPackageInstallObserver2 installObserver) { if (installObserver != null) { try { - Bundle extras = extrasForInstallResult(res); - installObserver.onPackageInstalled(res.name, res.returnCode, - res.returnMsg, extras); + Bundle extras = extrasForInstallResult(info); + installObserver.onPackageInstalled(info.name, info.returnCode, + info.returnMsg, extras); } catch (RemoteException e) { Slog.i(TAG, "Observer no longer exists."); } @@ -2089,6 +2134,14 @@ public class PackageManagerService extends IPackageManager.Stub mHandler.sendMessageDelayed(message, DEFERRED_NO_KILL_POST_DELETE_DELAY_MS); } + private void scheduleDeferredNoKillInstallObserver(PackageInstalledInfo info, + IPackageInstallObserver2 observer) { + String packageName = info.pkg.packageName; + mNoKillInstallObservers.put(packageName, Pair.create(info, observer)); + Message message = mHandler.obtainMessage(DEFERRED_NO_KILL_INSTALL_OBSERVER, packageName); + mHandler.sendMessageDelayed(message, DEFERRED_NO_KILL_INSTALL_OBSERVER_DELAY_MS); + } + /** * Gets the type of the external storage a package is installed on. * @param packageVolume The storage volume of the package.