RFR: (S): JDK-8200613: SA: jstack throws UnmappedAddressException with a CDS core file

[Jini George]

Thank you very much, Chris, Kevin and Ioi for your comments!

I will send another webrev with this change enabled under an opt-out 
flag, as you suggest, and would look at unmapping the JDK modules file 
and if possible, the NIO mapped files too in the signal handler.

Thanks a bunch,
Jini.

On 11/9/2018 11:53 PM, Ioi Lam wrote:
> Hi Jini,
> 
> Thanks for investigating the size expansion issue.
> 
> I agree that the size increase is worth it. Even when not using SA, if 
> we open the core file inside GDB, we cannot read certain sections in the 
> CDS archive (such as the RO section and strings sections). That would 
> make debugging difficult. So I am in favor of this change.
> 
> For the JDK modules file, maybe we can unmap it in the signal handler, 
> before going ahead with the core dump? I think it's hardly needed for 
> debugging purposes. (Perhaps we can also do the same for the NIO mapped 
> files?)
> 
> A opt-flag as suggested by Kevin is a good idea.
> 
> Thanks
> 
> - Ioi
> 
> On 11/9/18 3:29 AM, Kevin Walls wrote:
>> Hi Jini,
>>
>> Looks good to me.  It might be a significant increase in size of 
>> _some_ core files, but so many core files we see are much larger, in 
>> gigabytes++ of course, so the CDS data size should not be such a 
>> significant increase on (I think) most files.
>>
>> The flexibiity of always having the CDS data there is very 
>> significant.  A core file should ideally be usable, without 
>> additionally requiring the CDS archive from the machine.  That 
>> additional human round-trip upload request on every transmitted core 
>> that needs investigating, seems like a less efficient route...).
>>
>> Is there an opt-out?  It's conditional on UseSharedSpaces but could 
>> there be a flag to disable, in case we see crashes with gigabytes of 
>> private mappings that we really don't want to retain (the user would 
>> have to know to set a flag, to disable the new coredump filter ahead 
>> of time).
>>
>> Thanks!
>> Kevin
>>
>>
>> On 29/10/2018 06:02, Jini George wrote:
>>> Thank you very much, Ioi, for looking into this, and the 
>>> clarification offline. My bad, I had missed the earlier mail from 
>>> you. :-( My responses below.
>>>
>>> Yes, I had tested this on MacOS. The issue does not exist on MacOS 
>>> since the file backed private mmap()-ed regions get dumped into the 
>>> MacOS corefiles by default.
>>>
>>> The corefile sizes on Linux do increase due to this change. And the 
>>> increase would also include any file mapped using NIO with 
>>> MapMode.PRIVATE. The typical corefile size increase with this change 
>>> would include the following components at a high level:
>>>
>>> * Any NIO file mapping with MapMode.PRIVATE.
>>> * Any file mmap()-ed by any native library with MAP_PRIVATE.
>>> * The read only CDS regions (ro and od): Of the order of a few MB.
>>> * The shared strings CDS region. (typically less than 1 MB).
>>> * 2 MB per native shared library (regions with ---p permissions 
>>> mapped by the dynamic linker for better alignment and for keeping 
>>> libraries efficiently shareable).
>>> * The JDK 'modules' file. (About 140 MB).
>>>
>>> So, without including any NIO mapping, I typically see around 250-300 
>>> MB increase in the corefile sizes. I agree that the size increase 
>>> could be a cause for concern, but for FWIW, these privately mapped 
>>> files get dumped into the corefile for MacOS too. And the corefile 
>>> sizes for the same program on MacOS are way larger (of the order of a 
>>> few GB as against about 300 MB on Linux (without the change)).
>>>
>>> The advantage of fixing this by modifying the coredump_filter v/s 
>>> doing it in SA (by reading in more sections of the shared archive 
>>> file) is that this would benefit other debuggers like gdb also. (And 
>>> reduces the dependence on having the shared archive file being 
>>> available at the time of debugging). If folks still think this is a 
>>> cause for concern, I could make modifications to fix this by reading 
>>> in the regions from the shared archive file in the SA code. I also 
>>> wonder if it is worth taking a relook at the mapping types of the 
>>> various CDS regions also.
>>>
>>> Thank you,
>>> Jini.
>>>
>>> On 10/22/2018 10:27 AM, Ioi Lam wrote:
>>>> Hi Jini,
>>>>
>>>> Did you see my earlier reply? I might have sent it out during the 
>>>> mail server outage days :-(
>>>>
>>>> http://openjdk.5641.n7.nabble.com/RFR-S-JDK-8200613-SA-jstack-throws-UnmappedAddressException-with-a-CDS-core-file-td352681.html#a353026 
>>>>
>>>>
>>>> Here was my reply again:
>>>>
>>>>> Hi Jini,
>>>>>
>>>>> The changes looks good to me.
>>>>>
>>>>> Have you tested this on MacOS? CDS heap support is also enabled on
>>>>> MacOS. See macros.hpp:
>>>>>
>>>>>      #if INCLUDE_CDS && INCLUDE_G1GC && defined(_LP64) && 
>>>>> !defined(_WINDOWS)
>>>>>      #define INCLUDE_CDS_JAVA_HEAP 1
>>>>>
>>>>> Also, besides CDS, do we know how often other files will be mmaped 
>>>>> with
>>>>> MAP_PRIVATE? Will users get huge core files because CDS is enabled? In
>>>>> JDK 12, CDS will be enabled by default (see JDK-8202951), so all users
>>>>> will be affected by the following line:
>>>>>
>>>>>    if (UseSharedSpaces) {
>>>>>      set_coredump_filter(FILE_BACKED_PVT_BIT);
>>>>>    }
>>>>>
>>>>> Maybe you can run an big app such as Eclipse, trigger a core dump, and
>>>>> compare the size of the core file before/after this change?
>>>>>
>>>>> Thanks
>>>>>
>>>>> - Ioi 
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> Thanks
>>>>
>>>> - Ioi
>>>>
>>>>
>>>> On 10/21/18 8:58 PM, Jini George wrote:
>>>>> Gentle reminder!
>>>>>
>>>>> Thanks,
>>>>> - Jini
>>>>>
>>>>> On 10/9/2018 11:31 AM, Jini George wrote:
>>>>>> Hello!
>>>>>>
>>>>>> [Including runtime-dev since the changes are in runtime code]
>>>>>>
>>>>>> Requesting reviews for:
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8200613/webrev.01/
>>>>>> BugID: https://bugs.openjdk.java.net/browse/JDK-8200613
>>>>>>
>>>>>> Issue: jhsdb jstack would throw an UnmappedAddressException with a 
>>>>>> core file generated from a CDS enabled java process. This is seen 
>>>>>> only with Linux and with G1GC, while trying to read in data from 
>>>>>> the shared strings region (the closed archive heap space). This 
>>>>>> region (which is a file backed private memory region) is not 
>>>>>> dumped into the corefile for Linux. This, being a heap region (and 
>>>>>> therefore being a read-write region) is also not read in from the 
>>>>>> classes.jsa file in SA since only the read only regions are read 
>>>>>> in while processing the core file. (The expectation being that the 
>>>>>> read write regions are in the core file).
>>>>>>
>>>>>> Proposed solution: The proposed solution is to have the 
>>>>>> coredump_filter value corresponding to the CDS process to include 
>>>>>> bit 2 (file-backed private memory), so that the file-backed 
>>>>>> private memory region also gets dumped into the corefile. The 
>>>>>> proposed fix is in src/hotspot/os/linux/os_linux.cpp.
>>>>>>
>>>>>> Thanks,
>>>>>> Jini.
>>>>>>
>>>>
>>
>