8353727: HeapDumpPath doesn't expand %p

Reviewed-by: stuefe, lmesnik
This commit is contained in:
Kevin Walls 2025-04-09 14:49:04 +00:00
parent c3e043956e
commit 7a7b9ed7fe
2 changed files with 34 additions and 61 deletions

View File

@ -43,6 +43,7 @@
#include "oops/objArrayOop.inline.hpp"
#include "oops/oop.inline.hpp"
#include "oops/typeArrayOop.inline.hpp"
#include "runtime/arguments.hpp"
#include "runtime/continuationWrapper.inline.hpp"
#include "runtime/frame.inline.hpp"
#include "runtime/handles.inline.hpp"
@ -2747,71 +2748,45 @@ void HeapDumper::dump_heap() {
void HeapDumper::dump_heap(bool oome) {
static char base_path[JVM_MAXPATHLEN] = {'\0'};
static uint dump_file_seq = 0;
char* my_path;
char my_path[JVM_MAXPATHLEN];
const int max_digit_chars = 20;
const char* dump_file_name = "java_pid";
const char* dump_file_ext = HeapDumpGzipLevel > 0 ? ".hprof.gz" : ".hprof";
const char* dump_file_name = HeapDumpGzipLevel > 0 ? "java_pid%p.hprof.gz" : "java_pid%p.hprof";
// The dump file defaults to java_pid<pid>.hprof in the current working
// directory. HeapDumpPath=<file> can be used to specify an alternative
// dump file name or a directory where dump file is created.
if (dump_file_seq == 0) { // first time in, we initialize base_path
// Calculate potentially longest base path and check if we have enough
// allocated statically.
const size_t total_length =
(HeapDumpPath == nullptr ? 0 : strlen(HeapDumpPath)) +
strlen(os::file_separator()) + max_digit_chars +
strlen(dump_file_name) + strlen(dump_file_ext) + 1;
if (total_length > sizeof(base_path)) {
// Set base path (name or directory, default or custom, without seq no), doing %p substitution.
const char *path_src = (HeapDumpPath != nullptr && HeapDumpPath[0] != '\0') ? HeapDumpPath : dump_file_name;
if (!Arguments::copy_expand_pid(path_src, strlen(path_src), base_path, JVM_MAXPATHLEN - max_digit_chars)) {
warning("Cannot create heap dump file. HeapDumpPath is too long.");
return;
}
bool use_default_filename = true;
if (HeapDumpPath == nullptr || HeapDumpPath[0] == '\0') {
// HeapDumpPath=<file> not specified
} else {
strcpy(base_path, HeapDumpPath);
// check if the path is a directory (must exist)
DIR* dir = os::opendir(base_path);
if (dir == nullptr) {
use_default_filename = false;
} else {
// HeapDumpPath specified a directory. We append a file separator
// (if needed).
os::closedir(dir);
size_t fs_len = strlen(os::file_separator());
if (strlen(base_path) >= fs_len) {
char* end = base_path;
end += (strlen(base_path) - fs_len);
if (strcmp(end, os::file_separator()) != 0) {
strcat(base_path, os::file_separator());
}
// Check if the path is an existing directory
DIR* dir = os::opendir(base_path);
if (dir != nullptr) {
os::closedir(dir);
// Path is a directory. Append a file separator (if needed).
size_t fs_len = strlen(os::file_separator());
if (strlen(base_path) >= fs_len) {
char* end = base_path;
end += (strlen(base_path) - fs_len);
if (strcmp(end, os::file_separator()) != 0) {
strcat(base_path, os::file_separator());
}
}
// Then add the default name, with %p substitution. Use my_path temporarily.
if (!Arguments::copy_expand_pid(dump_file_name, strlen(dump_file_name), my_path, JVM_MAXPATHLEN - max_digit_chars)) {
warning("Cannot create heap dump file. HeapDumpPath is too long.");
return;
}
const size_t dlen = strlen(base_path);
jio_snprintf(&base_path[dlen], sizeof(base_path) - dlen, "%s", my_path);
}
// If HeapDumpPath wasn't a file name then we append the default name
if (use_default_filename) {
const size_t dlen = strlen(base_path); // if heap dump dir specified
jio_snprintf(&base_path[dlen], sizeof(base_path)-dlen, "%s%d%s",
dump_file_name, os::current_process_id(), dump_file_ext);
}
const size_t len = strlen(base_path) + 1;
my_path = (char*)os::malloc(len, mtInternal);
if (my_path == nullptr) {
warning("Cannot create heap dump file. Out of system memory.");
return;
}
strncpy(my_path, base_path, len);
strncpy(my_path, base_path, JVM_MAXPATHLEN);
} else {
// Append a sequence number id for dumps following the first
const size_t len = strlen(base_path) + max_digit_chars + 2; // for '.' and \0
my_path = (char*)os::malloc(len, mtInternal);
if (my_path == nullptr) {
warning("Cannot create heap dump file. Out of system memory.");
return;
}
jio_snprintf(my_path, len, "%s.%d", base_path, dump_file_seq);
}
dump_file_seq++; // increment seq number for next time we dump
@ -2819,5 +2794,4 @@ void HeapDumper::dump_heap(bool oome) {
HeapDumper dumper(false /* no GC before heap dump */,
oome /* pass along out-of-memory-error flag */);
dumper.dump(my_path, tty, HeapDumpGzipLevel);
os::free(my_path);
}

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2025, 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
@ -71,10 +71,12 @@ public class TestHeapDumpOnOutOfMemoryError {
}
}
test(args[1]);
System.out.println("PASSED");
}
static void test(String type) throws Exception {
String heapdumpFilename = type + ".hprof";
// Test using %p pid substitution in HeapDumpPath:
String heapdumpFilename = type + ".%p.hprof";
ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder("-XX:+HeapDumpOnOutOfMemoryError",
"-XX:HeapDumpPath=" + heapdumpFilename,
// Note: When trying to provoke a metaspace OOM we may generate a lot of classes. In debug VMs this
@ -94,13 +96,10 @@ public class TestHeapDumpOnOutOfMemoryError {
OutputAnalyzer output = new OutputAnalyzer(pb.start());
output.stdoutShouldNotBeEmpty();
output.shouldContain("Dumping heap to " + type + ".hprof");
File dump = new File(heapdumpFilename);
Asserts.assertTrue(dump.exists() && dump.isFile(),
"Could not find dump file " + dump.getAbsolutePath());
HprofParser.parse(new File(heapdumpFilename));
System.out.println("PASSED");
String expectedHeapdumpFilename = type + "." + output.pid() + ".hprof";
output.shouldContain("Dumping heap to " + expectedHeapdumpFilename);
File dump = new File(expectedHeapdumpFilename);
Asserts.assertTrue(dump.exists() && dump.isFile(), "Expected heap dump file " + dump.getAbsolutePath());
HprofParser.parse(new File(expectedHeapdumpFilename));
}
}