8265465: jcmd VM.cds should keep already dumped archive when exception happens
Reviewed-by: iklam, ccheung
diff --git a/src/java.base/share/classes/jdk/internal/misc/CDS.java b/src/java.base/share/classes/jdk/internal/misc/CDS.java
index 4284303..4bed16a 100644
--- a/src/java.base/share/classes/jdk/internal/misc/CDS.java
+++ b/src/java.base/share/classes/jdk/internal/misc/CDS.java
@@ -257,21 +257,24 @@
*/
private static void dumpSharedArchive(boolean isStatic, String fileName) throws Exception {
String currentPid = String.valueOf(ProcessHandle.current().pid());
- String archiveFile = fileName != null ? fileName :
+ String archiveFileName = fileName != null ? fileName :
"java_pid" + currentPid + (isStatic ? "_static.jsa" : "_dynamic.jsa");
- // delete if archive file aready exists
- File fileArchive = new File(archiveFile);
- if (fileArchive.exists()) {
- fileArchive.delete();
+ String tempArchiveFileName = archiveFileName + ".temp";
+ File tempArchiveFile = new File(tempArchiveFileName);
+ // The operation below may cause exception if the file or its dir is protected.
+ if (!tempArchiveFile.exists()) {
+ tempArchiveFile.createNewFile();
}
+ tempArchiveFile.delete();
+
if (isStatic) {
- String listFile = archiveFile + ".classlist";
- File fileList = new File(listFile);
- if (fileList.exists()) {
- fileList.delete();
+ String listFileName = archiveFileName + ".classlist";
+ File listFile = new File(listFileName);
+ if (listFile.exists()) {
+ listFile.delete();
}
- dumpClassList(listFile);
+ dumpClassList(listFileName);
String jdkHome = System.getProperty("java.home");
String classPath = System.getProperty("java.class.path");
List<String> cmds = new ArrayList<String>();
@@ -280,8 +283,8 @@
cmds.add(classPath);
cmds.add("-Xlog:cds");
cmds.add("-Xshare:dump");
- cmds.add("-XX:SharedClassListFile=" + listFile);
- cmds.add("-XX:SharedArchiveFile=" + archiveFile);
+ cmds.add("-XX:SharedClassListFile=" + listFileName);
+ cmds.add("-XX:SharedArchiveFile=" + tempArchiveFileName);
// All runtime args.
String[] vmArgs = VM.getRuntimeArguments();
@@ -301,26 +304,33 @@
proc.waitFor();
// done, delete classlist file.
- if (fileList.exists()) {
- fileList.delete();
- }
+ listFile.delete();
+
// Check if archive has been successfully dumped. We won't reach here if exception happens.
// Throw exception if file is not created.
- if (!fileArchive.exists()) {
- throw new RuntimeException("Archive file " + archiveFile +
+ if (!tempArchiveFile.exists()) {
+ throw new RuntimeException("Archive file " + tempArchiveFileName +
" is not created, please check stdout file " +
stdOutFile + " or stderr file " +
stdErrFile + " for more detail");
}
} else {
- dumpDynamicArchive(archiveFile);
- if (!fileArchive.exists()) {
- throw new RuntimeException("Archive file " + archiveFile +
+ dumpDynamicArchive(tempArchiveFileName);
+ if (!tempArchiveFile.exists()) {
+ throw new RuntimeException("Archive file " + tempArchiveFileName +
" is not created, please check process " +
currentPid + " output for more detail");
}
}
+ // Override the existing archive file
+ File archiveFile = new File(archiveFileName);
+ if (archiveFile.exists()) {
+ archiveFile.delete();
+ }
+ if (!tempArchiveFile.renameTo(archiveFile)) {
+ throw new RuntimeException("Cannot rename temp file " + tempArchiveFileName + " to archive file" + archiveFileName);
+ }
// Everyting goes well, print out the file name.
- System.out.println((isStatic ? "Static" : " Dynamic") + " dump to file " + archiveFile);
+ System.out.println((isStatic ? "Static" : " Dynamic") + " dump to file " + archiveFileName);
}
}
diff --git a/test/hotspot/jtreg/TEST.groups b/test/hotspot/jtreg/TEST.groups
index 43dc1e8..2e469ff 100644
--- a/test/hotspot/jtreg/TEST.groups
+++ b/test/hotspot/jtreg/TEST.groups
@@ -315,6 +315,7 @@
-runtime/cds/appcds/javaldr/LockDuringDump.java \
-runtime/cds/appcds/jcmd/JCmdTestStaticDump.java \
-runtime/cds/appcds/jcmd/JCmdTestDynamicDump.java \
+ -runtime/cds/appcds/jcmd/JCmdTestFileSafety.java \
-runtime/cds/appcds/methodHandles \
-runtime/cds/appcds/sharedStrings \
-runtime/cds/appcds/ArchiveRelocationTest.java \
diff --git a/test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestDumpBase.java b/test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestDumpBase.java
index 3b4eceb..563e069 100644
--- a/test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestDumpBase.java
+++ b/test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestDumpBase.java
@@ -28,6 +28,7 @@
import java.util.List;
import jdk.test.lib.apps.LingeredApp;
+import jdk.test.lib.dcmd.CommandExecutorException;
import jdk.test.lib.dcmd.PidJcmdExecutor;
import jdk.test.lib.process.OutputAnalyzer;
import jtreg.SkippedException;
@@ -74,6 +75,18 @@
public static final boolean EXPECT_PASS = true;
public static final boolean EXPECT_FAIL = !EXPECT_PASS;
+ // If delete the created archive after test.
+ private static boolean keepArchive = false;
+ public static void setKeepArchive(boolean v) { keepArchive = v; }
+ public static void checkFileExistence(String fileName, boolean checkExist) throws Exception {
+ File file = new File(fileName);
+ boolean exist = file.exists();
+ boolean resultIsTrue = checkExist ? exist : !exist;
+ if (!resultIsTrue) {
+ throw new RuntimeException("File " + fileName + " should " + (checkExist ? "exist" : "not exist"));
+ }
+ }
+
protected static void buildJars() throws Exception {
testJar = JarBuilder.build("test", TEST_CLASSES);
bootJar = JarBuilder.build("boot", BOOT_CLASSES);
@@ -147,7 +160,7 @@
args.add("-Xlog:class+load");
LingeredApp app = createLingeredApp(args.toArray(new String[0]));
- app.setLogFileName("JCmdTestDynamicDump.log." + (logFileCount++));
+ app.setLogFileName("JCmdTest-Run-" + (isStatic ? "Static.log" : "Dynamic.log.") + (logFileCount++));
app.stopApp();
String output = app.getOutput().getStdout();
if (messages != null) {
@@ -159,19 +172,17 @@
}
}
- protected static void test(String archiveFile, long pid,
+ protected static void test(String fileName, long pid,
boolean useBoot, boolean expectOK, String... messages) throws Exception {
System.out.println("Expected: " + (expectOK ? "SUCCESS" : "FAIL"));
- String fileName = archiveFile != null ? archiveFile :
+ String archiveFileName = fileName != null ? fileName :
("java_pid" + pid + (isStatic ? "_static.jsa" : "_dynamic.jsa"));
- File file = new File(fileName);
- if (file.exists()) {
- file.delete();
- }
+
+ File archiveFile = new File(archiveFileName);
String jcmd = "VM.cds " + (isStatic ? "static_dump" : "dynamic_dump");
- if (archiveFile != null) {
- jcmd += " " + archiveFile;
+ if (archiveFileName != null) {
+ jcmd += " " + archiveFileName;
}
PidJcmdExecutor cmdExecutor = new PidJcmdExecutor(String.valueOf(pid));
@@ -179,15 +190,14 @@
if (expectOK) {
output.shouldHaveExitValue(0);
- if (!file.exists()) {
- throw new RuntimeException("Could not create shared archive: " + fileName);
- } else {
- runWithArchiveFile(fileName, useBoot, messages);
- file.delete();
+ checkFileExistence(archiveFileName, true);
+ runWithArchiveFile(archiveFileName, useBoot, messages);
+ if (!keepArchive) {
+ archiveFile.delete();
}
} else {
- if (file.exists()) {
- throw new RuntimeException("Should not create shared archive " + fileName);
+ if (!keepArchive) {
+ checkFileExistence(archiveFileName, false);
}
}
}
diff --git a/test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestFileSafety.java b/test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestFileSafety.java
new file mode 100644
index 0000000..19669bc
--- /dev/null
+++ b/test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestFileSafety.java
@@ -0,0 +1,107 @@
+/*
+ * Copyright (c) 2021, 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 8265465
+ * @summary Test jcmd to dump static shared archive.
+ * @requires vm.cds
+ * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds
+ * @modules jdk.jcmd/sun.tools.common:+open
+ * @compile ../test-classes/Hello.java JCmdTestDumpBase.java
+ * @build sun.hotspot.WhiteBox
+ * @build JCmdTestLingeredApp JCmdTestFileSafety
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller sun.hotspot.WhiteBox
+ * @run main/othervm/timeout=480 -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI JCmdTestFileSafety
+ */
+
+import java.io.File;
+import java.io.IOException;
+import jdk.test.lib.cds.CDSTestUtils;
+import jdk.test.lib.apps.LingeredApp;
+import jdk.test.lib.Platform;
+
+public class JCmdTestFileSafety extends JCmdTestDumpBase {
+
+ static void test() throws Exception {
+ buildJars();
+
+ LingeredApp app = null;
+ long pid;
+
+ int test_count = 1;
+ String subDir = "subdir";
+ File outputDirFile = new File(subDir);
+ if (!outputDirFile.exists()) {
+ outputDirFile.mkdir();
+ }
+ outputDirFile.setWritable(true);
+ String localFileName = subDir + File.separator + "MyStaticDump.jsa";
+ setIsStatic(true/*static*/);
+ // Set target dir not writable, do static dump
+ print2ln(test_count++ + " Set target dir not writable, do static dump");
+ setKeepArchive(true);
+ app = createLingeredApp("-cp", allJars);
+ pid = app.getPid();
+ test(localFileName, pid, noBoot, EXPECT_PASS);
+ outputDirFile.setWritable(false);
+ test(localFileName, pid, noBoot, EXPECT_FAIL);
+ app.stopApp();
+ outputDirFile.setWritable(true);
+ checkFileExistence(localFileName, true/*exist*/);
+
+ setIsStatic(false/*dynamic*/);
+ // Set target dir not writable, do dynamic dump
+ print2ln(test_count++ + " Set target dir not writable, do dynamic dump");
+ setKeepArchive(true);
+ outputDirFile.setWritable(true);
+ app = createLingeredApp("-cp", allJars, "-XX:+RecordDynamicDumpInfo");
+ pid = app.getPid();
+ localFileName = subDir + File.separator + "MyDynamicDump.jsa";
+ test(localFileName, pid, noBoot, EXPECT_PASS);
+ app.stopApp();
+ // cannot dynamically dump twice, restart
+ app = createLingeredApp("-cp", allJars, "-XX:+RecordDynamicDumpInfo");
+ pid = app.getPid();
+ outputDirFile.setWritable(false);
+ test(localFileName, pid, noBoot, EXPECT_FAIL);
+ outputDirFile.setWritable(true);
+ app.stopApp();
+ // MyDynamicDump.jsa should exist
+ checkFileExistence(localFileName, true);
+ File rmFile = new File(localFileName);
+ rmFile.delete();
+ outputDirFile.delete();
+ }
+
+ public static void main(String... args) throws Exception {
+ if (Platform.isWindows()) {
+ // ON windows, File operation resulted difference from other OS.
+ // Set dir to not accessible for write, we still can run the test
+ // to create archive successfully which is not expected.
+ throw new jtreg.SkippedException("Test skipped on Windows");
+ }
+ runTest(JCmdTestFileSafety::test);
+ }
+}
