[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR (S): 8237777 "Dumping core ..." is shown despite claiming that "# No core dump will be writt
From:       Yasumasa Suenaga <suenaga () oss ! nttdata ! com>
Date:       2020-04-30 23:51:42
Message-ID: 31df089a-4394-5855-9001-5e27c845887b () oss ! nttdata ! com
[Download RAW message or body]

Looks good.

Yasumasa (ysuenaga)


On 2020/05/01 1:15, gerard ziemski wrote:
> 
> 
> On 4/29/20 6:33 PM, David Holmes wrote:
> > 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. ??
> 
> Sorry, here is the webrev for all the relevant platforms:
> 
> webrev:http://cr.openjdk.java.net/~gziemski/8237777_rev4
> testing: hs_tier1,2,3,4,5,6,7 in progress...
> 
> I wasn't 100% sure whether to include Solaris here, but I did in the end, though I \
> can skip it if that will mess up the effort to remove Solaris support. 
> 
> cheers
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]