RFR: 8233706: JFR emergency dump should be performed after error reporting

[Yasumasa Suenaga]

Hi Thomas,

On 2020/02/02 18:32, Thomas Stüfe wrote:
> Hi Yasumasa,
> 
> very quick, not a full review yet.
> 
> can you please explain a bit more about:
> 
> - what does all this have to do with Windows MiniDumps?
> - why do we have to open the file descriptor early and stow it away?

Markus suggested it in [1].
In case of Windows, os::check_dump_limit() would construct dumpfile path and open file handle for dumping.
Dumping is happen latter. I guess it is for printing coredump location to hs_err.
Emergency JFR dump can perform similarly.


> - Not sure why you bother changing the return-resourcearea pattern to a caller provided buffer for create_emergency_dump_path()? I mean, using resourcearea is bad in error handling, so in principle this makes sense, but then the code creates another resourcearea buffer one frame up inside build_dump_path() anyway so why even bother.

Crash might be happen outside of JavaThread.
I saw error in VeryEarlyAssertTest jtreg test. It would happen SEGV outside of JavaThread.
So we cannot use the resource area in there.

Further investigation, I think all call path for build_dump_path() might be called from outside of JavaThread.
So I will change not to use ResourceMark.


> Small nit, instead of passing a pointer and a comment "array should be of length X" I would always do one of these:
> 
> void foo(char s[X]);
> or
> void foo(char* s, size_t len);
> 
> First variant is technically the same as passing a pointer but with clear documentation of intent, no comment needed.

Thank you for the comment. I will fix it in new webrev.


Thanks,

Yasumasa


[1] https://mail.openjdk.java.net/pipermail/hotspot-jfr-dev/2020-January/001044.html


> Cheers, Thomas
> 
> 
> On Sun, Feb 2, 2020 at 5:31 PM Yasumasa Suenaga <suenaga at oss.nttdata.com <mailto:suenaga at oss.nttdata.com>> wrote:
> 
>     Hi all,
> 
>     Please review this change.
> 
>         JBS: https://bugs.openjdk.java.net/browse/JDK-8233706
>         webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8233706/webrev.00/
> 
>     Error reporting (hs_err log) should happen as close as possible to the error point.
>     However JFR emergency dump would be performed before it.
> 
>     JFR emergency dump should be performed after error reporting (e.g. NMT reporting, CI Replay)
> 
>     This webrev separates opening FD like minidump for Windows and writing flight record.
>     Also create_emergency_dump_path() is no longer dependent on ResourceMark.
> 
> 
>     Thanks,
> 
>     Yasumasa
>