Merge "Don't hold install lock while hashing dynamic code files."

3 files changed, 15 insertions, 18 deletions

diff --git a/services/core/java/com/android/server/pm/dex/DexLogger.java b/services/core/java/com/android/server/pm/dex/DexLogger.java
index 68a755b382ca..78fa82c6bcdd 100644
--- a/services/core/java/com/android/server/pm/dex/DexLogger.java
+++ b/services/core/java/com/android/server/pm/dex/DexLogger.java
@@ -28,7 +28,6 @@ import android.util.PackageUtils;
 import android.util.Slog;
 import android.util.SparseArray;
 
-import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.server.pm.Installer;
 import com.android.server.pm.Installer.InstallerException;
@@ -53,21 +52,18 @@ public class DexLogger {
 
     private final IPackageManager mPackageManager;
     private final PackageDynamicCodeLoading mPackageDynamicCodeLoading;
-    private final Object mInstallLock;
-    @GuardedBy("mInstallLock")
     private final Installer mInstaller;
 
-    public DexLogger(IPackageManager pms, Installer installer, Object installLock) {
-        this(pms, installer, installLock, new PackageDynamicCodeLoading());
+    public DexLogger(IPackageManager pms, Installer installer) {
+        this(pms, installer, new PackageDynamicCodeLoading());
     }
 
     @VisibleForTesting
-    DexLogger(IPackageManager pms, Installer installer, Object installLock,
+    DexLogger(IPackageManager pms, Installer installer,
             PackageDynamicCodeLoading packageDynamicCodeLoading) {
         mPackageManager = pms;
         mPackageDynamicCodeLoading = packageDynamicCodeLoading;
         mInstaller = installer;
-        mInstallLock = installLock;
     }
 
     public Set<String> getAllPackagesWithDynamicCodeLoading() {
@@ -131,14 +127,16 @@ public class DexLogger {
             }
 
             byte[] hash = null;
-            synchronized (mInstallLock) {
-                try {
-                    hash = mInstaller.hashSecondaryDexFile(filePath, packageName, appInfo.uid,
-                            appInfo.volumeUuid, storageFlags);
-                } catch (InstallerException e) {
-                    Slog.e(TAG, "Got InstallerException when hashing file " + filePath
-                            + ": " + e.getMessage());
-                }
+            try {
+                // Note that we do not take the install lock here. Hashing should never interfere
+                // with app update/compilation/removal. We may get anomalous results if a file
+                // changes while we hash it, but that can happen anyway and is harmless for our
+                // purposes.
+                hash = mInstaller.hashSecondaryDexFile(filePath, packageName, appInfo.uid,
+                        appInfo.volumeUuid, storageFlags);
+            } catch (InstallerException e) {
+                Slog.e(TAG, "Got InstallerException when hashing file " + filePath
+                        + ": " + e.getMessage());
             }
 
             String fileName = new File(filePath).getName();
diff --git a/services/core/java/com/android/server/pm/dex/DexManager.java b/services/core/java/com/android/server/pm/dex/DexManager.java
index e57d9d7ab61c..b54683673e7b 100644
--- a/services/core/java/com/android/server/pm/dex/DexManager.java
+++ b/services/core/java/com/android/server/pm/dex/DexManager.java
@@ -129,7 +129,7 @@ public class DexManager {
         mPackageDexOptimizer = pdo;
         mInstaller = installer;
         mInstallLock = installLock;
-        mDexLogger = new DexLogger(pms, installer, installLock);
+        mDexLogger = new DexLogger(pms, installer);
     }
 
     public DexLogger getDexLogger() {
diff --git a/services/tests/servicestests/src/com/android/server/pm/dex/DexLoggerTests.java b/services/tests/servicestests/src/com/android/server/pm/dex/DexLoggerTests.java
index 3b6b48b6aa3f..f817e8e33b31 100644
--- a/services/tests/servicestests/src/com/android/server/pm/dex/DexLoggerTests.java
+++ b/services/tests/servicestests/src/com/android/server/pm/dex/DexLoggerTests.java
@@ -77,7 +77,6 @@ public class DexLoggerTests {
 
     @Mock IPackageManager mPM;
     @Mock Installer mInstaller;
-    private final Object mInstallLock = new Object();
 
     private PackageDynamicCodeLoading mPackageDynamicCodeLoading;
     private DexLogger mDexLogger;
@@ -103,7 +102,7 @@ public class DexLoggerTests {
         };
 
         // For test purposes capture log messages as well as sending to the event log.
-        mDexLogger = new DexLogger(mPM, mInstaller, mInstallLock, mPackageDynamicCodeLoading) {
+        mDexLogger = new DexLogger(mPM, mInstaller, mPackageDynamicCodeLoading) {
             @Override
                 void writeDclEvent(int uid, String message) {
                     super.writeDclEvent(uid, message);