From 14062f2382388a3a15802bf8b8b24d1f41d548c6 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Mon, 2 Oct 2017 10:23:25 +0100 Subject: [PATCH] Preserve order of shared library files Shared libraries are stored in a list so order is preserved. However, when they are resolved to files, e.g. for use as a class path the file names are added to an ArraySet which loses the order. Presumably they are added to a Set to eliminate duplicates. This switches to a LinkedHashSet which will preserve the order in which the files are added while still avoiding duplicates. It is possible that this could cause app compatibility issues as the order in which shared libraries is being added is changing. Problems can only arise if two libraries whose order changes have duplicate classes and/or resources. In that case the app was only working by luck, as the order provided by ArraySet is based on the numerical order of hash codes. This was found while investigating performance regressions in GoogleDialer, unfortunately it does not fix the regressions. Bug: 65552462 Test: flash -w and systrace GoogleDialer to ensure correct order Change-Id: I0e94471cc481437712f7cf0dab63d88f50cf3b14 Merged-In: Ia01ce4821fa53e4785716b72c4f87a0b0ab4dcc8 --- .../server/pm/PackageManagerService.java | 17 ++++++++++++----- 1 file changed, 12 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 2b3bfc82f9828..2a1270fc585c5 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -325,6 +325,7 @@ import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -9846,7 +9847,8 @@ public class PackageManagerService extends IPackageManager.Stub } } - private void addSharedLibraryLPr(ArraySet usesLibraryFiles, SharedLibraryEntry file, + private void addSharedLibraryLPr(Set usesLibraryFiles, + SharedLibraryEntry file, PackageParser.Package changingLib) { if (file.path != null) { usesLibraryFiles.add(file.path); @@ -9875,7 +9877,10 @@ public class PackageManagerService extends IPackageManager.Stub if (pkg == null) { return; } - ArraySet usesLibraryFiles = null; + // The collection used here must maintain the order of addition (so + // that libraries are searched in the correct order) and must have no + // duplicates. + Set usesLibraryFiles = null; if (pkg.usesLibraries != null) { usesLibraryFiles = addSharedLibrariesLPw(pkg.usesLibraries, null, null, pkg.packageName, changingLib, true, null); @@ -9896,10 +9901,10 @@ public class PackageManagerService extends IPackageManager.Stub } } - private ArraySet addSharedLibrariesLPw(@NonNull List requestedLibraries, + private Set addSharedLibrariesLPw(@NonNull List requestedLibraries, @Nullable int[] requiredVersions, @Nullable String[] requiredCertDigests, @NonNull String packageName, @Nullable PackageParser.Package changingLib, - boolean required, @Nullable ArraySet outUsedLibraries) + boolean required, @Nullable Set outUsedLibraries) throws PackageManagerException { final int libCount = requestedLibraries.size(); for (int i = 0; i < libCount; i++) { @@ -9944,7 +9949,9 @@ public class PackageManagerService extends IPackageManager.Stub } if (outUsedLibraries == null) { - outUsedLibraries = new ArraySet<>(); + // Use LinkedHashSet to preserve the order of files added to + // usesLibraryFiles while eliminating duplicates. + outUsedLibraries = new LinkedHashSet<>(); } addSharedLibraryLPr(outUsedLibraries, libEntry, changingLib); }