commit 64067b25b1160117aca1def5889e57d78011df3e
author Alex Buynytskyy <alexbuy@google.com> Sat Apr 25 15:56:52 2020 -0700
committer Alex Buynytskyy <alexbuy@google.com> Mon Apr 27 11:06:29 2020 -0700
tree 8d7b762ac7e9c1e1a1813652bb3b9dfd82403be8
parent 03a2c3833d4b872061bf83956191470abe55b241

Fix for Binder one-way deadlock.

Dispatch session destroy to be executed later.
We can't do anything blocking during one-way transaction processing.
This blocks all one-way transactions to this Binder, including other
status updates.

Bug: b/153874006
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest
Test: atest IncrementalInstallerTest#testIncrementalInstallFails_onPrepareImageReturnsFalse
Change-Id: Ie4385d603fca8b5205775e849e92846cff672f56

services/core/java/com/android/server/pm/PackageInstallerSession.java

diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index 330f4b3..d9275f5 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -163,6 +163,7 @@
     private static final int MSG_STREAM_VALIDATE_AND_COMMIT = 1;
     private static final int MSG_INSTALL = 2;
     private static final int MSG_ON_PACKAGE_INSTALLED = 3;
+    private static final int MSG_SESSION_VERIFICATION_FAILURE = 4;
 
     /** XML constants used for persisting a session */
     static final String TAG_SESSION = "session";
@@ -449,6 +450,11 @@
                             packageName, returnCode, message, extras);
 
                     break;
+                case MSG_SESSION_VERIFICATION_FAILURE:
+                    final int error = msg.arg1;
+                    final String detailMessage = (String) msg.obj;
+                    onSessionVerificationFailure(error, detailMessage);
+                    break;
             }
 
             return true;
@@ -1479,12 +1485,15 @@
     }
 
     private PackageManagerException onSessionVerificationFailure(PackageManagerException e) {
+        onSessionVerificationFailure(e.error, ExceptionUtils.getCompleteMessage(e));
+        return e;
+    }
+
+    private void onSessionVerificationFailure(int error, String detailMessage) {
         // Session is sealed but could not be verified, we need to destroy it.
         destroyInternal();
         // Dispatch message to remove session from PackageInstallerService.
-        dispatchSessionFinished(e.error, ExceptionUtils.getCompleteMessage(e), null);
-
-        return e;
+        dispatchSessionFinished(error, detailMessage, null);
     }
 
     private void onDataLoaderUnrecoverable() {
@@ -2629,9 +2638,8 @@
                     IDataLoader dataLoader = dataLoaderManager.getDataLoader(dataLoaderId);
                     if (dataLoader == null) {
                         mDataLoaderFinished = true;
-                        onSessionVerificationFailure(
-                                new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE,
-                                        "Failure to obtain data loader"));
+                        dispatchSessionVerificationFailure(INSTALL_FAILED_MEDIA_UNAVAILABLE,
+                                "Failure to obtain data loader");
                         return;
                     }
 
@@ -2676,9 +2684,8 @@
                         }
                         case IDataLoaderStatusListener.DATA_LOADER_IMAGE_NOT_READY: {
                             mDataLoaderFinished = true;
-                            onSessionVerificationFailure(
-                                    new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE,
-                                            "Failed to prepare image."));
+                            dispatchSessionVerificationFailure(INSTALL_FAILED_MEDIA_UNAVAILABLE,
+                                    "Failed to prepare image.");
                             if (manualStartAndDestroy) {
                                 dataLoader.destroy(dataLoaderId);
                             }
@@ -2686,9 +2693,8 @@
                         }
                         case IDataLoaderStatusListener.DATA_LOADER_UNRECOVERABLE:
                             mDataLoaderFinished = true;
-                            onSessionVerificationFailure(
-                                    new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE,
-                                            "DataLoader reported unrecoverable failure."));
+                            dispatchSessionVerificationFailure(INSTALL_FAILED_MEDIA_UNAVAILABLE,
+                                    "DataLoader reported unrecoverable failure.");
                             return;
                     }
                 } catch (RemoteException e) {
@@ -2720,6 +2726,11 @@
         return false;
     }
 
+    private void dispatchSessionVerificationFailure(int error, String detailMessage) {
+        mHandler.obtainMessage(MSG_SESSION_VERIFICATION_FAILURE, error, -1,
+                detailMessage).sendToTarget();
+    }
+
     @Override
     public int[] getChildSessionIds() {
         final int[] childSessionIds = mChildSessionIds.copyKeys();

services/incremental/IncrementalService.cpp

diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index cae5027..1036858 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -761,10 +761,7 @@
     std::unique_lock l2(ifs->lock);
     if (ifs->bindPoints.size() <= 1) {
         ifs->bindPoints.clear();
-        std::thread([this, ifs, l2 = std::move(l2)]() mutable {
-            mJni->initializeForCurrentThread();
-            deleteStorageLocked(*ifs, std::move(l2));
-        }).detach();
+        deleteStorageLocked(*ifs, std::move(l2));
     } else {
         const std::string savedFile = std::move(bindIt->second.savedFilename);
         ifs->bindPoints.erase(bindIt);