[PATCH] Improve Windows Minidump File Specification
Matthew Briggs
matthew.e.briggs at gmail.com
Wed Jul 31 09:59:25 PDT 2013
The -XX:CrashFileDirectory idea is appealing to me. My main concern
with the Windows minidump case isn't the ability to have fine grained
contorl over the actual minidump filename, but rather just being able
to create the file in a location that I can specify so as to ensure
permissions, etc. won't block an attempt. -XX:CrashFileDirectory
handles that and seems like a good way to express the feature across
platforms.
I'm more uncertain on -XX:ErrorFile and how to possibly blend that
into any enhancements to the core/mini dump locations. For Windows
mini dumps I had just taken the view of "-XX:ErrorFile is a given,
it's a well established and functional feature across platforms, so
just try to tweak the mini dump handling to be more like that". I
could see an appeal to having both dumps and the error log text file
all bundled together in the same output directory, though.
I'm including a revision of my original patch, which introduces the
-XX:CrashFileDirectory option and adjusts the Windows processing
logic. The minidump file will always be called hs_err_pid%p.mdmp, but
now attempts will be made to write it in the location specified by
-XX:CrashFileDirectory, then the working directory, and finally an OS
temp directory.
Thanks,
Matt
diff -r 9d7b55c8a0c4 src/os/windows/vm/os_windows.cpp
--- a/src/os/windows/vm/os_windows.cpp Thu Jul 25 03:18:31 2013 -0700
+++ b/src/os/windows/vm/os_windows.cpp Wed Jul 31 17:53:48 2013 +0100
@@ -934,6 +934,57 @@
}
}
+/** Expand a pattern into a buffer starting at pos and create a file
using constructed path */
+static HANDLE expand_and_create(const char* pattern, char* buf,
size_t buflen, size_t pos) {
+ HANDLE file = INVALID_HANDLE_VALUE;
+ if (Arguments::copy_expand_pid(pattern, strlen(pattern), &buf[pos],
buflen - pos)) {
+ file = CreateFile(buf, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS,
FILE_ATTRIBUTE_NORMAL, NULL);
+ }
+ return file;
+}
+
+/**
+ * Construct file name for a minidump file and return it's file handle.
+ * Name and location depends on crashFileDirectory, file_pattern
params and access
+ * permissions.
+ */
+static HANDLE prepare_minidump_file(const char* crashFileDirectory,
const char* file_pattern, char* buf, size_t buflen) {
+ HANDLE file = INVALID_HANDLE_VALUE;
+
+ // If possible, use specified directory to construct file name
+ if (crashFileDirectory != NULL && strlen(crashFileDirectory) > 0) {
+ int pos = jio_snprintf(buf, buflen, "%s%s", crashFileDirectory,
os::file_separator());
+ if (pos > 0) {
+ file = expand_and_create(file_pattern, buf, buflen, pos);
+ }
+ }
+
+ // Either user didn't specify, or the user's location failed, so
try the current directory
+ if (file == INVALID_HANDLE_VALUE) {
+ const char* cwd = os::get_current_directory(buf, buflen);
+ if (cwd != NULL) {
+ size_t pos = strlen(cwd);
+ int fsep_len = jio_snprintf(&buf[pos], buflen-pos, "%s",
os::file_separator());
+ pos += fsep_len;
+ if (fsep_len > 0) {
+ file = expand_and_create(file_pattern, buf, buflen, pos);
+ }
+ }
+ }
+
+ // Try temp directory if it exists.
+ if (file == INVALID_HANDLE_VALUE) {
+ const char* tmpdir = os::get_temp_directory();
+ if (tmpdir != NULL && strlen(tmpdir) > 0) {
+ int pos = jio_snprintf(buf, buflen, "%s%s", tmpdir,
os::file_separator());
+ if (pos > 0) {
+ file = expand_and_create(file_pattern, buf, buflen, pos);
+ }
+ }
+ }
+
+ return file;
+}
static BOOL (WINAPI *_MiniDumpWriteDump) ( HANDLE, DWORD, HANDLE,
MINIDUMP_TYPE, PMINIDUMP_EXCEPTION_INFORMATION,
PMINIDUMP_USER_STREAM_INFORMATION, PMINIDUMP_CALLBACK_INFORMATION);
@@ -948,7 +999,6 @@
DWORD processId = GetCurrentProcessId();
HANDLE dumpFile;
MINIDUMP_TYPE dumpType;
- static const char* cwd;
// Default is to always create dump for debug builds, on product
builds only dump on server versions of Windows.
#ifndef ASSERT
@@ -994,10 +1044,7 @@
MiniDumpWithUnloadedModules);
#endif
- cwd = get_current_directory(NULL, 0);
- jio_snprintf(buffer, bufferSize, "%s\\hs_err_pid%u.mdmp",cwd,
current_process_id());
- dumpFile = CreateFile(buffer, GENERIC_WRITE, 0, NULL,
CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
-
+ dumpFile = prepare_minidump_file(CrashFileDirectory,
"hs_err_pid%p.mdmp", buffer, bufferSize);
if (dumpFile == INVALID_HANDLE_VALUE) {
VMError::report_coredump_status("Failed to create file for
dumping", false);
return;
diff -r 9d7b55c8a0c4 src/share/vm/runtime/globals.hpp
--- a/src/share/vm/runtime/globals.hpp Thu Jul 25 03:18:31 2013 -0700
+++ b/src/share/vm/runtime/globals.hpp Wed Jul 31 17:53:48 2013 +0100
@@ -825,6 +825,9 @@
product(bool, CreateMinidumpOnCrash, false, \
"Create minidump on VM fatal error") \
\
+ product(ccstr, CrashFileDirectory , NULL, \
+ "If a crash file is created, save to this directory") \
+ \
product_pd(bool, UseOSErrorReporting, \
"Let VM fatal error propagate to the OS (ie. WER on Windows)") \
\
On Wed, Jul 31, 2013 at 4:47 AM, David Holmes <david.holmes at oracle.com> wrote:
> On 31/07/2013 5:44 PM, Richard Fearn wrote:
>>>
>>> It seems like you are asking for a similar piece of functionality as
>>> Richard(cc:d) did:
>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2013-July/010390.html
>>
>>
>> Yes, sounds very similar...
>>
>>> Since Windows is the only platform where we can decide on the file name
>>> of
>>> the mdmp/core file it would be good if we could make this into a
>>> platform-independent flag -XX:CrashFileDirectory or something to that
>>> effect.
>>>
>>> Then we could change the code to create the hs_err_pid<num>.log in that
>>> directory
>>
>>
>> The location of that file (the fatal error log) can already be
>> specified by -XX:ErrorFile, though.
>>
>>> and
>>> * change the Windows code to create the mdmp in that directory
>>> * change the *nix code to chdir(2) to the target directory before calling
>>> abort(3)
>>
>>
>> As an experiment, I added a -XX:CoreDumpDirectory option, and did the
>> chdir quite early - before the fatal error log was written. This meant
>> the fatal error log went into the CoreDumpDirectory, too, which I
>> wasn't happy with, given the option name. However, if the option was
>> called CrashFileDirectory, it would make more sense if the fatal error
>> log were written into that directory as well.
>>
>> I moved the chdir later, but hit another problem: the fatal error log
>> gives the current directory as a suggestion as to where the core dump
>> can be found.
>
>
> That of course would have to be updated as part of a patch to add a specific
> dump location.
>
> David
> -----
>
>
>
> If the current directory is changed later on, that will
>>
>> be incorrect. A nice side effect of allowing the fatal error log to go
>> into the CrashFileDirectory would be that the current directory
>> *could* be changed earlier (unless there's something else that would
>> be affected by this, that I haven't spotted).
>>
>> I'd be happy to provide patches for the changes I've made, though I
>> agree that it might be good to have a unified approach to this.
>>
>> Rich
>>
>
More information about the hotspot-runtime-dev
mailing list