RFR 8222379: JFR TestClassLoadEvent.java failed due to EXCEPTION_ACCESS_VIOLATION

[coleen.phillimore at oracle.com]

On 4/17/19 1:50 PM, Jiangli Zhou wrote:
> Hi Coleen,
>
> Looks reasonable to me.
>
> - src/hotspot/os/windows/os_windows.cpp
> 5005   // There is a very small theoretical window between the unmap_memory()
> 5006   // call above and the map_memory() call below where a thread in native
> 5007   // code may be able to access an address that is no longer mapped.
> The comment refers to the 'unmap_memory() call above and the map_memory() call below'. Since the calls are removed, could you please update the comment as well.

I can update the comments.  How about:

// Remap a block of memory.
char* os::pd_remap_memory(int fd, const char* file_name, size_t file_offset,
                           char *addr, size_t bytes, bool read_only,
                           bool allow_exec) {
   // This OS does not allow existing memory maps to be remapped so we
   // 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;
}


> - src/hotspot/share/memory/filemap.cpp
> 853 // If a tool agent is in use (debugging enabled), or JFR, we must 
> map the address space RW
> 854 if (JvmtiExport::can_modify_any_class() || 
> JvmtiExport::can_walk_any_space() ||
> 855 Arguments::has_jfr_option()) {
>   856     si->_read_only = false;
>   857   }
> 858 #ifdef _WINDOWS
> 859 // Windows cannot remap read-only shared memory to read-write when 
> required for
> 860 // RedefineClasses, which is also used by JFR. Always map windows 
> regions as RW.
> 861 si->_read_only = false;
> 862 #endif
> To avoid setting si->_read_only twice on Windows, maybe putting the code at line #853-#857 under #else after #ifdef _WINDOWS?

Okay, that's an improvement too.

Thanks Jiangli!
Coleen

> Best regards,
> Jiangli
>
> On Wed, Apr 17, 2019 at 5:25 AM <coleen.phillimore at oracle.com 
> <mailto:coleen.phillimore at oracle.com>> wrote:
>
>     Summary: Give fatal error if CDS loses archive mapping; but map
>     Windows
>     RW because remapping is dangerous.
>
>     Ioi and I discussed this change and thought it is best. Windows only
>     maps the CDS archive around 50% time because of ASLR and this retains
>     the startup performance improvements for CDS on windows.
>
>     Tested with mach5 tier1-3.
>
>     open webrev at
>     http://cr.openjdk.java.net/~coleenp/2019/8222379.01/webrev
>     bug link https://bugs.openjdk.java.net/browse/JDK-8222379
>
>     Thanks,
>     Coleen
>