am df843862: am 05c1f749: Merge "Fix lock inversion in PackageInstaller." into lmp-dev

* commit 'df8438627733ce2f19f1735df535ed52be60d396':
  Fix lock inversion in PackageInstaller.

2 files changed, 31 insertions, 15 deletions

diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java
index fa09d48..c106546 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerService.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerService.java
@@ -144,6 +144,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub {
 
     private final File mStagingDir;
     private final HandlerThread mInstallThread;
+    private final Handler mInstallHandler;
 
     private final Callbacks mCallbacks;
 
@@ -188,6 +189,8 @@ public class PackageInstallerService extends IPackageInstaller.Stub {
         mInstallThread = new HandlerThread(TAG);
         mInstallThread.start();
 
+        mInstallHandler = new Handler(mInstallThread.getLooper());
+
         mCallbacks = new Callbacks(mInstallThread.getLooper());
 
         mSessionsFile = new AtomicFile(
@@ -961,13 +964,19 @@ public class PackageInstallerService extends IPackageInstaller.Stub {
             mCallbacks.notifySessionProgressChanged(session.sessionId, session.userId, progress);
         }
 
-        public void onSessionFinished(PackageInstallerSession session, boolean success) {
+        public void onSessionFinished(final PackageInstallerSession session, boolean success) {
             mCallbacks.notifySessionFinished(session.sessionId, session.userId, success);
-            synchronized (mSessions) {
-                mSessions.remove(session.sessionId);
-                mHistoricalSessions.put(session.sessionId, session);
-            }
-            writeSessionsAsync();
+
+            mInstallHandler.post(new Runnable() {
+                @Override
+                public void run() {
+                    synchronized (mSessions) {
+                        mSessions.remove(session.sessionId);
+                        mHistoricalSessions.put(session.sessionId, session);
+                        writeSessionsLocked();
+                    }
+                }
+            });
         }
 
         public void onSessionPrepared(PackageInstallerSession session) {
@@ -976,11 +985,13 @@ public class PackageInstallerService extends IPackageInstaller.Stub {
             writeSessionsAsync();
         }
 
-        public void onSessionSealed(PackageInstallerSession session) {
+        public void onSessionSealedBlocking(PackageInstallerSession session) {
             // It's very important that we block until we've recorded the
             // session as being sealed, since we never want to allow mutation
             // after sealing.
-            writeSessionsLocked();
+            synchronized (mSessions) {
+                writeSessionsLocked();
+            }
         }
     }
 }
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index 4cbc1c4..fb0e357 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -410,7 +410,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
     public void commit(IntentSender statusReceiver) {
         Preconditions.checkNotNull(statusReceiver);
 
+        final boolean wasSealed;
         synchronized (mLock) {
+            wasSealed = mSealed;
             if (!mSealed) {
                 // Verify that all writers are hands-off
                 for (FileBridge bridge : mBridges) {
@@ -418,17 +420,20 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                         throw new SecurityException("Files still open");
                     }
                 }
-
-                // Persist the fact that we've sealed ourselves to prevent
-                // mutations of any hard links we create.
                 mSealed = true;
-                mCallback.onSessionSealed(this);
             }
+
+            // Client staging is fully done at this point
+            mClientProgress = 1f;
+            computeProgressLocked(true);
         }
 
-        // Client staging is fully done at this point
-        mClientProgress = 1f;
-        computeProgressLocked(true);
+        if (!wasSealed) {
+            // Persist the fact that we've sealed ourselves to prevent
+            // mutations of any hard links we create. We do this without holding
+            // the session lock, since otherwise it's a lock inversion.
+            mCallback.onSessionSealedBlocking(this);
+        }
 
         // This ongoing commit should keep session active, even though client
         // will probably close their end.