From de7fd1c3061cfbfdbd5d7cc2b1ba0ee8d432ee0a Mon Sep 17 00:00:00 2001 From: Mandy Chung Date: Tue, 30 May 2023 21:01:12 +0000 Subject: [PATCH] 8307944: ClassFileDumper should only load java.nio.file.Path if enabled Reviewed-by: rriggs --- .../invoke/InnerClassLambdaMetafactory.java | 3 +- .../java/lang/invoke/MethodHandleStatics.java | 3 +- .../java/lang/invoke/MethodHandles.java | 3 +- .../jdk/internal/util/ClassFileDumper.java | 130 +++++++----------- .../invoke/DumpMethodHandleInternals.java | 56 ++++++++ .../lambda/LogGeneratedClassesTest.java | 6 +- 6 files changed, 109 insertions(+), 92 deletions(-) create mode 100644 test/jdk/java/lang/invoke/DumpMethodHandleInternals.java diff --git a/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java b/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java index 46aff94276e..f651d41d0b5 100644 --- a/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java +++ b/src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java @@ -35,7 +35,6 @@ import sun.security.action.GetBooleanAction; import java.io.Serializable; import java.lang.constant.ConstantDescs; import java.lang.reflect.Modifier; -import java.nio.file.Path; import java.util.LinkedHashSet; import java.util.Set; @@ -93,7 +92,7 @@ import static jdk.internal.org.objectweb.asm.Opcodes.*; // -Djdk.invoke.LambdaMetafactory.dumpProxyClassFiles // or -Djdk.invoke.LambdaMetafactory.dumpProxyClassFiles=true final String dumpProxyClassesKey = "jdk.invoke.LambdaMetafactory.dumpProxyClassFiles"; - lambdaProxyClassFileDumper = ClassFileDumper.getInstance(dumpProxyClassesKey, Path.of("DUMP_LAMBDA_PROXY_CLASS_FILES")); + lambdaProxyClassFileDumper = ClassFileDumper.getInstance(dumpProxyClassesKey, "DUMP_LAMBDA_PROXY_CLASS_FILES"); final String disableEagerInitializationKey = "jdk.internal.lambda.disableEagerInitialization"; disableEagerInitialization = GetBooleanAction.privilegedGetProperty(disableEagerInitializationKey); diff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandleStatics.java b/src/java.base/share/classes/java/lang/invoke/MethodHandleStatics.java index ba190525e00..5bbe030c548 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodHandleStatics.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandleStatics.java @@ -31,7 +31,6 @@ import jdk.internal.util.ClassFileDumper; import sun.security.action.GetPropertyAction; import java.lang.reflect.ClassFileFormatVersion; -import java.nio.file.Path; import java.util.Properties; import static java.lang.invoke.LambdaForm.basicTypeSignature; @@ -98,7 +97,7 @@ class MethodHandleStatics { props.getProperty("java.lang.invoke.MethodHandleImpl.MAX_ARITY", "255")); DUMP_CLASS_FILES = ClassFileDumper.getInstance("jdk.invoke.MethodHandle.dumpMethodHandleInternals", - Path.of("DUMP_METHOD_HANDLE_INTERNALS")); + "DUMP_METHOD_HANDLE_INTERNALS"); if (CUSTOMIZE_THRESHOLD < -1 || CUSTOMIZE_THRESHOLD > 127) { throw newInternalError("CUSTOMIZE_THRESHOLD should be in [-1...127] range"); diff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java index e3dcaac8922..8123ecff79b 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java @@ -56,7 +56,6 @@ import java.lang.reflect.Member; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.nio.ByteOrder; -import java.nio.file.Path; import java.security.ProtectionDomain; import java.util.ArrayList; import java.util.Arrays; @@ -2252,7 +2251,7 @@ public class MethodHandles { } private static final ClassFileDumper DEFAULT_DUMPER = ClassFileDumper.getInstance( - "jdk.invoke.MethodHandle.dumpClassFiles", Path.of("DUMP_CLASS_FILES")); + "jdk.invoke.MethodHandle.dumpClassFiles", "DUMP_CLASS_FILES"); static class ClassFile { final String name; // internal name diff --git a/src/java.base/share/classes/jdk/internal/util/ClassFileDumper.java b/src/java.base/share/classes/jdk/internal/util/ClassFileDumper.java index 1bec5ca34a1..afb3d1374ab 100644 --- a/src/java.base/share/classes/jdk/internal/util/ClassFileDumper.java +++ b/src/java.base/share/classes/jdk/internal/util/ClassFileDumper.java @@ -27,16 +27,13 @@ package jdk.internal.util; import jdk.internal.misc.VM; import sun.security.action.GetPropertyAction; -import java.io.FilePermission; import java.io.IOException; -import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; import java.security.AccessController; import java.security.PrivilegedAction; import java.util.HexFormat; import java.util.Objects; -import java.util.PropertyPermission; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; @@ -52,36 +49,6 @@ public final class ClassFileDumper { private static final ConcurrentHashMap DUMPER_MAP = new ConcurrentHashMap<>(); - /** - * Returns a ClassFileDumper instance for the given key. To enable - * dumping of the generated classes, set the system property via - * -D=. - * - * The system property is read only once when it is the first time - * the dumper instance for the given key is created. - * - * If not enabled, this method returns ClassFileDumper with null - * dump path. - */ - public static ClassFileDumper getInstance(String key) { - Objects.requireNonNull(key); - - var dumper = DUMPER_MAP.get(key); - if (dumper == null) { - String path = GetPropertyAction.privilegedGetProperty(key); - Path dir; - if (path == null || path.trim().isEmpty()) { - dir = null; - } else { - dir = validateDumpDir(Path.of(path.trim())); - } - var newDumper = new ClassFileDumper(key, dir); - var v = DUMPER_MAP.putIfAbsent(key, newDumper); - dumper = v != null ? v : newDumper; - } - return dumper; - } - /** * Returns a ClassFileDumper instance for the given key with a given * dump path. To enable dumping of the generated classes @@ -89,53 +56,49 @@ public final class ClassFileDumper { * * The system property is read only once when it is the first time * the dumper instance for the given key is created. - * - * If not enabled, this method returns ClassFileDumper with null - * dump path. */ - public static ClassFileDumper getInstance(String key, Path path) { + public static ClassFileDumper getInstance(String key, String path) { Objects.requireNonNull(key); Objects.requireNonNull(path); var dumper = DUMPER_MAP.get(key); if (dumper == null) { - String value = GetPropertyAction.privilegedGetProperty(key); - boolean enabled = value != null && value.isEmpty() - ? true : Boolean.parseBoolean(value); - Path dir = enabled ? validateDumpDir(path) : null; - var newDumper = new ClassFileDumper(key, dir); + var newDumper = new ClassFileDumper(key, path); var v = DUMPER_MAP.putIfAbsent(key, newDumper); dumper = v != null ? v : newDumper; } - if (dumper.isEnabled() && !path.equals(dumper.dumpPath())) { + if (dumper.isEnabled() && !path.equals(dumper.dumpDir)) { throw new IllegalArgumentException("mismatched dump path for " + key); } return dumper; } private final String key; - private final Path dumpDir; + private final String dumpDir; + private final boolean enabled; private final AtomicInteger counter = new AtomicInteger(); - private ClassFileDumper(String key, Path path) { + private ClassFileDumper(String key, String path) { + String value = GetPropertyAction.privilegedGetProperty(key); this.key = key; + boolean enabled = value != null && value.isEmpty() ? true : Boolean.parseBoolean(value); + if (enabled) { + validateDumpDir(path); + } this.dumpDir = path; + this.enabled = enabled; } public String key() { return key; } public boolean isEnabled() { - return dumpDir != null; + return enabled; } - public Path dumpPath() { - return dumpDir; - } - - public Path pathname(String internalName) { - return dumpDir.resolve(encodeForFilename(internalName) + ".class"); + private Path pathname(String name) { + return Path.of(dumpDir, encodeForFilename(name) + ".class"); } /** @@ -169,46 +132,47 @@ public final class ClassFileDumper { @SuppressWarnings("removal") private void write(Path path, byte[] bytes) { AccessController.doPrivileged(new PrivilegedAction<>() { - @Override public Void run() { - try { - Path dir = path.getParent(); - Files.createDirectories(dir); - Files.write(path, bytes); - } catch (Exception ex) { - if (VM.isModuleSystemInited()) { - // log only when lambda is ready to use - System.getLogger(ClassFileDumper.class.getName()) - .log(System.Logger.Level.WARNING, "Exception writing to " + - path.toString() + " " + ex.getMessage()); - } - // simply don't care if this operation failed + @Override public Void run() { + try { + Files.createDirectories(path.getParent()); + Files.write(path, bytes); + } catch (Exception ex) { + if (VM.isModuleSystemInited()) { + // log only when lambda is ready to use + System.getLogger(ClassFileDumper.class.getName()) + .log(System.Logger.Level.WARNING, "Exception writing to " + + path + " " + ex.getMessage()); } - return null; - }}, - null, - new FilePermission("<>", "read, write"), - // createDirectories may need it - new PropertyPermission("user.dir", "read")); + // simply don't care if this operation failed + } + return null; + }}); } + /* + * Validate if the given dir is a writeable directory if exists. + */ @SuppressWarnings("removal") - private static Path validateDumpDir(Path path) { - return AccessController.doPrivileged(new PrivilegedAction<>() { - @Override - public Path run() { + private static Path validateDumpDir(String dir) { + return AccessController.doPrivileged(new PrivilegedAction<>() { + @Override + public Path run() { + Path path = Path.of(dir); + if (Files.notExists(path)) { try { Files.createDirectories(path); } catch (IOException ex) { - throw new UncheckedIOException("Fail to create " + path, ex); + throw new IllegalArgumentException("Fail to create " + path, ex); } - if (!Files.isDirectory(path)) { - throw new IllegalArgumentException("Path " + path + " is not a directory"); - } else if (!Files.isWritable(path)) { - throw new IllegalArgumentException("Directory " + path + " is not writable"); - } - return path; } - }); + if (!Files.isDirectory(path)) { + throw new IllegalArgumentException("Path " + path + " is not a directory"); + } else if (!Files.isWritable(path)) { + throw new IllegalArgumentException("Directory " + path + " is not writable"); + } + return path; + } + }); } private static final HexFormat HEX = HexFormat.of().withUpperCase(); diff --git a/test/jdk/java/lang/invoke/DumpMethodHandleInternals.java b/test/jdk/java/lang/invoke/DumpMethodHandleInternals.java new file mode 100644 index 00000000000..7de9cbf4a96 --- /dev/null +++ b/test/jdk/java/lang/invoke/DumpMethodHandleInternals.java @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2023, 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 8307944 + * @library /test/lib + * @build DumpMethodHandleInternals + * @run main DumpMethodHandleInternals + + * @summary Test startup with -Djdk.invoke.MethodHandle.dumpMethodHandleInternals + * to work properly + */ + +import java.nio.file.Files; +import java.nio.file.Path; + +import jdk.test.lib.process.ProcessTools; + +public class DumpMethodHandleInternals { + + private static final Path DUMP_DIR = Path.of("DUMP_METHOD_HANDLE_INTERNALS"); + + public static void main(String[] args) throws Exception { + if (ProcessTools.executeTestJava("-Djdk.invoke.MethodHandle.dumpMethodHandleInternals", + "-version") + .outputTo(System.out) + .errorTo(System.out) + .getExitValue() != 0) + throw new RuntimeException("Test failed - see output"); + + if (Files.notExists(DUMP_DIR)) + throw new RuntimeException(DUMP_DIR + " not created"); + } + +} diff --git a/test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java b/test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java index e227770e8f1..26ed7b0058e 100644 --- a/test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java +++ b/test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java @@ -175,10 +175,10 @@ public class LogGeneratedClassesTest extends LUtils { "-Djdk.invoke.LambdaMetafactory.dumpProxyClassFiles", "com.example.TestLambda"); assertEquals(tr.testOutput.stream() - .filter(s -> s.contains("java.nio.file.FileAlreadyExistsException")) + .filter(s -> s.contains("DUMP_LAMBDA_PROXY_CLASS_FILES is not a directory")) .count(), 1, "only show error once"); - assertTrue(tr.exitValue !=0); + assertTrue(tr.exitValue != 0); } private static boolean isWriteableDirectory(Path p) { @@ -237,7 +237,7 @@ public class LogGeneratedClassesTest extends LUtils { "-Djdk.invoke.LambdaMetafactory.dumpProxyClassFiles", "com.example.TestLambda"); assertEquals(tr.testOutput.stream() - .filter(s -> s.contains("is not writable")) + .filter(s -> s.contains("DUMP_LAMBDA_PROXY_CLASS_FILES is not writable")) .count(), 1, "only show error once"); assertTrue(tr.exitValue != 0);