# HG changeset patch # User redestad # Date 1412950575 -7200 # Fri Oct 10 16:16:15 2014 +0200 # Node ID 4b012965555cb3f44cda68bcecb3724f7c809db8 # Parent 4110a76278578a8c21bb9a0767e66d6deb380fcb 8060130: Simplify the synchronization of defining and getting java.lang.Package Reviewed-by: mchung, plevart, shade diff --git a/src/java.base/share/classes/java/lang/ClassLoader.java b/src/java.base/share/classes/java/lang/ClassLoader.java --- a/src/java.base/share/classes/java/lang/ClassLoader.java +++ b/src/java.base/share/classes/java/lang/ClassLoader.java @@ -29,12 +29,10 @@ import java.io.File; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; -import java.net.MalformedURLException; import java.net.URL; import java.security.AccessController; import java.security.AccessControlContext; import java.security.CodeSource; -import java.security.Policy; import java.security.PrivilegedAction; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; @@ -54,7 +52,6 @@ import sun.misc.CompoundEnumeration; import sun.misc.Resource; import sun.misc.URLClassPath; -import sun.misc.VM; import sun.reflect.CallerSensitive; import sun.reflect.Reflection; import sun.reflect.misc.ReflectUtil; @@ -268,8 +265,8 @@ // The packages defined in this class loader. Each package name is mapped // to its corresponding Package object. - // @GuardedBy("itself") - private final HashMap packages = new HashMap<>(); + private final ConcurrentHashMap packages + = new ConcurrentHashMap<>(); private static Void checkCreateClassLoader() { SecurityManager security = System.getSecurityManager(); @@ -1575,17 +1572,17 @@ String implVendor, URL sealBase) throws IllegalArgumentException { - synchronized (packages) { - Package pkg = getPackage(name); - if (pkg != null) { - throw new IllegalArgumentException(name); - } - pkg = new Package(name, specTitle, specVersion, specVendor, - implTitle, implVersion, implVendor, - sealBase, this); - packages.put(name, pkg); - return pkg; + Package pkg = getPackage(name); + if (pkg != null) { + throw new IllegalArgumentException(name); } + pkg = new Package(name, specTitle, specVersion, specVendor, + implTitle, implVersion, implVendor, + sealBase, this); + if (packages.putIfAbsent(name, pkg) != null) { + throw new IllegalArgumentException(name); + } + return pkg; } /** @@ -1601,26 +1598,13 @@ * @since 1.2 */ protected Package getPackage(String name) { - Package pkg; - synchronized (packages) { - pkg = packages.get(name); - } + Package pkg = packages.get(name); if (pkg == null) { if (parent != null) { pkg = parent.getPackage(name); } else { pkg = Package.getSystemPackage(name); } - if (pkg != null) { - synchronized (packages) { - Package pkg2 = packages.get(name); - if (pkg2 == null) { - packages.put(name, pkg); - } else { - pkg = pkg2; - } - } - } } return pkg; } @@ -1635,22 +1619,18 @@ * @since 1.2 */ protected Package[] getPackages() { - Map map; - synchronized (packages) { - map = new HashMap<>(packages); - } Package[] pkgs; if (parent != null) { pkgs = parent.getPackages(); } else { pkgs = Package.getSystemPackages(); } + + Map map = packages; if (pkgs != null) { + map = new HashMap<>(packages); for (Package pkg : pkgs) { - String pkgName = pkg.getName(); - if (map.get(pkgName) == null) { - map.put(pkgName, pkg); - } + map.putIfAbsent(pkg.getName(), pkg); } } return map.values().toArray(new Package[map.size()]); diff --git a/src/java.base/share/classes/java/lang/Package.java b/src/java.base/share/classes/java/lang/Package.java --- a/src/java.base/share/classes/java/lang/Package.java +++ b/src/java.base/share/classes/java/lang/Package.java @@ -26,27 +26,21 @@ package java.lang; import java.lang.reflect.AnnotatedElement; -import java.io.InputStream; -import java.util.Enumeration; -import java.util.StringTokenizer; import java.io.File; import java.io.FileInputStream; -import java.io.FileNotFoundException; import java.io.IOException; import java.net.URL; import java.net.MalformedURLException; import java.security.AccessController; import java.security.PrivilegedAction; +import java.util.concurrent.ConcurrentHashMap; import java.util.jar.JarInputStream; import java.util.jar.Manifest; import java.util.jar.Attributes; import java.util.jar.Attributes.Name; -import java.util.jar.JarException; import java.util.Map; -import java.util.HashMap; -import java.util.Iterator; import sun.net.www.ParseUtil; import sun.reflect.CallerSensitive; @@ -538,17 +532,15 @@ * Returns the loaded system package for the specified name. */ static Package getSystemPackage(String name) { - synchronized (pkgs) { - Package pkg = pkgs.get(name); - if (pkg == null) { - name = name.replace('.', '/').concat("/"); - String fn = getSystemPackage0(name); - if (fn != null) { - pkg = defineSystemPackage(name, fn); - } + Package pkg = pkgs.get(name); + if (pkg == null) { + name = name.replace('.', '/').concat("/"); + String fn = getSystemPackage0(name); + if (fn != null) { + pkg = defineSystemPackage(name, fn); } - return pkg; } + return pkg; } /* @@ -557,74 +549,98 @@ static Package[] getSystemPackages() { // First, update the system package map with new package names String[] names = getSystemPackages0(); - synchronized (pkgs) { - for (String name : names) { + for (String name : names) { + if (!pkgs.containsKey(name)) { defineSystemPackage(name, getSystemPackage0(name)); } - return pkgs.values().toArray(new Package[pkgs.size()]); } + return pkgs.values().toArray(new Package[pkgs.size()]); } private static Package defineSystemPackage(final String iname, final String fn) { - return AccessController.doPrivileged(new PrivilegedAction() { - public Package run() { - String name = iname; - // Get the cached code source url for the file name - URL url = urls.get(fn); - if (url == null) { - // URL not found, so create one - File file = new File(fn); - try { - url = ParseUtil.fileToEncodedURL(file); - } catch (MalformedURLException e) { - } - if (url != null) { - urls.put(fn, url); - // If loading a JAR file, then also cache the manifest - if (file.isFile()) { - mans.put(fn, loadManifest(fn)); - } - } - } - // Convert to "."-separated package name - name = name.substring(0, name.length() - 1).replace('/', '.'); - Package pkg; - Manifest man = mans.get(fn); - if (man != null) { - pkg = new Package(name, man, url, null); - } else { - pkg = new Package(name, null, null, null, - null, null, null, null, null); - } - pkgs.put(name, pkg); - return pkg; - } - }); + // Convert to "."-separated package name + String name = iname.substring(0, iname.length() - 1).replace('/', '.'); + // Creates a cached manifest for the file name, allowing + // only-once, lazy reads of manifest from jar files + CachedManifest cachedManifest = createCachedManifest(fn); + pkgs.putIfAbsent(name, new Package(name, cachedManifest.getManifest(), + cachedManifest.getURL(), null)); + // Ensure we only expose one Package object + return pkgs.get(name); } - /* - * Returns the Manifest for the specified JAR file name. - */ - private static Manifest loadManifest(String fn) { - try (FileInputStream fis = new FileInputStream(fn); - JarInputStream jis = new JarInputStream(fis, false)) - { - return jis.getManifest(); - } catch (IOException e) { - return null; + private static CachedManifest createCachedManifest(String fn) { + if (!manifests.containsKey(fn)) { + manifests.putIfAbsent(fn, new CachedManifest(fn)); } + return manifests.get(fn); } // The map of loaded system packages - private static Map pkgs = new HashMap<>(31); + private static final ConcurrentHashMap pkgs + = new ConcurrentHashMap<>(); - // Maps each directory or zip file name to its corresponding url - private static Map urls = new HashMap<>(10); + // Maps each directory or zip file name to its corresponding manifest, if + // it exists + private static final ConcurrentHashMap manifests + = new ConcurrentHashMap<>(); - // Maps each code source url for a jar file to its manifest - private static Map mans = new HashMap<>(10); + private static class CachedManifest { + private static final Manifest EMPTY_MANIFEST = new Manifest(); + private final String fileName; + private final URL url; + private volatile Manifest manifest; + + CachedManifest(final String fileName) { + this.fileName = fileName; + this.url = AccessController.doPrivileged(new PrivilegedAction() { + public URL run() { + final File file = new File(fileName); + if (file.isFile()) { + try { + return ParseUtil.fileToEncodedURL(file); + } catch (MalformedURLException e) { + } + } + return null; + } + }); + } + + public URL getURL() { + return url; + } + + public Manifest getManifest() { + if (url == null) { + return EMPTY_MANIFEST; + } + Manifest m = manifest; + if (m != null) { + return m; + } + synchronized (this) { + m = manifest; + if (m != null) { + return m; + } + m = AccessController.doPrivileged(new PrivilegedAction() { + public Manifest run() { + try (FileInputStream fis = new FileInputStream(fileName); + JarInputStream jis = new JarInputStream(fis, false)) { + return jis.getManifest(); + } catch (IOException e) { + return null; + } + } + }); + manifest = m = (m == null ? EMPTY_MANIFEST : m); + } + return m; + } + } private static native String getSystemPackage0(String name); private static native String[] getSystemPackages0(); diff --git a/test/java/lang/ClassLoader/GetSystemPackage.java b/test/java/lang/ClassLoader/GetSystemPackage.java new file mode 100644 --- /dev/null +++ b/test/java/lang/ClassLoader/GetSystemPackage.java @@ -0,0 +1,211 @@ +/* + * Copyright (c) 1999, 2014 Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8060130 + * @library /lib/testlibrary + * @build package2.Class2 GetSystemPackage jdk.testlibrary.* + * @summary Test if getSystemPackage() return consistent values for cases + * where a manifest is provided or not and ensure only jars on + * bootclasspath gets resolved via Package.getSystemPackage + * @run main GetSystemPackage + */ + +import java.lang.Exception; +import java.lang.InterruptedException; +import java.lang.Package; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.lang.Process; +import java.lang.ProcessBuilder; +import java.lang.RuntimeException; +import java.lang.String; +import java.util.jar.Attributes; +import java.util.jar.JarEntry; +import java.util.jar.JarOutputStream; +import java.util.jar.Manifest; +import jdk.testlibrary.ProcessTools; + +public class GetSystemPackage { + + static final String testClassesDir = System.getProperty("test.classes", "."); + static final File tmpFolder = new File(testClassesDir); + static final String manifestTitle = "Special JAR"; + + public static void main(String ... args) throws Exception { + if (args.length == 0) { + buildJarsAndInitiateSystemPackageTest(); + return; + } + switch (args[0]) { + case "system-manifest": + verifyPackage(true, true); + break; + case "system-no-manifest": + verifyPackage(false, true); + break; + case "non-system-manifest": + verifyPackage(true, false); + break; + case "non-system-no-manifest": + default: + verifyPackage(false, false); + break; + } + } + + private static void buildJarsAndInitiateSystemPackageTest() throws Exception { + Manifest m = new Manifest(); + // not setting MANIFEST_VERSION prevents META-INF/MANIFEST.MF from getting written + m.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0"); + m.getMainAttributes().put(Attributes.Name.SPECIFICATION_TITLE, manifestTitle); + JarBuilder manifestJar = new JarBuilder(tmpFolder, "manifest.jar", m); + manifestJar.addClassFile("package2/Class2.class", testClassesDir + "/package2/Class2.class"); + manifestJar.addClassFile("GetSystemPackage.class", testClassesDir + "/GetSystemPackage.class"); + manifestJar.addClassFile("GetSystemPackageClassLoader.class", testClassesDir + "/GetSystemPackageClassLoader.class"); + manifestJar.build(); + + JarBuilder noManifestJar = new JarBuilder(tmpFolder, "no-manifest.jar", null); + noManifestJar.addClassFile("package2/Class2.class", testClassesDir + "/package2/Class2.class"); + noManifestJar.addClassFile("GetSystemPackage.class", testClassesDir + "/GetSystemPackage.class"); + noManifestJar.addClassFile("GetSystemPackageClassLoader.class", testClassesDir + "/GetSystemPackageClassLoader.class"); + noManifestJar.build(); + + try { + runSubProcess("Package of a system package with manifest setup could not be resolved.", + "-Xbootclasspath/p:" + testClassesDir + "/manifest.jar", "GetSystemPackage", "system-manifest"); + + runSubProcess("System package from bootclasspath directory could not be resolved.", + "-Xbootclasspath/p:" + testClassesDir, "GetSystemPackage", "system-no-manifest"); + + runSubProcess("System package from JAR without manifest could not be resolved.", + "-Xbootclasspath/p:" + testClassesDir + "/no-manifest.jar", "GetSystemPackage", "system-no-manifest"); + + runSubProcess("Package not on bootclasspath could be resolved via system classpath.", + "-cp", testClassesDir + "/manifest.jar", "GetSystemPackage", "non-system-manifest"); + + runSubProcess("Package with no manifest not on bootclasspath could be resolved via system classpath.", + "-cp", testClassesDir + "/no-manifest.jar", "GetSystemPackage", "non-system-no-manifest"); + + } catch (InterruptedException | IOException x) { + throw new RuntimeException(x); + } + } + + private static void runSubProcess(String messageOnError, String ... args) throws InterruptedException, IOException { + ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(args); + int res = pb.directory(tmpFolder).inheritIO().start().waitFor(); + if (res != 0) { + throw new RuntimeException(messageOnError); + } + } + + private static void verifyPackage(boolean hasManifest, boolean isSystemPackage) throws Exception { + Class c = Class.forName("package2.Class2"); + + // force the use of a classloader with no parent, bypassing classpath + Package pkg = c.getPackage(); + + GetSystemPackageClassLoader classLoader = new GetSystemPackageClassLoader(); + if (isSystemPackage) { + pkg = classLoader.getSystemPackage("package2"); + if (pkg == null) { + throw new Exception("System package could not be found via Package.getSystemPackage"); + } + } else if (classLoader.getSystemPackage("package2") != null) { + throw new Exception("Non-system package could be found via Package.getSystemPackage"); + } + + String specificationTitle = pkg.getSpecificationTitle(); + if (!"package2".equals(pkg.getName())) { + throw new Exception("Invalid package for Class2"); + } + + if (hasManifest && (specificationTitle == null + || !manifestTitle.equals(specificationTitle))) { + throw new Exception("Invalid manifest for package " + pkg.getName() + + ": was " + specificationTitle + " expected: Special JAR"); + } + if (!hasManifest && specificationTitle != null) { + throw new Exception("Invalid manifest for package " + pkg.getName() + + ": was " + specificationTitle + " expected: null"); + } + } +} + +/* + * This classloader bypasses the system classloader to give as direct access + * to Package.getSystemPackage() as possible + */ +class GetSystemPackageClassLoader extends ClassLoader { + + public GetSystemPackageClassLoader() { + super(null); + } + + public Package getSystemPackage(String name) { + return super.getPackage(name); + } + +} + +/* + * Helper class for building jar files + */ +class JarBuilder { + private JarOutputStream os; + + public JarBuilder(File tmpFolder, String jarName, Manifest manifest) + throws FileNotFoundException, IOException + { + File jarFile = new File(tmpFolder, jarName); + if (manifest != null) { + this.os = new JarOutputStream(new FileOutputStream(jarFile), manifest); + } else { + this.os = new JarOutputStream(new FileOutputStream(jarFile)); + } + } + + public void addClassFile(String pathFromRoot, String file) + throws IOException + { + byte[] buf = new byte[1024]; + try (FileInputStream in = new FileInputStream(file)) { + JarEntry entry = new JarEntry(pathFromRoot); + os.putNextEntry(entry); + int len; + while ((len = in.read(buf)) > 0) { + os.write(buf, 0, len); + } + os.closeEntry(); + } + } + + public void build() throws IOException { + os.close(); + } +}