8222379: JFR TestClassLoadEvent.java failed due to EXCEPTION_ACCESS_VIOLATION
authorcoleenp
Thu, 18 Apr 2019 07:02:07 -0400
changeset 54573 b73893f7fee3
parent 54572 75a42622414e
child 54574 7b74bbe5085b
8222379: JFR TestClassLoadEvent.java failed due to EXCEPTION_ACCESS_VIOLATION Summary: Give fatal error if CDS loses archive mapping. Reviewed-by: iklam, ccheung, jiangli
src/hotspot/os/windows/os_windows.cpp
src/hotspot/share/memory/filemap.cpp
src/hotspot/share/runtime/arguments.cpp
src/hotspot/share/runtime/arguments.hpp
src/hotspot/share/utilities/macros.hpp
--- a/src/hotspot/os/windows/os_windows.cpp	Fri Apr 12 09:13:50 2019 +0200
+++ b/src/hotspot/os/windows/os_windows.cpp	Thu Apr 18 07:02:07 2019 -0400
@@ -5000,17 +5000,13 @@
                           char *addr, size_t bytes, bool read_only,
                           bool allow_exec) {
   // This OS does not allow existing memory maps to be remapped so we
-  // have to unmap the memory before we remap it.
-  if (!os::unmap_memory(addr, bytes)) {
-    return NULL;
-  }
-
-  // There is a very small theoretical window between the unmap_memory()
-  // call above and the map_memory() call below where a thread in native
-  // code may be able to access an address that is no longer mapped.
-
-  return os::map_memory(fd, file_name, file_offset, addr, bytes,
-                        read_only, allow_exec);
+  // would have to unmap the memory before we remap it.
+
+  // Because there is a small window between unmapping memory and mapping
+  // it in again with different protections, CDS archives are mapped RW
+  // on windows, so this function isn't called.
+  ShouldNotReachHere();
+  return NULL;
 }
 
 
--- a/src/hotspot/share/memory/filemap.cpp	Fri Apr 12 09:13:50 2019 +0200
+++ b/src/hotspot/share/memory/filemap.cpp	Thu Apr 18 07:02:07 2019 -0400
@@ -806,13 +806,14 @@
                                 addr, size, false /* !read_only */,
                                 si->_allow_exec);
   close();
+  // These have to be errors because the shared region is now unmapped.
   if (base == NULL) {
-    fail_continue("Unable to remap shared readonly space (errno=%d).", errno);
-    return false;
+    log_error(cds)("Unable to remap shared readonly space (errno=%d).", errno);
+    vm_exit(1);
   }
   if (base != addr) {
-    fail_continue("Unable to remap shared readonly space at required address.");
-    return false;
+    log_error(cds)("Unable to remap shared readonly space (errno=%d).", errno);
+    vm_exit(1);
   }
   si->_read_only = false;
   return true;
@@ -849,10 +850,17 @@
   size_t size = align_up(used, alignment);
   char *requested_addr = region_addr(i);
 
-  // If a tool agent is in use (debugging enabled), we must map the address space RW
-  if (JvmtiExport::can_modify_any_class() || JvmtiExport::can_walk_any_space()) {
+#ifdef _WINDOWS
+  // Windows cannot remap read-only shared memory to read-write when required for
+  // RedefineClasses, which is also used by JFR.  Always map windows regions as RW.
+  si->_read_only = false;
+#else
+  // If a tool agent is in use (debugging enabled), or JFR, we must map the address space RW
+  if (JvmtiExport::can_modify_any_class() || JvmtiExport::can_walk_any_space() ||
+      Arguments::has_jfr_option()) {
     si->_read_only = false;
   }
+#endif // _WINDOWS
 
   // map the contents of the CDS archive in this memory
   char *base = os::map_memory(_fd, _full_path, si->_file_offset,
@@ -868,7 +876,6 @@
   MemTracker::record_virtual_memory_type((address)base, mtClassShared);
 #endif
 
-
   if (!verify_region_checksum(i)) {
     return NULL;
   }
--- a/src/hotspot/share/runtime/arguments.cpp	Fri Apr 12 09:13:50 2019 +0200
+++ b/src/hotspot/share/runtime/arguments.cpp	Thu Apr 18 07:02:07 2019 -0400
@@ -271,17 +271,25 @@
 }
 
 #if INCLUDE_JFR
+static bool _has_jfr_option = false;  // is using JFR
+
 // return true on failure
 static bool match_jfr_option(const JavaVMOption** option) {
   assert((*option)->optionString != NULL, "invariant");
   char* tail = NULL;
   if (match_option(*option, "-XX:StartFlightRecording", (const char**)&tail)) {
+    _has_jfr_option = true;
     return Jfr::on_start_flight_recording_option(option, tail);
   } else if (match_option(*option, "-XX:FlightRecorderOptions", (const char**)&tail)) {
+    _has_jfr_option = true;
     return Jfr::on_flight_recorder_option(option, tail);
   }
   return false;
 }
+
+bool Arguments::has_jfr_option() {
+  return _has_jfr_option;
+}
 #endif
 
 static void logOption(const char* opt) {
--- a/src/hotspot/share/runtime/arguments.hpp	Fri Apr 12 09:13:50 2019 +0200
+++ b/src/hotspot/share/runtime/arguments.hpp	Thu Apr 18 07:02:07 2019 -0400
@@ -649,6 +649,8 @@
   static bool check_unsupported_cds_runtime_properties() NOT_CDS_RETURN0;
 
   static bool atojulong(const char *s, julong* result);
+
+  static bool has_jfr_option() NOT_JFR_RETURN_(false);
 };
 
 // Disable options not supported in this release, with a warning if they
--- a/src/hotspot/share/utilities/macros.hpp	Fri Apr 12 09:13:50 2019 +0200
+++ b/src/hotspot/share/utilities/macros.hpp	Thu Apr 18 07:02:07 2019 -0400
@@ -279,8 +279,10 @@
 
 #if INCLUDE_JFR
 #define JFR_ONLY(code) code
+#define NOT_JFR_RETURN_(code) /* next token must be ; */
 #else
 #define JFR_ONLY(code)
+#define NOT_JFR_RETURN_(code) { return code; }
 #endif
 
 #ifndef INCLUDE_JVMCI