RFR 8222379: JFR TestClassLoadEvent.java failed due to EXCEPTION_ACCESS_VIOLATION

[Jiangli Zhou]

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.

- 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?

Best regards,

Jiangli


On Wed, Apr 17, 2019 at 5:25 AM <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
>
>