Merge changes I130736e0,I47ad6c84

* changes:
  Protect mIncrementalFileStorages with mLock (2/n)
  Protect access to mChildSessionIds with mLock (1/n)

1 files changed, 31 insertions, 16 deletions

diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index 5f488711ceb1..ca16d5730f78 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -405,7 +405,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
 
     private boolean mDataLoaderFinished = false;
 
-    // TODO(b/159663586): should be protected by mLock
+    @GuardedBy("mLock")
     private IncrementalFileStorages mIncrementalFileStorages;
 
     private static final FileFilter sAddedApkFilter = new FileFilter() {
@@ -1125,13 +1125,18 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
             return;
         }
         if (isMultiPackage()) {
-            final SparseIntArray remainingSessions = mChildSessionIds.clone();
+            final SparseIntArray remainingSessions;
+            final int[] childSessionIds;
+            synchronized (mLock) {
+                remainingSessions = mChildSessionIds.clone();
+                childSessionIds = mChildSessionIds.copyKeys();
+            }
             final IntentSender childIntentSender =
                     new ChildStatusIntentReceiver(remainingSessions, statusReceiver)
                             .getIntentSender();
             boolean sealFailed = false;
-            for (int i = mChildSessionIds.size() - 1; i >= 0; --i) {
-                final int childSessionId = mChildSessionIds.keyAt(i);
+            for (int i = childSessionIds.length - 1; i >= 0; --i) {
+                final int childSessionId = childSessionIds[i];
                 // seal all children, regardless if any of them fail; we'll throw/return
                 // as appropriate once all children have been processed
                 if (!mSessionProvider.getSession(childSessionId)
@@ -1163,13 +1168,17 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
         }
 
         if (isMultiPackage()) {
-            int childCount = mChildSessionIds.size();
+            final int[] childSessionIds;
+            synchronized (mLock) {
+                childSessionIds = mChildSessionIds.copyKeys();
+            }
+            int childCount = childSessionIds.length;
 
             // This will contain all child sessions that do not encounter an unrecoverable failure
             ArrayList<PackageInstallerSession> nonFailingSessions = new ArrayList<>(childCount);
 
             for (int i = childCount - 1; i >= 0; --i) {
-                final int childSessionId = mChildSessionIds.keyAt(i);
+                final int childSessionId = childSessionIds[i];
                 // commit all children, regardless if any of them fail; we'll throw/return
                 // as appropriate once all children have been processed
                 try {
@@ -2588,7 +2597,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
      * Adds a child session ID without any safety / sanity checks. This should only be used to
      * build a session from XML or similar.
      */
-    void addChildSessionIdInternal(int sessionId) {
+    @GuardedBy("mLock")
+    void addChildSessionIdLocked(int sessionId) {
         mChildSessionIds.put(sessionId, 0);
     }
 
@@ -2964,7 +2974,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
 
     @Override
     public int[] getChildSessionIds() {
-        final int[] childSessionIds = mChildSessionIds.copyKeys();
+        final int[] childSessionIds;
+        synchronized (mLock) {
+            childSessionIds = mChildSessionIds.copyKeys();
+        }
         if (childSessionIds != null) {
             return childSessionIds;
         }
@@ -2990,7 +3003,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                 return;
             }
             childSession.setParentSessionId(this.sessionId);
-            addChildSessionIdInternal(childSessionId);
+            addChildSessionIdLocked(childSessionId);
         }
     }
 
@@ -3157,10 +3170,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
             for (FileBridge bridge : mBridges) {
                 bridge.forceClose();
             }
-        }
-        if (mIncrementalFileStorages != null) {
-            mIncrementalFileStorages.cleanUp();
-            mIncrementalFileStorages = null;
+            if (mIncrementalFileStorages != null) {
+                mIncrementalFileStorages.cleanUp();
+                mIncrementalFileStorages = null;
+            }
         }
         // For staged sessions, we don't delete the directory where the packages have been copied,
         // since these packages are supposed to be read on reboot.
@@ -3197,9 +3210,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
     }
 
     private void cleanStageDir() {
-        if (mIncrementalFileStorages != null) {
-            mIncrementalFileStorages.cleanUp();
-            mIncrementalFileStorages = null;
+        synchronized (mLock) {
+            if (mIncrementalFileStorages != null) {
+                mIncrementalFileStorages.cleanUp();
+                mIncrementalFileStorages = null;
+            }
         }
         try {
             mPm.mInstaller.rmPackageDir(stageDir.getAbsolutePath());