On Mon, 25 Jan 2021 19:43:56 GMT, Chris Plummer <[email protected]> wrote:
>> Roland Westrelin has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> use CoreUtils
>
> test/lib/jdk/test/lib/util/CoreUtils.java line 193:
>
>> 191: public static String getCoreFileLocation(String crashOutputString)
>> throws IOException {
>> 192: return getCoreFileLocation(crashOutputString, -1);
>> 193: }
>
> In order to be consistent, it seems that all users of `getCoreFileLocation()`
> should now be passing in the pid and we should get rid of this version that
> doesn't require it. That means they also all need to be converted to use
> `ProcessTools.executeProcessPreservePID()`.
I pushed a new change that updates all tests.
> test/lib/jdk/test/lib/util/CoreUtils.java line 176:
>
>> 174: Asserts.assertGT(coreFile.length(), 0L,
>> "Unexpected core size");
>> 175: System.out.println("coredumpctl
>> succeeded");
>> 176: return core;
>
> At the start of the outermost if/else block, there is a comment that says
> "See if we can figure out the likely reason the core file was not found".
> However, now we are doing further attempts to find the core file. At the very
> least the comment should be updated. Maybe add something like "Recover from
> failure if possible."
ok.
> test/hotspot/jtreg/compiler/ciReplay/CiReplayBase.java line 182:
>
>> 180: if (needCoreDump) {
>> 181: try {
>> 182: String coreFileLocation =
>> CoreUtils.getCoreFileLocation(crashOutputString);
>
> Don't you need to pass in `pid` here?
right.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2195
