RFR 8098821: Crash in system dictionary initialization with shared strings

[Daniel D. Daugherty]

On 6/17/15 1:24 PM, Coleen Phillimore wrote:
> Summary: map string regions after the compressed class base is known
>
> bug link https://bugs.openjdk.java.net/browse/JDK-8098821
>
> open webrev at http://cr.openjdk.java.net/~coleenp/8098821_jdk.01/

test/lib/sun/hotspot/WhiteBox.java
     No comments.

> open webrev at http://cr.openjdk.java.net/~coleenp/8098821_hotspot.01/

src/share/vm/memory/filemap.hpp
     No comments.


src/share/vm/memory/filemap.cpp
     L666:                      "size %dM.", max_heap_size() >> 20);
         Nit: There's got to be a macro to convert from size-in-bytes
         to size-in-MB...

     L686:         StringTable::ignore_shared_strings(true);
         Shouldn't this function have 'set_' as a prefix?

         Update: So the "getter" is "StringTable::shared_string_ignored()"
         and the "setter" is "StringTable::ignore_shared_strings(true)".
         That seems inconsistent with HotSpot style, but the naming
         pre-dates this change so OK.

     L724:       // the shared string data is mapped successfuly
         nit typo: 'successfuly' -> 'successfully'

src/share/vm/memory/metaspace.cpp
     L3300:     if (UseSharedSpaces) {
<snip>
     L3308:       if (mapinfo->initialize() && 
MetaspaceShared::map_shared_spaces(mapinfo)) {
<snip>
     L3323:       } else {
     L3324:         assert(!mapinfo->is_open() && !UseSharedSpaces,
     L3325:                "archive file not closed or shared spaces not 
disabled.");

         The assert() on L3324 is in the else-block for the
         if-statement on L3308. That's inside the if-block
         on L3300 so I don't see how the "&& !UseSharedSpaces"
         part of the assert() is working here.

         For what it's worth, it looks like the original code
         had a similar problem.

src/share/vm/memory/metaspaceShared.cpp
     old L1004:       mapinfo->map_string_regions() &&
         So I'm guessing that this map_string_regions() call has
         migrated to Metaspace::global_initialize() in metaspace.cpp.

     old L1005:       mapinfo->verify_string_regions() &&
         I'm not seeing where this verify_string_regions() call migrated.
         Is it no longer needed?

     old L1017:     mapinfo->unmap_string_regions();
         I'm not see where this unmap_string_regions() call migrated.
         Is it no longer needed?

src/share/vm/prims/whitebox.cpp
     No comments.


Dan


>
> Tested with failing tests on solaris and linux and JPRT.  The failing 
> scenario and code impact is limited, so running more tests wouldn't 
> exercise this code.
>
> New test code and white box API contributed by Misha.
>
> Thanks,
> Coleen
>
>
>