diff options
author | Calin Juravle <calin@google.com> | 2020-02-18 00:04:07 +0000 |
---|---|---|
committer | Calin Juravle <calin@google.com> | 2020-02-18 10:35:10 -0800 |
commit | e050375d5aca489dc5838f9efd1585e00c6d268c (patch) | |
tree | 5384faee952b73652433983c8a0b2ee5eee4d20b | |
parent | 46641b982b48c7ec11466cd7dbfe76de6c375a66 (diff) |
Reland "Fix shared libraries not being reported"
Original commit:
This CL topic fixes shared libraries not being reporter via Reporter.
This particular CL changes the BaseDexClassLoader.Reporter interface to
report ClassLoader context strings rather than ClassLoader instances.
These ClassLoader context strings are computed directly by the runtime
(see the changes within art in the same topic).
Test: atest libcore.dalvik.system.BaseDexClassLoaderTest
Bug: 148494302
Reason for revert: Re-land
Reverted Changes:
I295a6e99e:Revert "Fix shared libraries not being reported vi...
Ib58066e8f:Revert "[DexLoadReporter] Report classloader conte...
Change-Id: I63db10d85c3316dd9f08c0d75ab34dd3f1b62d28
3 files changed, 162 insertions, 64 deletions
diff --git a/dalvik/src/main/java/dalvik/system/BaseDexClassLoader.java b/dalvik/src/main/java/dalvik/system/BaseDexClassLoader.java index d0a449bdb7..4a9e863118 100644 --- a/dalvik/src/main/java/dalvik/system/BaseDexClassLoader.java +++ b/dalvik/src/main/java/dalvik/system/BaseDexClassLoader.java @@ -24,8 +24,11 @@ import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Enumeration; +import java.util.HashMap; import java.util.List; +import java.util.Map; import sun.misc.CompoundEnumeration; /** @@ -134,28 +137,17 @@ public class BaseDexClassLoader extends ClassLoader { * Reports the current class loader chain to the registered {@code reporter}. */ private void reportClassLoaderChain() { - ArrayList<ClassLoader> classLoadersChain = new ArrayList<>(); - ArrayList<String> classPaths = new ArrayList<>(); - - classLoadersChain.add(this); - classPaths.add(String.join(File.pathSeparator, pathList.getDexPaths())); - - ClassLoader bootClassLoader = ClassLoader.getSystemClassLoader().getParent(); - ClassLoader current = getParent(); - - while (current != null && current != bootClassLoader) { - classLoadersChain.add(current); - if (current instanceof BaseDexClassLoader) { - BaseDexClassLoader bdcCurrent = (BaseDexClassLoader) current; - classPaths.add(String.join(File.pathSeparator, bdcCurrent.pathList.getDexPaths())); - } else { - // We can't determine the classpath for arbitrary class loaders. - classPaths.add(null); - } - current = current.getParent(); + String[] classPathAndClassLoaderContexts = computeClassLoaderContextsNative(); + if (classPathAndClassLoaderContexts.length == 0) { + return; } - - reporter.report(classLoadersChain, classPaths); + Map<String, String> dexFileMapping = + new HashMap<>(classPathAndClassLoaderContexts.length / 2); + for (int i = 0; i < classPathAndClassLoaderContexts.length; i += 2) { + dexFileMapping.put(classPathAndClassLoaderContexts[i], + classPathAndClassLoaderContexts[i + 1]); + } + reporter.report(Collections.unmodifiableMap(dexFileMapping)); } /** @@ -373,19 +365,16 @@ public class BaseDexClassLoader extends ClassLoader { @libcore.api.CorePlatformApi public interface Reporter { /** - * Reports the construction of a BaseDexClassLoader and provides information about the - * class loader chain. + * Reports the construction of a BaseDexClassLoader and provides opaque information about + * the class loader chain. For example, if the childmost ClassLoader in the chain: + * {@quote BaseDexClassLoader { foo.dex } -> BaseDexClassLoader { base.apk } + * -> BootClassLoader } was just initialized then the load of {@code "foo.dex"} would be + * reported with a classLoaderContext of {@code "PCL[];PCL[base.apk]"}. * - * @param classLoadersChain the chain of class loaders used during the construction of the - * class loader. The first element is the BaseDexClassLoader being constructed, - * the second element is its parent, and so on. - * @param classPaths the class paths of the class loaders present in - * {@param classLoadersChain}. The first element corresponds to the first class - * loader and so on. A classpath is represented as a list of dex files separated by - * {@code File.pathSeparator}. If the class loader is not a BaseDexClassLoader the - * classpath will be null. + * @param contextsMap A map from dex file paths to the class loader context used to load + * each dex file. */ @libcore.api.CorePlatformApi - void report(List<ClassLoader> classLoadersChain, List<String> classPaths); + void report(Map<String, String> contextsMap); } } diff --git a/luni/src/test/java/libcore/dalvik/system/BaseDexClassLoaderTest.java b/luni/src/test/java/libcore/dalvik/system/BaseDexClassLoaderTest.java index 1642a11f47..7d3b6f2b87 100644 --- a/luni/src/test/java/libcore/dalvik/system/BaseDexClassLoaderTest.java +++ b/luni/src/test/java/libcore/dalvik/system/BaseDexClassLoaderTest.java @@ -34,6 +34,7 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Enumeration; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -48,18 +49,15 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public final class BaseDexClassLoaderTest { private static class Reporter implements BaseDexClassLoader.Reporter { - public final List<ClassLoader> classLoaders = new ArrayList<>(); - public final List<String> loadedDexPaths = new ArrayList<>(); + public final Map<String, String> loadedDexMapping = new HashMap<String, String>(); @Override - public void report(List<ClassLoader> loaders, List<String> dexPaths) { - classLoaders.addAll(loaders); - loadedDexPaths.addAll(dexPaths); + public void report(Map<String, String> contextMap) { + loadedDexMapping.putAll(contextMap); } void reset() { - classLoaders.clear(); - loadedDexPaths.clear(); + loadedDexMapping.clear(); } } @@ -122,17 +120,15 @@ public final class BaseDexClassLoaderTest { BaseDexClassLoader cl1 = new PathClassLoader(jar.getPath(), ClassLoader.getSystemClassLoader()); - // Verify the reported data. - assertEquals(2, reporter.loadedDexPaths.size()); - assertEquals(2, reporter.classLoaders.size()); + assertEquals(1, reporter.loadedDexMapping.size()); - // First class loader should be the one loading the files - assertEquals(jar.getPath(), reporter.loadedDexPaths.get(0)); - assertEquals(cl1, reporter.classLoaders.get(0)); - - // Second class loader should be the system class loader. - // Don't check the actual classpath as that might vary based on system properties. - assertEquals(ClassLoader.getSystemClassLoader(), reporter.classLoaders.get(1)); + String[] contexts = reporter.loadedDexMapping.get(jar.getPath()).split(";"); + assertEquals(2, contexts.length); + // Verify the context for the loaded dex files. + assertEquals("PCL[]", contexts[0]); + // We cannot fully verify the context of the system class loader because its classpath + // may vary based on system properties and whether or not we are in a test environment. + assertTrue(contexts[1].startsWith("PCL[")); } @Test @@ -141,16 +137,10 @@ public final class BaseDexClassLoaderTest { ClassLoader unknownLoader = new ClassLoader(ClassLoader.getSystemClassLoader()) {}; BaseDexClassLoader cl1 = new PathClassLoader(jar.getPath(), unknownLoader); - assertEquals(3, reporter.loadedDexPaths.size()); - assertEquals(3, reporter.classLoaders.size()); - - assertEquals(jar.getPath(), reporter.loadedDexPaths.get(0)); - assertEquals(cl1, reporter.classLoaders.get(0)); - - assertNull(reporter.loadedDexPaths.get(1)); - assertEquals(unknownLoader, reporter.classLoaders.get(1)); - - assertEquals(ClassLoader.getSystemClassLoader(), reporter.classLoaders.get(2)); + // Verify the dex path gets reported, but with no class loader context due to the foreign + // class loader. + assertEquals(Map.of(jar.getPath(), "=UnsupportedClassLoaderContext="), + reporter.loadedDexMapping); } @Test @@ -158,8 +148,15 @@ public final class BaseDexClassLoaderTest { BaseDexClassLoader cl1 = new PathClassLoader(jar.getPath(), ClassLoader.getSystemClassLoader()); - assertEquals(2, reporter.loadedDexPaths.size()); - assertEquals(2, reporter.classLoaders.size()); + assertEquals(1, reporter.loadedDexMapping.size()); + + String[] contexts = reporter.loadedDexMapping.get(jar.getPath()).split(";"); + assertEquals(2, contexts.length); + // Verify the context for the loaded dex files. + assertEquals("PCL[]", contexts[0]); + // We cannot fully verify the context of the system class loader because its classpath + // may vary based on system properties and whether or not we are in a test environment. + assertTrue(contexts[1].startsWith("PCL[")); // Check we don't report after the reporter is unregistered. unregisterReporter(); @@ -169,8 +166,120 @@ public final class BaseDexClassLoaderTest { BaseDexClassLoader cl2 = new PathClassLoader(jar.getPath(), pcl); // Verify nothing reported - assertEquals(0, reporter.loadedDexPaths.size()); - assertEquals(0, reporter.classLoaders.size()); + assertEquals(Map.<String, String>of(), reporter.loadedDexMapping); + } + + @Test + public void testReporting_multipleJars() throws Exception { + // Load the jar file using a PathClassLoader. + BaseDexClassLoader cl1 = new PathClassLoader( + String.join(File.pathSeparator, jar.getPath(), jar2.getPath()), + ClassLoader.getSystemClassLoader()); + + assertEquals(2, reporter.loadedDexMapping.size()); + // Verify the first jar. + String[] contexts = reporter.loadedDexMapping.get(jar.getPath()).split(";"); + assertEquals(2, contexts.length); + // Verify the context for the loaded dex files. + assertEquals("PCL[]", contexts[0]); + // We cannot fully verify the context of the system class loader because its classpath + // may vary based on system properties and whether or not we are in a test environment. + assertTrue(contexts[1].startsWith("PCL[")); + + // Verify the second jar. + String[] contexts2 = reporter.loadedDexMapping.get(jar2.getPath()).split(";"); + assertEquals(2, contexts2.length); + // Verify the context for the loaded dex files. + assertEquals("PCL[" + jar.getPath() + "]", contexts2[0]); + // We cannot fully verify the context of the system class loader because its classpath + // may vary based on system properties and whether or not we are in a test environment. + assertTrue(contexts2[1].startsWith("PCL[")); + } + + + /** + * Separates the system class loader context from the rest of the context. + * Returns an array of 2 elements, where index 0 is the application class loader context + * without the system class loader and index 0 is the system class loader context. + */ + private String[] separateSystemClassLoaderContext(String context) { + int clcSeparatorIndex = context.lastIndexOf(";"); + String jarContext = context.substring(0, clcSeparatorIndex); + String systemClassLoaderContext = context.substring(clcSeparatorIndex + 1); + return new String[] {jarContext, systemClassLoaderContext}; + } + + @Test + public void testReporting_withSharedLibraries() throws Exception { + final ClassLoader parent = ClassLoader.getSystemClassLoader(); + final ClassLoader sharedLoaders[] = new ClassLoader[] { + new PathClassLoader(jar2.getPath(), /* librarySearchPath */ null, parent), + }; + // Reset so we don't get load reports from creating the shared library CL + reporter.reset(); + + BaseDexClassLoader bdcl = new PathClassLoader(jar.getPath(), null, parent, sharedLoaders); + + assertEquals(1, reporter.loadedDexMapping.size()); + + String[] contexts = separateSystemClassLoaderContext( + reporter.loadedDexMapping.get(jar.getPath())); + // We cannot fully verify the context of the system class loader because its classpath + // may vary based on system properties and whether or not we are in a test environment. + assertTrue(contexts[1].startsWith("PCL[")); + // Verify the context for the loaded dex files. The system class loader should be part + // of the shared library class loader. + assertEquals("PCL[]{PCL[" + jar2.getPath() + "];" + contexts[1] + "}", + contexts[0]); + } + + @Test + public void testReporting_multipleJars_withSharedLibraries() throws Exception { + final ClassLoader parent = ClassLoader.getSystemClassLoader(); + final String sharedJarPath = resourcesMap.get("parent.jar").getAbsolutePath(); + final ClassLoader sharedLoaders[] = new ClassLoader[] { + new PathClassLoader(sharedJarPath, /* librarySearchPath */ null, parent), + }; + // Reset so we don't get load reports from creating the shared library CL + reporter.reset(); + + BaseDexClassLoader bdcl = new PathClassLoader( + String.join(File.pathSeparator, jar.getPath(), jar2.getPath()), + null, parent, sharedLoaders); + + assertEquals(2, reporter.loadedDexMapping.size()); + + + // Verify the first jar. + String[] contexts = separateSystemClassLoaderContext( + reporter.loadedDexMapping.get(jar.getPath())); + String contextSuffix = "{PCL[" + sharedJarPath + "];" + contexts[1] + "}"; + + // We cannot fully verify the context of the system class loader because its classpath + // may vary based on system properties and whether or not we are in a test environment. + assertTrue(contexts[1].startsWith("PCL[")); + // Verify the context for the loaded dex files. + assertEquals("PCL[]" + contextSuffix, contexts[0]); + // We cannot fully verify the context of the system class loader because its classpath + // may vary based on system properties and whether or not we are in a test environment. + assertTrue(contexts[1].startsWith("PCL[")); + + // Verify the second jar. + String[] contexts2 = separateSystemClassLoaderContext( + reporter.loadedDexMapping.get(jar2.getPath())); + String contextSuffix2 = "{PCL[" + sharedJarPath + "];" + contexts2[1] + "}"; + + // Verify the context for the loaded dex files. + assertEquals("PCL[" + jar.getPath() + "]" + contextSuffix2, contexts2[0]); + // We cannot fully verify the context of the system class loader because its classpath + // may vary based on system properties and whether or not we are in a test environment. + assertTrue(contexts2[1].startsWith("PCL[")) ; + } + + @Test + public void testReporting_emptyPath() throws Exception { + BaseDexClassLoader cl1 = new PathClassLoader("", ClassLoader.getSystemClassLoader()); + assertEquals(Map.<String, String>of(), reporter.loadedDexMapping); } /* package */ static List<String> readResources(ClassLoader cl, String resourceName) diff --git a/mmodules/core_platform_api/api/platform/current-api.txt b/mmodules/core_platform_api/api/platform/current-api.txt index 707e3a3b35..1ca16bc169 100644 --- a/mmodules/core_platform_api/api/platform/current-api.txt +++ b/mmodules/core_platform_api/api/platform/current-api.txt @@ -535,7 +535,7 @@ package dalvik.system { } public static interface BaseDexClassLoader.Reporter { - method public void report(java.util.List<java.lang.ClassLoader>, java.util.List<java.lang.String>); + method public void report(java.util.Map<java.lang.String,java.lang.String>); } public final class BlockGuard { |