RFR (S): 8237777 "Dumping core ..." is shown despite claiming that "# No core dump will be written."

[gerard ziemski]

Thank you for the question Dan!

I checked and I believe that we're OK. I also checked for the "Current 
thread is" phrase.

There are a few tests that show an example crash printout containing the 
phrase, but those are just comments and I decided against touching them 
for such trivial change.

I did schedule Mach5 hs_tier1,2,3,4,5,6,7 tests, so hopefully we'll be 
in good shape if they pass...


cheers

On 4/29/20 4:04 PM, Daniel D. Daugherty wrote:
>> Maybe we should simply remove the line:
>>
>>     out.print_raw_cr("Dumping core ...");
>>
>> completely?
>
> Please make sure that you check for tests that are checking for this
> output line/pattern.
>
> Dan
>
> On 4/29/20 12:39 PM, gerard ziemski wrote:
>>
>>
>> On 4/24/20 11:44 PM, David Holmes wrote:
>>> On 25/04/2020 4:02 am, gerard ziemski wrote:
>>>>
>>>>
>>>> On 4/23/20 7:55 PM, David Holmes wrote:
>>>>
>>>> [trimmed]
>>>>
>>>>> Given that the "Dumping core ..." is only shown in non-product 
>>>>> builds I think we are way over-thinking/engineering this. We have:
>>>>>
>>>>>   if (dump_core) {
>>>>>   ...
>>>>> #ifndef PRODUCT
>>>>>     ...
>>>>>     out.print_raw_cr("Dumping core ...");
>>>>> #endif
>>>>>     ::abort(); // dump core
>>>>>   }
>>>>>
>>>>> so lets just change one line:
>>>>>
>>>>>     out.print_raw_cr("Requesting a core dump if possible ...");
>>>>>
>>>>> otherwise you need one boolean to control whether we call abort vs 
>>>>> exit, and a second boolean to control what we print.
>>>>>
>>>>> Or change that one line to:
>>>>>
>>>>>     out.print_raw_cr("Calling ::abort() ...");
>>>>
>>>> Making sure we do not mess up the core creation logic is paramount, 
>>>> so I like your suggestion to keep it simple, but perhaps we can 
>>>> take it one small step further and just not bother with that 
>>>> message at all?
>>>>
>>>> Maybe we should simply remove the line:
>>>>
>>>>     out.print_raw_cr("Dumping core ...");
>>>>
>>>> completely? After all, we print lots of core dumping related info 
>>>> anyhow.
>>>
>>> We've been printing
>>>
>>>   Current thread is xxx
>>>   Dumping core ...
>>>
>>> to the default output stream in non-product builds since day one of 
>>> this code - literally. Why only non-product and why only for when 
>>> dump_core is true I have no idea. The whole thing seems somewhat 
>>> superfluous. I'd suggest either tweaking the second line, or 
>>> removing it all, rather than just deleting the second line (which 
>>> gives some additional context to the first).
>>
>> I gave it some thought, looked at a few Linux distros, and personally 
>> I'd like to see both of those lines to go away. We already print the 
>> crashed thread info in the hs_err crash log, even before we get to 
>> those 2 lines, so we're not risking loosing the "current thread" info.
>>
>> bug:https://bugs.openjdk.java.net/browse/JDK-8237777
>> webrev:http://cr.openjdk.java.net/~gziemski/8237777_rev3
>> tests:  tested with Mach5 tier-hs1,2,3,4,5
>>
>> Lastly, I believe that there is still much we can do to improve the 
>> core file related messages, but that will be covered in JDK-8243196
>>
>>
>> cheers
>>
>