RFR: 8338883: Add core dump info to -Xlog:os=info [v4]

David Holmes dholmes at openjdk.org
Tue Sep 10 08:03:07 UTC 2024


On Mon, 9 Sep 2024 20:31:39 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> We add a small feature, which adds core dump info, which till now was only printed as part of hs_err log file, directly to stdout, when desired using log mechanism, i.e. `-Xlog:os=info`
>> 
>> For example, if we see:
>> 
>> `core dump info: core.28283`
>> 
>> we know all is set up correctly and we can expect a core file if java process crashes. If we see:
>> 
>> `core dump info: Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again`
>> 
>> instead, however, we know that "ulimit -c unlimited" needs to be set.
>> 
>> Testing: 
>> - passes `"MACH5 runtime/ErrorHandling/CreateCoredumpOnCrash.java"`
>> - full MACH5 test in progress...
>
> Gerard Ziemski has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix bufferSize name

Changes requested by dholmes (Reviewer).

src/hotspot/os/posix/os_posix.cpp line 143:

> 141:   }
> 142: 
> 143:   VMError::record_coredump_status(buffer, status);

I think this was meant to be conditional:
Suggestion:

if (!check_only) {
  VMError::record_coredump_status(buffer, status);
}

src/hotspot/os/windows/os_windows.cpp line 1321:

> 1319:   }
> 1320: 
> 1321:   VMError::record_coredump_status(buffer, status);

Again I think this was intended to be conditional on `check_only`.

test/hotspot/jtreg/runtime/ErrorHandling/CreateCoredumpOnCrash.java line 58:

> 56:           oa.reportDiagnosticSummary();
> 57:           oa.shouldContain("core dump info").shouldNotContain("CreateCoredumpOnCrash turned off, no core file dumped").
> 58:                   shouldNotHaveExitValue(0);

Nit: you should call `reportDiagnosticSummary` last otherwise on failure it will be called twice: once directly  by the test and then indirectly by the failing "should" function.

-------------

PR Review: https://git.openjdk.org/jdk/pull/20734#pullrequestreview-2291657193
PR Review Comment: https://git.openjdk.org/jdk/pull/20734#discussion_r1751454172
PR Review Comment: https://git.openjdk.org/jdk/pull/20734#discussion_r1751456393
PR Review Comment: https://git.openjdk.org/jdk/pull/20734#discussion_r1751459241


More information about the hotspot-runtime-dev mailing list