RFR: 8255917: runtime/cds/SharedBaseAddress.java failed "assert(reserved_rgn != 0LL) failed: No reserved region" [v4]

[Ioi Lam]

On Tue, 8 Dec 2020 16:45:20 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Hi Ioi,
>> 
>> Where is the release you worry about? Since on Windows, since 8255978, os::release_memory() notices if the region the caller wants release does not correspond exactly to a memory mapping at the OS level, and will assert. Do you see that assert?
>> 
>> I try to understand:
>> 
>> if useBaseAddress==true, Yumin now creates two separate mappings, and can release them individually
>> if useBaseAddress==false, there is one mapping as before, but we split now shallow. But we don't release it since we use file IO to read into it. If someone were to release one of those, we should see an assert on Windows.
>> (I am a tiny bit unhappy about the increasing complexity of the patch, since it negates some of the work done to simplify it back in June.)
>> 
>> About tracing, since 8256864 we trace Virtualxxx calls, so the tracing is already there - we trace VirtualFree() errrors for "os=info". If you only care for windows, that tracing could suffice.
>> 
>> Thanks, Thomas
>
> If `useBaseAddress==false`, after the total_space is successfully mapped, subsequent operations mail fail. For example,
> 
> - `mapinfo->map_regions()` may fail to commit the necessary memory for doing `os::read()`.
> - `mapinfo->validate_shared_path_table()` may fail because the runtime classpath is not compatible
> 
> In these cases, we need to call MetaspaceShared::release_reserved_spaces().
> 
>> Do you see that assert?
> 
> Hmm, I think we should add a new test for this specifically: `java -XX:ArchiveRelocationMode=1 -cp mispatched.jar` to force the failure in `mapinfo->validate_shared_path_table()`.
> 
>>  If you only care for windows, that tracing could suffice.
> 
> Since we are changing the `split` parameter for all platforms, I think we should test for all platforms, not just windows.

The new test case can be something like this:

$ java -XX:DumpLoadedClassList=HelloWorld.classlist -cp HelloWorld.jar HelloWorld
Hello World
$ java -Xshare:dump -XX:SharedClassListFile=HelloWorld.classlist -cp HelloWorld.jar \
       -XX:SharedArchiveFile=tmp.jsa -XX:SharedBaseAddress=0 
$ java -Xshare:auto -XX:SharedArchiveFile=tmp.jsa -cp XXX.jar:HelloWorld.jar -Xlog:cds \
       -showversion HelloWorld
...
[0.034s][info][cds] Archive(s) were created with -XX:SharedBaseAddress=0. Always map at os-selected address.
[0.034s][info][cds] Try to map archive(s) at an alternative address
[0.034s][info][cds] Mapped static  region #0 at base 0x00007f21f7800000 top 0x00007f21f7806000 (MiscCode)
[0.034s][info][cds] Mapped static  region #1 at base 0x00007f21f7806000 top 0x00007f21f7a1d000 (ReadWrite)
[0.034s][info][cds] Mapped static  region #2 at base 0x00007f21f7a1d000 top 0x00007f21f7daf000 (ReadOnly)
[0.048s][info][cds] UseSharedSpaces: shared class paths mismatch (hint: enable -Xlog:class+path=info to diagnose the failure)
[0.048s][info][cds] Unmapping region #0 at base 0x00007f21f7800000 (MiscCode)
[0.048s][info][cds] Unmapping region #1 at base 0x00007f21f7806000 (ReadWrite)
[0.048s][info][cds] Unmapping region #2 at base 0x00007f21f7a1d000 (ReadOnly)
[0.048s][info][cds] UseSharedSpaces: Unable to map shared spaces
java version "16-internal" 2021-03-16
Java(TM) SE Runtime Environment (slowdebug build 16-internal+0-adhoc.iklam.open)
Java HotSpot(TM) 64-Bit Server VM (slowdebug build 16-internal+0-adhoc.iklam.open, mixed mode)
Hello World
You can set a breakpoint at `MetaspaceShared::release_reserved_spaces()` to make sure it's called with `total_rs`.

Note that `-Xshare:auto` must be used. 

If `-Xshare:on` is used, the VM will exit immediately without calling `MetaspaceShared::release_reserved_spaces()`. Most of the CDS tests are executed with `-Xshare:on`. That's why we didn't see the assert on Windows with Yumin's earlier patch -- in Mach5 tier 4, we run test/hotspot/jtreg/runtime/cds/appcds/BootClassPathMismatch.java with `-XX:ArchiveRelocationMode=1`, but the test itself would use with `-Xshare:on`.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1657