8060130: Simplify the synchronization of defining and getting java.lang.Package
Reviewed-by: mchung, plevart, shade
--- a/jdk/src/java.base/share/classes/java/lang/ClassLoader.java Wed Nov 05 11:11:27 2014 +0000
+++ b/jdk/src/java.base/share/classes/java/lang/ClassLoader.java Thu Nov 06 00:06:26 2014 +0100
@@ -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<String, Package> packages = new HashMap<>();
+ private final ConcurrentHashMap<String, Package> 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<String, Package> map;
- synchronized (packages) {
- map = new HashMap<>(packages);
- }
Package[] pkgs;
if (parent != null) {
pkgs = parent.getPackages();
} else {
pkgs = Package.getSystemPackages();
}
+
+ Map<String, Package> 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()]);
--- a/jdk/src/java.base/share/classes/java/lang/Package.java Wed Nov 05 11:11:27 2014 +0000
+++ b/jdk/src/java.base/share/classes/java/lang/Package.java Thu Nov 06 00:06:26 2014 +0100
@@ -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<Package>() {
- 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<String, Package> pkgs = new HashMap<>(31);
+ private static final ConcurrentHashMap<String, Package> pkgs
+ = new ConcurrentHashMap<>();
+
+ // Maps each directory or zip file name to its corresponding manifest, if
+ // it exists
+ private static final ConcurrentHashMap<String, CachedManifest> manifests
+ = new ConcurrentHashMap<>();
+
+ 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<URL>() {
+ public URL run() {
+ final File file = new File(fileName);
+ if (file.isFile()) {
+ try {
+ return ParseUtil.fileToEncodedURL(file);
+ } catch (MalformedURLException e) {
+ }
+ }
+ return null;
+ }
+ });
+ }
- // Maps each directory or zip file name to its corresponding url
- private static Map<String, URL> urls = new HashMap<>(10);
+ public URL getURL() {
+ return url;
+ }
- // Maps each code source url for a jar file to its manifest
- private static Map<String, Manifest> mans = new HashMap<>(10);
+ 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<Manifest>() {
+ 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();
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/lang/ClassLoader/GetSystemPackage.java Thu Nov 06 00:06:26 2014 +0100
@@ -0,0 +1,249 @@
+/*
+ * Copyright (c) 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.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+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);
+
+ buildJar("manifest.jar", m);
+ buildJar("no-manifest.jar", null);
+
+ runSubProcess("System package with manifest improperly resolved.",
+ "-Xbootclasspath/p:" + testClassesDir + "/manifest.jar",
+ "GetSystemPackage", "system-manifest");
+
+ runSubProcess("System package from directory improperly resolved.",
+ "-Xbootclasspath/p:" + testClassesDir, "GetSystemPackage",
+ "system-no-manifest");
+
+ runSubProcess("System package with no manifest improperly resolved",
+ "-Xbootclasspath/p:" + testClassesDir + "/no-manifest.jar",
+ "GetSystemPackage", "system-no-manifest");
+
+ runSubProcess("Classpath package with manifest improperly resolved",
+ "-cp", testClassesDir + "/manifest.jar", "GetSystemPackage",
+ "non-system-manifest");
+
+ runSubProcess("Classpath package with no manifest improperly resolved",
+ "-cp", testClassesDir + "/no-manifest.jar", "GetSystemPackage",
+ "non-system-no-manifest");
+
+ }
+
+ private static void buildJar(String name, Manifest man) throws Exception {
+ JarBuilder jar = new JarBuilder(tmpFolder, name, man);
+ jar.addClassFile("package2/Class2.class",
+ testClassesDir + "/package2/Class2.class");
+ jar.addClassFile("GetSystemPackage.class",
+ testClassesDir + "/GetSystemPackage.class");
+ jar.addClassFile("GetSystemPackageClassLoader.class",
+ testClassesDir + "/GetSystemPackageClassLoader.class");
+ jar.build();
+ }
+
+ private static void runSubProcess(String messageOnError, String ... args)
+ throws Exception
+ {
+ 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");
+ Package pkg = c.getPackage();
+ if (pkg == null || pkg != Package.getPackage("package2") ||
+ !"package2".equals(pkg.getName())) {
+ fail("package2 not found via Package.getPackage()");
+ }
+
+ String specificationTitle = pkg.getSpecificationTitle();
+ if (!"package2".equals(pkg.getName())) {
+ fail("Invalid package for Class2");
+ }
+
+ if (hasManifest && (specificationTitle == null
+ || !manifestTitle.equals(specificationTitle))) {
+ fail("Invalid manifest for package " + pkg.getName());
+ }
+ if (!hasManifest && specificationTitle != null) {
+ fail("Invalid manifest for package " + pkg.getName() + ": was " +
+ specificationTitle + " expected: null");
+ }
+
+ // force the use of a classloader with no parent, then retrieve the
+ // package in a way that bypasses the classloader pkg maps
+ GetSystemPackageClassLoader classLoader =
+ new GetSystemPackageClassLoader();
+ Package systemPkg = classLoader.getSystemPackage("package2");
+
+ if (findPackage("java.lang") == null) {
+ fail("java.lang not found via Package.getPackages()");
+ }
+ Package foundPackage = findPackage("package2");
+ if (isSystemPackage) {
+ if (systemPkg == null) {
+ fail("System package could not be found via getSystemPackage");
+ }
+ if (foundPackage != systemPkg || systemPkg != pkg) {
+ fail("Inconsistent package found via Package.getPackages()");
+ }
+ } else {
+ if (systemPkg != null) {
+ fail("Non-system package could be found via getSystemPackage");
+ }
+ if (foundPackage == null) {
+ fail("Non-system package not found via Package.getPackages()");
+ }
+ }
+ }
+
+ private static Package findPackage(String name) {
+ Package[] packages = Package.getPackages();
+ for (Package p : packages) {
+ System.out.println(p);
+ if (p.getName().equals(name)) {
+ return p;
+ }
+ }
+ return null;
+ }
+
+ private static void fail(String message) {
+ throw new RuntimeException(message);
+ }
+}
+
+/*
+ * 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();
+ }
+}