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

[David Holmes]

On 30/04/2020 2:39 am, 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.

I believe those 2 lines will always print to the console/tty. It is 
possible if hs_err log generation fails that only the console output may 
be seen. But in that case without a hs_err log I don't think those two 
lines really add anything useful.

> bug:https://bugs.openjdk.java.net/browse/JDK-8237777
> webrev:http://cr.openjdk.java.net/~gziemski/8237777_rev3

The webrev only contains os_bsd.cpp. ??

David

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