[PATCH] Improve Windows Minidump File Specification

[Matthew Briggs]

Hello,

In running a Java application in a Windows Service context, minidump files
have proven helpful in diagnosing crashes due to faulty native code
(invoked through JNI).  In production contexts I'm familiar with, these
minidumps are often not produced, though, due to a combination of
configuration and limitations in the JVM's minidump location choices.
 Specifically, the JVM only allows for minidump files to be created in the
working directory of the Windows Service process.  A typical production
deployment runs the Windows Service under the limited NETWORK SERVICE
account, which in particular cannot write to its working directory (usually
located under C:\Program Files).

The included patch establishes a new -XX:MinidumpFile JVM option, in the
spirit of the existing -XX:ErrorFile option.  The handling code for the
latter is adapted to maintain similar structure and functionality.  A
Windows minidump file may thus be specified by a user (including support
for %p syntax), falling back to a default filename targeted in the process
working directory and then an OS temporary directory.

I've manually tested the patch using a simple Java application that
intentionally causes an access violation through JNI invocation of native
code.  I've tested explicit user specification of -XX:MinidumpFile, with
and without %p,  along with the two fallback cases.

This is my first interaction with the OpenJDK community, so I apologize in
advance if I've failed to follow proper patch submission guidelines and
procedures.  I'm eager to learn more about how the community operates, and
to hear if this patch in particular is of interest.  Any feedback is
appreciated.

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 Sun Jul 28 14:50:32 2013 +0100
@@ -934,6 +934,55 @@
   }
 }

+/** 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 pattern, default_pattern params and access
+ * permissions.
+ */
+static HANDLE prepare_minidump_file(const char* pattern, const char*
default_pattern, char* buf, size_t buflen) {
+  HANDLE file = INVALID_HANDLE_VALUE;
+
+  // If possible, use specified pattern to construct file name
+  if (pattern != NULL) {
+    file = expand_and_create(pattern, buf, buflen, 0);
+  }
+
+  // Either user didn't specify, or the user's location failed,
+  // so use the default name in 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(default_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(default_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 +997,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 +1042,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(MinidumpFile, "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 Sun Jul 28 14:50:32 2013 +0100
@@ -825,6 +825,10 @@
   product(bool, CreateMinidumpOnCrash, false,
  \
           "Create minidump on VM fatal error")
 \

 \
+  product(ccstr, MinidumpFile, NULL,
 \
+         "If a minidump is created, save to this file"
 \
+         "[default: ./hs_err_pid%p.mdmp] (%p replaced with pid)")
  \
+
 \
   product_pd(bool, UseOSErrorReporting,
  \
           "Let VM fatal error propagate to the OS (ie. WER on Windows)")
 \

 \
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130728/00bc36bd/attachment.html